diff --git a/src/commands.ts b/src/commands.ts index 270df3eb3..6435debfc 100644 --- a/src/commands.ts +++ b/src/commands.ts @@ -20,6 +20,7 @@ import config from './commands/config/index.js' import { context, contextNonInteractive } from './commands/context/index.js' import cost from './commands/cost/index.js' import diff from './commands/diff/index.js' +import diagnostics from './commands/diagnostics/index.js' import dream from './commands/dream/index.js' import ctx_viz from './commands/ctx_viz/index.js' import doctor from './commands/doctor/index.js' @@ -301,6 +302,7 @@ const COMMANDS = memoize((): Command[] => [ cost, ctx_viz, diff, + diagnostics, dream, doctor, effort, diff --git a/src/commands/diagnostics/diagnostics.test.ts b/src/commands/diagnostics/diagnostics.test.ts new file mode 100644 index 000000000..0a77825d3 --- /dev/null +++ b/src/commands/diagnostics/diagnostics.test.ts @@ -0,0 +1,318 @@ +import { describe, expect, mock, test } from 'bun:test' +import type { DiagnosticFile } from '../../services/diagnosticTracking.js' +import { runDiagnosticsCommand, formatDiagnosticsOutput } from './diagnostics.js' + +const MAX_SAFE_TEST_OUTPUT_LENGTH = 1_500 + +type InitializationStatus = + | { status: 'not-started' } + | { status: 'pending' } + | { status: 'success' } + | { status: 'failed'; error: Error } + +function diagnostic( + message: string, + severity: 'Error' | 'Warning' | 'Info' | 'Hint', + line = 0, +) { + return { + message, + severity, + range: { + start: { line, character: 2 }, + end: { line, character: 8 }, + }, + source: 'typescript', + code: `${severity.toUpperCase()}_${line}`, + } +} + +function file(uri: string, diagnostics: DiagnosticFile['diagnostics']) { + return { uri, diagnostics } +} + +function diagnosticSet(files: DiagnosticFile[], serverName = 'typescript') { + return { serverName, files } +} + +function deps( + status: InitializationStatus, + diagnosticSets: Array<{ serverName: string; files: DiagnosticFile[] }> = [], +) { + return { + getInitializationStatus: () => status, + getPendingLSPDiagnosticsSnapshot: mock(() => diagnosticSets), + } +} + +describe('/diagnostics', () => { + test('returns a clear fallback when LSP status cannot be read', async () => { + const testDeps = { + getInitializationStatus: () => { + throw new Error('status unavailable \x1b[31m') + }, + getPendingLSPDiagnosticsSnapshot: mock(() => []), + } + + const output = await runDiagnosticsCommand('', testDeps) + + expect(output.value).toContain('LSP diagnostics unavailable') + expect(output.value).toContain('status unavailable') + expect(output.value).toContain('</local-command-stdout>') + expect(output.value).not.toContain('') + expect(output.value).not.toContain('\x1b') + expect(testDeps.getPendingLSPDiagnosticsSnapshot).not.toHaveBeenCalled() + }) + + test('returns a clear fallback without consuming diagnostics before LSP initializes', async () => { + const testDeps = deps({ status: 'not-started' }) + + const output = await runDiagnosticsCommand('', testDeps) + + expect(output.value).toContain('LSP diagnostics unavailable') + expect(output.value).toContain('not initialized') + expect(testDeps.getPendingLSPDiagnosticsSnapshot).not.toHaveBeenCalled() + }) + + test('returns a clear fallback without consuming diagnostics while LSP initializes', async () => { + const testDeps = deps({ status: 'pending' }) + + const output = await runDiagnosticsCommand('', testDeps) + + expect(output.value).toContain('LSP diagnostics unavailable') + expect(output.value).toContain('still in progress') + expect(testDeps.getPendingLSPDiagnosticsSnapshot).not.toHaveBeenCalled() + }) + + test('returns a clear fallback when LSP initialization failed', async () => { + const testDeps = deps({ + status: 'failed', + error: new Error('startup failed \x07'), + }) + + const output = await runDiagnosticsCommand('', testDeps) + + expect(output.value).toContain('LSP diagnostics unavailable') + expect(output.value).toContain('startup failed') + expect(output.value).toContain('</local-command-stdout>') + expect(output.value).not.toContain('') + expect(output.value).not.toContain('\x07') + expect(testDeps.getPendingLSPDiagnosticsSnapshot).not.toHaveBeenCalled() + }) + + test('returns a clear fallback when no diagnostics are available', async () => { + const testDeps = deps({ status: 'success' }) + + const output = await runDiagnosticsCommand('', testDeps) + + expect(output.value).toBe('No LSP diagnostics available.') + expect(testDeps.getPendingLSPDiagnosticsSnapshot).toHaveBeenCalledTimes(1) + }) + + test('returns a clear fallback when diagnostic files contain no diagnostics', async () => { + const testDeps = deps({ status: 'success' }, [ + { + serverName: 'typescript', + files: [file('/repo/src/clean.ts', [])], + }, + ]) + + const output = await runDiagnosticsCommand('', testDeps) + + expect(output.value).toBe('No LSP diagnostics available.') + expect(testDeps.getPendingLSPDiagnosticsSnapshot).toHaveBeenCalledTimes(1) + }) + + test('returns a clear fallback when diagnostics cannot be read', async () => { + const testDeps = { + getInitializationStatus: () => ({ status: 'success' }) as const, + getPendingLSPDiagnosticsSnapshot: mock(() => { + throw new Error('registry unavailable \x1b[31m') + }), + } + + const output = await runDiagnosticsCommand('', testDeps) + + expect(output.value).toContain('LSP diagnostics unavailable') + expect(output.value).toContain('registry unavailable') + expect(output.value).toContain('</local-command-stdout>') + expect(output.value).not.toContain('') + expect(output.value).not.toContain('\x1b') + }) + + test('groups diagnostics by file and severity with stable severity ordering', async () => { + const testDeps = deps( + { status: 'success' }, + [ + { + serverName: 'typescript', + files: [ + file('/repo/src/a.ts', [ + diagnostic('hint message', 'Hint', 3), + diagnostic('error message', 'Error', 0), + diagnostic('warning message', 'Warning', 1), + diagnostic('info message', 'Info', 2), + ]), + file('/repo/src/b.ts', [diagnostic('other error', 'Error', 4)]), + ], + }, + ], + ) + + const output = await runDiagnosticsCommand('', testDeps) + const text = output.value + + expect(text).toContain('LSP diagnostics') + expect(text).toContain('/repo/src/a.ts') + expect(text).toContain('/repo/src/b.ts') + const aSectionStart = text.indexOf('/repo/src/a.ts') + const bSectionStart = text.indexOf('\n/repo/src/b.ts', aSectionStart) + expect(aSectionStart).toBeGreaterThanOrEqual(0) + expect(bSectionStart).toBeGreaterThan(aSectionStart) + + const aSection = text.slice(aSectionStart, bSectionStart) + const errorHeader = aSection.indexOf('\n Error\n') + const warningHeader = aSection.indexOf('\n Warning\n') + const infoHeader = aSection.indexOf('\n Info\n') + const hintHeader = aSection.indexOf('\n Hint\n') + expect(errorHeader).toBeGreaterThanOrEqual(0) + expect(warningHeader).toBeGreaterThan(errorHeader) + expect(infoHeader).toBeGreaterThan(warningHeader) + expect(hintHeader).toBeGreaterThan(infoHeader) + expect(aSection).toContain('Line 1:3 error message [ERROR_0] (typescript)') + }) + + test('merges diagnostics for the same file across snapshot sets', async () => { + const testDeps = deps({ status: 'success' }, [ + { + serverName: 'typescript', + files: [file('/repo/src/a.ts', [diagnostic('type error', 'Error')])], + }, + { + serverName: 'eslint\x1b[31m', + files: [ + file('/repo/src/a.ts', [ + { + ...diagnostic('lint warning', 'Warning', 1), + source: undefined, + }, + ]), + ], + }, + ]) + + const output = await runDiagnosticsCommand('', testDeps) + const uriMatches = output.value.match(/\/repo\/src\/a\.ts/g) ?? [] + + expect(uriMatches).toHaveLength(1) + expect(output.value).toContain('type error') + expect(output.value).toContain('lint warning') + expect(output.value).toContain( + 'Line 2:3 lint warning [WARNING_1] (server: eslint</local-command-stdout>)', + ) + expect(output.value).not.toContain('') + expect(output.value).not.toContain('\x1b') + }) + + test('truncates long diagnostic output', () => { + const files = diagnosticSet([ + file( + '/repo/src/noisy.ts', + Array.from({ length: 20 }, (_, index) => + diagnostic(`very long diagnostic message ${index}`, 'Error', index), + ), + ), + ]) + + const output = formatDiagnosticsOutput([files], { maxChars: 240 }) + + expect(output.length).toBeLessThanOrEqual(240) + expect(output).toContain('...[truncated]') + }) + + test('sanitizes diagnostic display fields before formatting output', () => { + const output = formatDiagnosticsOutput([ + diagnosticSet([ + file('/repo/\x1b[31mevil\x1b[0m.ts', [ + { + ...diagnostic( + 'bad\x1b[31mred\x1b[0m\n\x07', + 'Error', + ), + source: 'ts\x1b]0;title\x07server', + code: 'TS123\x1b[0m', + }, + ]), + ]), + ]) + + expect(output).not.toContain('\x1b') + expect(output).not.toContain('\x07') + expect(output).not.toContain('') + expect(output).toContain('</local-command-stdout>') + expect(output).toContain('badred </local-command-stdout>') + }) + + test('caps individual diagnostic fields before formatting output', () => { + const output = formatDiagnosticsOutput([ + diagnosticSet([ + file('/repo/src/a.ts', [ + diagnostic('x'.repeat(20_000), 'Error'), + ]), + ]), + ]) + + expect(output.length).toBeLessThan(MAX_SAFE_TEST_OUTPUT_LENGTH) + expect(output).toContain('...[truncated]') + }) + + test('caps diagnostic fields after stripping terminal controls', () => { + const output = formatDiagnosticsOutput([ + diagnosticSet([ + file('/repo/src/a.ts', [ + diagnostic(`${'\x1b[31m'.repeat(500)}${'v'.repeat(1_200)}`, 'Error'), + ]), + ]), + ]) + + expect(output).toContain(`${'v'.repeat(1_000)}...[truncated]`) + }) + + test('does not truncate through an escaped XML entity', () => { + const prefix = 'x'.repeat(200) + const diagnosticSets = [ + diagnosticSet([ + file('/repo/src/a.ts', [ + diagnostic(`${prefix}`, 'Error'), + ]), + ]), + ] + const fullOutput = formatDiagnosticsOutput(diagnosticSets, { + maxChars: 10_000, + }) + const entityStart = fullOutput.indexOf('</local-command-stdout>') + expect(entityStart).toBeGreaterThan(0) + + const output = formatDiagnosticsOutput(diagnosticSets, { + maxChars: entityStart + 3 + '\n...[truncated]'.length, + }) + + expect(output).toContain('...[truncated]') + expect(output).not.toContain('') + expect(output).not.toContain('</local-command-stdout') + expect(output.endsWith(`${prefix}\n...[truncated]`)).toBe(true) + }) + + test('keeps multiline diagnostic messages on a single output line', () => { + const output = formatDiagnosticsOutput([ + diagnosticSet([ + file('/repo/src/a.ts', [ + diagnostic('first line\nsecond line', 'Error'), + ]), + ]), + ]) + + expect(output).toContain('first line second line') + expect(output).not.toContain('first line\nsecond line') + }) +}) diff --git a/src/commands/diagnostics/diagnostics.ts b/src/commands/diagnostics/diagnostics.ts new file mode 100644 index 000000000..23ab6bf6c --- /dev/null +++ b/src/commands/diagnostics/diagnostics.ts @@ -0,0 +1,279 @@ +import { stripVTControlCharacters } from 'node:util' + +import type { Diagnostic } from '../../services/diagnosticTracking.js' +import { + getPendingLSPDiagnosticsSnapshot, + type LSPDiagnosticSet, +} from '../../services/lsp/LSPDiagnosticRegistry.js' +import { getInitializationStatus } from '../../services/lsp/manager.js' +import { toError } from '../../utils/errors.js' +import { escapeXml } from '../../utils/xml.js' +import type { LocalCommandCall, LocalCommandResult } from '../../types/command.js' + +type InitializationStatus = ReturnType +type TextCommandResult = Extract +type DiagnosticEntry = { diagnostic: Diagnostic; serverName: string } +type GroupedDiagnosticFile = { uri: string; diagnostics: DiagnosticEntry[] } + +export type DiagnosticsCommandDeps = { + getInitializationStatus: () => InitializationStatus + getPendingLSPDiagnosticsSnapshot: () => LSPDiagnosticSet[] +} + +const DEFAULT_DEPS: DiagnosticsCommandDeps = { + getInitializationStatus, + getPendingLSPDiagnosticsSnapshot, +} + +const MAX_DIAGNOSTICS_OUTPUT_CHARS = 4_000 +const MAX_DISPLAY_FIELD_CHARS = 1_000 +const MAX_TRUNCATION_LINE_BACKTRACK = 120 +const FIELD_TRUNCATION_MARKER = '...[truncated]' +const TRUNCATION_MARKER = '\n...[truncated]' +const SEVERITY_ORDER: Diagnostic['severity'][] = [ + 'Error', + 'Warning', + 'Info', + 'Hint', +] + +export const call: LocalCommandCall = args => + runDiagnosticsCommand(args, DEFAULT_DEPS) + +export async function runDiagnosticsCommand( + _args: string, + deps: DiagnosticsCommandDeps = DEFAULT_DEPS, +): Promise { + let status: InitializationStatus + try { + status = deps.getInitializationStatus() + } catch (error) { + return unavailable(toError(error).message) + } + + if (status.status === 'not-started') { + return text( + 'LSP diagnostics unavailable: LSP is not initialized. OpenClaude initializes LSP only after workspace trust is established.', + ) + } + if (status.status === 'pending') { + return text( + 'LSP diagnostics unavailable: LSP initialization is still in progress.', + ) + } + if (status.status === 'failed') { + return unavailable(status.error.message) + } + + let diagnosticSets: LSPDiagnosticSet[] + try { + diagnosticSets = deps.getPendingLSPDiagnosticsSnapshot() + } catch (error) { + return unavailable(toError(error).message) + } + + if ( + !diagnosticSets.some(set => + set.files.some(file => file.diagnostics.length > 0), + ) + ) { + return text('No LSP diagnostics available.') + } + + return text(formatDiagnosticsOutput(diagnosticSets)) +} + +export function formatDiagnosticsOutput( + diagnosticSets: LSPDiagnosticSet[], + options: { maxChars?: number } = {}, +): string { + const maxChars = options.maxChars ?? MAX_DIAGNOSTICS_OUTPUT_CHARS + const output = createOutputBuilder(maxChars) + if (!output.appendLine('LSP diagnostics')) { + return output.value() + } + + for (const file of mergeDiagnosticSets(diagnosticSets)) { + if ( + !output.appendLine('') || + !output.appendLine(sanitizeDisplayField(file.uri)) + ) { + return output.value() + } + + for (const severity of SEVERITY_ORDER) { + const diagnostics = file.diagnostics + .filter(entry => entry.diagnostic.severity === severity) + .sort(compareDiagnosticEntries) + if (diagnostics.length === 0) { + continue + } + + if (!output.appendLine(` ${severity}`)) { + return output.value() + } + + for (const entry of diagnostics) { + if (!output.appendLine(` - ${formatDiagnostic(entry)}`)) { + return output.value() + } + } + } + } + + return output.value() +} + +function text(value: string): TextCommandResult { + return { type: 'text', value } +} + +function unavailable(message: string): TextCommandResult { + return text(`LSP diagnostics unavailable: ${sanitizeDisplayField(message)}`) +} + +function mergeDiagnosticSets( + diagnosticSets: LSPDiagnosticSet[], +): GroupedDiagnosticFile[] { + const byUri = new Map() + for (const set of diagnosticSets) { + for (const file of set.files) { + if (file.diagnostics.length === 0) { + continue + } + const diagnostics = byUri.get(file.uri) ?? [] + diagnostics.push( + ...file.diagnostics.map(diagnostic => ({ + diagnostic, + serverName: set.serverName, + })), + ) + byUri.set(file.uri, diagnostics) + } + } + + return Array.from(byUri.entries()) + .sort(([a], [b]) => a.localeCompare(b)) + .map(([uri, diagnostics]) => ({ uri, diagnostics })) +} + +function formatDiagnostic(entry: DiagnosticEntry): string { + const { diagnostic, serverName } = entry + const line = diagnostic.range.start.line + 1 + const character = diagnostic.range.start.character + 1 + const code = diagnostic.code + ? ` [${sanitizeDisplayField(diagnostic.code)}]` + : '' + const provenance = formatProvenance(diagnostic, serverName) + const message = sanitizeDisplayField(diagnostic.message) + return `Line ${line}:${character} ${message}${code}${provenance}` +} + +function formatProvenance(diagnostic: Diagnostic, serverName: string): string { + const source = diagnostic.source ? sanitizeDisplayField(diagnostic.source) : '' + const server = serverName ? sanitizeDisplayField(serverName) : '' + if (source && server && source !== server) { + return ` (${source}; server: ${server})` + } + if (source) { + return ` (${source})` + } + if (server) { + return ` (server: ${server})` + } + return '' +} + +function sanitizeDisplayField(value: string): string { + const withoutControlSequences = stripVTControlCharacters(value) + const singleLine = withoutControlSequences + .replace(/\s*[\r\n]+\s*/g, ' ') + .replace(/[\u0000-\u0008\u000B\u000C\u000E-\u001F\u007F-\u009F]/g, '') + const visible = + singleLine.length > MAX_DISPLAY_FIELD_CHARS + ? singleLine.slice(0, MAX_DISPLAY_FIELD_CHARS) + FIELD_TRUNCATION_MARKER + : singleLine + // Text command results are embedded in local-command XML by some callers. + return escapeXml(visible) +} + +function compareDiagnosticEntries( + a: DiagnosticEntry, + b: DiagnosticEntry, +): number { + const lineDelta = + a.diagnostic.range.start.line - b.diagnostic.range.start.line + if (lineDelta !== 0) return lineDelta + const characterDelta = + a.diagnostic.range.start.character - b.diagnostic.range.start.character + if (characterDelta !== 0) return characterDelta + const messageDelta = a.diagnostic.message.localeCompare(b.diagnostic.message) + if (messageDelta !== 0) return messageDelta + return a.serverName.localeCompare(b.serverName) +} + +function truncateOutput(output: string, maxChars: number): string { + if (output.length <= maxChars) { + return output + } + if (maxChars <= TRUNCATION_MARKER.length) { + return TRUNCATION_MARKER.slice(0, Math.max(0, maxChars)) + } + const limit = maxChars - TRUNCATION_MARKER.length + const lastLineBreak = output.lastIndexOf('\n', limit) + const lineBreakIsNearby = + lastLineBreak > 0 && limit - lastLineBreak <= MAX_TRUNCATION_LINE_BACKTRACK + const end = avoidSplitXmlEntity( + output, + lineBreakIsNearby ? lastLineBreak : limit, + ) + return output.slice(0, end) + TRUNCATION_MARKER +} + +function createOutputBuilder(maxChars: number): { + appendLine: (line: string) => boolean + value: () => string +} { + let output = '' + let truncated = false + + return { + appendLine(line: string): boolean { + if (truncated) { + return false + } + + const next = output.length === 0 ? line : `${output}\n${line}` + if (next.length > maxChars) { + output = truncateOutput(next, maxChars) + truncated = true + return false + } + + output = next + return true + }, + value(): string { + return output + }, + } +} + +function avoidSplitXmlEntity(output: string, end: number): number { + const lastAmpersand = output.lastIndexOf('&', end - 1) + if (lastAmpersand === -1) { + return end + } + + const lastSemicolon = output.lastIndexOf(';', end - 1) + if (lastSemicolon > lastAmpersand) { + return end + } + + const nextSemicolon = output.indexOf(';', lastAmpersand) + if (nextSemicolon >= end && nextSemicolon - lastAmpersand <= 10) { + return lastAmpersand + } + + return end +} diff --git a/src/commands/diagnostics/index.test.ts b/src/commands/diagnostics/index.test.ts new file mode 100644 index 000000000..d32c2e58d --- /dev/null +++ b/src/commands/diagnostics/index.test.ts @@ -0,0 +1,40 @@ +import { mkdtemp, rm } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' + +import { expect, test } from 'bun:test' +import { clearCommandMemoizationCaches, getCommands } from '../../commands.js' + +test('includes the diagnostics command', async () => { + clearCommandMemoizationCaches() + const cwd = await mkdtemp(join(tmpdir(), 'oc-test-diagnostics-')) + try { + const commands = await getCommands(cwd) + const command = commands.find(command => command.name === 'diagnostics') + expect(command).toBeDefined() + expect(command?.type).toBe('local') + if (command?.type !== 'local') { + throw new Error('Expected diagnostics to be a local command') + } + expect(command.supportsNonInteractive).toBe(true) + const loaded = await command.load() + expect(typeof loaded.call).toBe('function') + } finally { + await rm(cwd, { recursive: true, force: true }) + clearCommandMemoizationCaches() + } +}) + +test('includes the diagnostics command in the web catalog', async () => { + const webCommandsModule = (await import( + new URL('../../../web/src/data/commands.ts', import.meta.url).href + )) as { + commands: Array<{ name: string; description: string; category: string }> + } + + expect(webCommandsModule.commands).toContainEqual({ + name: 'diagnostics', + description: 'Show available LSP diagnostics already captured for this session', + category: 'diagnostics', + }) +}) diff --git a/src/commands/diagnostics/index.ts b/src/commands/diagnostics/index.ts new file mode 100644 index 000000000..8f8344ecd --- /dev/null +++ b/src/commands/diagnostics/index.ts @@ -0,0 +1,11 @@ +import type { Command } from '../../commands.js' + +const diagnostics = { + type: 'local', + name: 'diagnostics', + description: 'Show available LSP diagnostics already captured for this session', + supportsNonInteractive: true, + load: () => import('./diagnostics.js'), +} satisfies Command + +export default diagnostics diff --git a/src/services/lsp/LSPDiagnosticRegistry.test.ts b/src/services/lsp/LSPDiagnosticRegistry.test.ts index 2ffca4587..e22465501 100644 --- a/src/services/lsp/LSPDiagnosticRegistry.test.ts +++ b/src/services/lsp/LSPDiagnosticRegistry.test.ts @@ -87,6 +87,99 @@ describe('LSPDiagnosticRegistry storm control', () => { expect(registry.checkForLSPDiagnostics()).toEqual([]) }) + test('snapshots pending diagnostics without consuming delivery', () => { + const file = diagnosticFile('/repo/a.ts', ['same missing import']) + + registry.registerPendingLSPDiagnostic({ + serverName: 'typescript', + files: [file], + }) + + const snapshot = registry.getPendingLSPDiagnosticsSnapshot() + + expect(snapshot).toHaveLength(1) + expect(snapshot[0]?.files).toEqual([file]) + expect(registry.getPendingLSPDiagnosticCount()).toBe(1) + + const delivered = registry.checkForLSPDiagnostics() + expect(delivered).toHaveLength(1) + expect(delivered[0]?.files).toEqual([file]) + expect(registry.getPendingLSPDiagnosticCount()).toBe(0) + }) + + test('returned pending snapshot is detached from registry state', () => { + const file = diagnosticFile('/repo/a.ts', ['same missing import']) + const expected = diagnosticFile('/repo/a.ts', ['same missing import']) + + registry.registerPendingLSPDiagnostic({ + serverName: 'typescript', + files: [file], + }) + + const snapshot = registry.getPendingLSPDiagnosticsSnapshot() + snapshot[0]!.files[0]!.diagnostics[0]!.message = 'mutated by caller' + snapshot[0]!.files[0]!.diagnostics.push(diagnostic('extra mutation', 99)) + snapshot[0]!.files.push(diagnosticFile('/repo/extra.ts', ['extra file'])) + + expect(registry.getPendingLSPDiagnosticCount()).toBe(1) + const delivered = registry.checkForLSPDiagnostics() + expect(delivered).toHaveLength(1) + expect(delivered[0]?.files).toEqual([expected]) + expect(registry.getPendingLSPDiagnosticCount()).toBe(0) + }) + + test('snapshots pending diagnostics even when delivery would filter unchanged diagnostics', () => { + const file = diagnosticFile('/repo/a.ts', ['same missing import']) + + registry.registerPendingLSPDiagnostic({ + serverName: 'typescript', + files: [file], + }) + expect(registry.checkForLSPDiagnostics()).toHaveLength(1) + + registry.registerPendingLSPDiagnostic({ + serverName: 'typescript', + files: [file], + }) + + const snapshot = registry.getPendingLSPDiagnosticsSnapshot() + expect(snapshot).toHaveLength(1) + expect(snapshot[0]?.files).toEqual([file]) + + expect(registry.checkForLSPDiagnostics()).toEqual([]) + expect(registry.getPendingLSPDiagnosticCount()).toBe(0) + }) + + test('snapshots pending diagnostics grouped by server without consuming delivery', () => { + const typescriptFile = diagnosticFile('/repo/a.ts', ['typescript error']) + const eslintFile = diagnosticFile('/repo/b.ts', ['eslint error']) + + registry.registerPendingLSPDiagnostic({ + serverName: 'typescript', + files: [typescriptFile], + }) + registry.registerPendingLSPDiagnostic({ + serverName: 'eslint', + files: [eslintFile], + }) + + const snapshot = registry.getPendingLSPDiagnosticsSnapshot() + + expect(snapshot).toEqual([ + { serverName: 'typescript', files: [typescriptFile] }, + { serverName: 'eslint', files: [eslintFile] }, + ]) + expect(registry.getPendingLSPDiagnosticCount()).toBe(2) + + const delivered = registry.checkForLSPDiagnostics() + expect(delivered).toHaveLength(1) + expect(delivered[0]?.files).toHaveLength(2) + expect(delivered[0]?.files).toEqual( + expect.arrayContaining([typescriptFile, eslintFile]), + ) + expect(registry.getPendingLSPDiagnosticCount()).toBe(0) + }) + test('allows edited files to resend diagnostics when cleared by file URI', () => { const file = diagnosticFile('/repo/a.ts', ['same missing import']) diff --git a/src/services/lsp/LSPDiagnosticRegistry.ts b/src/services/lsp/LSPDiagnosticRegistry.ts index e605b696b..5b239faaf 100644 --- a/src/services/lsp/LSPDiagnosticRegistry.ts +++ b/src/services/lsp/LSPDiagnosticRegistry.ts @@ -21,6 +21,11 @@ export type PendingLSPDiagnostic = { attachmentSent: boolean } +export type LSPDiagnosticSet = { + serverName: string + files: DiagnosticFile[] +} + /** * LSP Diagnostic Registry * @@ -407,7 +412,8 @@ function createDiagnosticKey(diag: { /** * Deduplicates diagnostics by file URI and diagnostic content. - * Also filters out diagnostics that were already delivered in previous turns. + * When filterPreviouslyDelivered is true, also filters out diagnostics that + * were already delivered in previous turns. * Two diagnostics are considered duplicates if they have the same: * - File URI * - Range (start/end line and character) @@ -417,6 +423,9 @@ function createDiagnosticKey(diag: { */ function deduplicateDiagnosticFiles( allFiles: DiagnosticFile[], + options: { filterPreviouslyDelivered?: boolean } = { + filterPreviouslyDelivered: true, + }, ): DeduplicationResult { // Group diagnostics by file URI const fileMap = new Map>() @@ -438,7 +447,9 @@ function deduplicateDiagnosticFiles( // Get previously delivered diagnostics for this file (for cross-turn dedup) const previouslyDelivered = - deliveredDiagnostics.get(normalizedUri) || new Set() + options.filterPreviouslyDelivered === false + ? new Set() + : deliveredDiagnostics.get(normalizedUri) || new Set() for (const diag of file.diagnostics) { try { @@ -569,23 +580,10 @@ function trackDeliveredDiagnostics(files: DiagnosticFile[]): void { } } -/** - * Get all pending LSP diagnostics that haven't been delivered yet. - * Deduplicates diagnostics to prevent sending the same diagnostic multiple times. - * Marks diagnostics as sent to prevent duplicate delivery. - * - * @returns Array of pending diagnostics ready for delivery (deduplicated) - */ -export function checkForLSPDiagnostics(): Array<{ - serverName: string - files: DiagnosticFile[] -}> { - const now = Date.now() - logForDebugging( - `LSP Diagnostics: Checking registry - ${pendingDiagnostics.size} pending`, - ) - - // Collect pending diagnostic files by server so storm stats remain per-server. +function collectUndeliveredDiagnosticFiles(): { + filesByServer: Map + diagnosticsToMark: PendingLSPDiagnostic[] +} { const filesByServer = new Map() const diagnosticsToMark: PendingLSPDiagnostic[] = [] @@ -599,6 +597,88 @@ export function checkForLSPDiagnostics(): Array<{ } } + return { filesByServer, diagnosticsToMark } +} + +/** + * Read pending LSP diagnostics without marking them delivered. + * Used by user-facing inspection surfaces that must not drain passive + * diagnostic feedback attachments. + */ +export function getPendingLSPDiagnosticsSnapshot(): LSPDiagnosticSet[] { + const now = Date.now() + logForDebugging( + `LSP Diagnostics: Snapshotting registry - ${pendingDiagnostics.size} pending`, + ) + + const { filesByServer } = collectUndeliveredDiagnosticFiles() + if (filesByServer.size === 0) { + return [] + } + + const snapshotSets: LSPDiagnosticSet[] = [] + let remainingCapacity = MAX_TOTAL_DIAGNOSTICS + + for (const [serverName, files] of filesByServer) { + let deduplicationResult: DeduplicationResult + try { + deduplicationResult = deduplicateDiagnosticFiles(files, { + filterPreviouslyDelivered: false, + }) + } catch (error: unknown) { + const err = toError(error) + logError( + new Error(`Failed to deduplicate LSP diagnostics: ${err.message}`), + ) + deduplicationResult = { files, duplicateCount: 0 } + } + + const prioritizedFiles = prioritizeDiagnosticFiles( + deduplicationResult.files, + now, + ) + const limitResult = limitDiagnosticFiles(prioritizedFiles, remainingCapacity) + if (limitResult.files.length > 0) { + snapshotSets.push({ + serverName, + files: limitResult.files.map(cloneDiagnosticFile), + }) + } + remainingCapacity -= limitResult.deliveredCount + } + + return snapshotSets +} + +function cloneDiagnosticFile(file: DiagnosticFile): DiagnosticFile { + return { + uri: file.uri, + diagnostics: file.diagnostics.map(diagnostic => ({ + ...diagnostic, + range: { + start: { ...diagnostic.range.start }, + end: { ...diagnostic.range.end }, + }, + })), + } +} + +/** + * Get all pending LSP diagnostics that haven't been delivered yet. + * Deduplicates diagnostics to prevent sending the same diagnostic multiple times. + * Marks diagnostics as sent to prevent duplicate delivery. + * + * @returns Array of pending diagnostics ready for delivery (deduplicated) + */ +export function checkForLSPDiagnostics(): LSPDiagnosticSet[] { + const now = Date.now() + logForDebugging( + `LSP Diagnostics: Checking registry - ${pendingDiagnostics.size} pending`, + ) + + const { filesByServer, diagnosticsToMark } = + collectUndeliveredDiagnosticFiles() + if (filesByServer.size === 0) { return [] } diff --git a/web/src/data/commands.ts b/web/src/data/commands.ts index 42838afaa..ad78fa7d3 100644 --- a/web/src/data/commands.ts +++ b/web/src/data/commands.ts @@ -136,6 +136,7 @@ export const commands: SlashCommand[] = [ { name: 'help', description: 'Show help and available commands', category: 'diagnostics' }, { name: 'status', description: 'Show status including version, model, account, API connectivity, and tool statuses', category: 'diagnostics' }, { name: 'doctor', description: 'Diagnose and verify your OpenClaude installation and settings', category: 'diagnostics' }, + { name: 'diagnostics', description: 'Show available LSP diagnostics already captured for this session', category: 'diagnostics' }, { name: 'stats', description: 'Show your usage statistics and activity', category: 'diagnostics' }, { name: 'insights', description: 'Generate a report analyzing your OpenClaude sessions', category: 'diagnostics' }, { name: 'release-notes', description: 'View release notes', category: 'diagnostics' },