Push per-file diagnostics for clients without pull diagnostics support - #4297
Conversation
Clients that do not advertise the textDocument.diagnostic capability (e.g. eglot before Dec 2025, Nova, Claude Code) previously received no file diagnostics at all, since the server only supports pull diagnostics and only pushes project-level diagnostics to the tsconfig URI. When the client advertises textDocument.publishDiagnostics but not textDocument.diagnostic, push per-file diagnostics for open files: - didOpen publishes initial diagnostics for the opened file - didChange schedules a debounced snapshot update that rebuilds dirty programs and republishes diagnostics for affected open files - didClose clears diagnostics for the closed file - workspace/diagnostic/refresh triggers (watched files, config changes, ATA updates) schedule a snapshot update instead of a refresh request, since push-only clients cannot re-pull Clients that support pull diagnostics see no behavior change, and the existing disablePushDiagnostics initialization option also disables the new path.
757a9c2 to
a6efbe1
Compare
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds per-file push diagnostics for open documents to support clients that don’t implement pull diagnostics, wiring this through session snapshot updates and the LSP server initialization.
Changes:
- Introduces
PushFileDiagnosticsEnabledand new update reasons to trigger snapshot updates for open-file diagnostic pushes. - Publishes/clears
textDocument/publishDiagnosticsfor open/closed files after snapshot updates, and computes per-file diagnostics viaProvidePushDiagnostics. - Adds end-to-end tests validating publish behavior on open/change/close and version capability handling.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/project/snapshot.go | Logs new snapshot update reasons for improved observability. |
| internal/project/session.go | Adds a push-only diagnostics path: schedule updates, request open resources, and publish per-file diagnostics post-snapshot. |
| internal/project/project_test.go | Adds coverage for push diagnostics behavior across open/change/close and capability variants. |
| internal/lsp/server.go | Enables per-file push diagnostics based on client pull/push capability detection. |
| internal/ls/diagnostics.go | Adds ProvidePushDiagnostics to produce publishDiagnostics-formatted diagnostics. |
- Do not gate push-file diagnostics on the textDocument.publishDiagnostics capability; it is a baseline notification and the capability only advertises extensions - Gate the per-file push path on PushDiagnosticsEnabled as well, so the global option always wins - Skip publishing empty diagnostics for files that never had any, and skip the clear on close for files with nothing published
* origin/main: Make LS checkers time out after being idle, segment based on use (take 2) (microsoft#3435) Fix external helper crash after module-status updates (microsoft#3994) Allow LSP server to use builtin watcher on Windows and macOS (microsoft#4288) Rebuilt CLI watcher around new `fswatch` package (microsoft#4026) Fix stack overflow crash in base constraint resolution (microsoft#4242) # Conflicts: # internal/project/session.go
|
Jake Bailey (@jakebailey) could you take a look at this PR? Claude code doesn't support pull diagnostics: anthropics/claude-code#40282 and TypeScript Go doesn't support push diagnostics. |
|
We haven't had time to test it; we'd have to disable pull diags and test in VS Code and make sure it works. Push diagnostics get tricky when dealing with LS restarts and other racy ish conditions. |
Jake Bailey (jakebailey)
added
the
Unmigrated PR
|
Thank you for contributing to the TypeScript native port! Development has moved from this repository back to the main microsoft/TypeScript repository. GitHub does not have PR transfer functionality, so we're closing this PR here. If this change is still relevant, please reopen it as a new pull request in See microsoft/typescript-go#4918 for more information about the move. |
Problem
The LSP server only supports pull diagnostics (
textDocument/diagnostic); push diagnostics are only used for config file errors. Clients that do not implement pull diagnostics get no file diagnostics at all. This affects eglot before Dec 2025, Nova, Claude Code, and other lighter LSP clients (see the discussion in #2362).Change
When the client advertises
textDocument.publishDiagnosticsbut nottextDocument.diagnostic, the server now pushes per-file diagnostics for open files:didOpenpublishes initial diagnostics for the opened filedidChangeschedules a debounced (500ms) snapshot update that requests open documents, so dirty programs are rebuilt eagerly, and republishes diagnostics for open files whose program updateddidCloseclears diagnostics for the closed fileworkspace/diagnostic/refresh(watched file changes, config changes, ATA updates) schedule a snapshot update instead, since push-only clients cannot re-pullClients that support pull diagnostics are unaffected (verified: zero per-file pushes when
textDocument.diagnosticis declared), and the existingdisablePushDiagnosticsinitialization option also disables the new path. Diagnostics are converted with the existingDiagnosticToLSPPushpath and include the document version.Validation
internal/project/project_test.go(open/change/close/disabled)go test ./internal/project/... ./internal/ls/... ./internal/lsp/...passesdidOpenand ~500ms after eachdidChange, are cleared ondidClose, and pull requests keep working alongsideNotes
I am aware of the pre-7.0 scope guidance in CONTRIBUTING.md; opening this as a draft since it is a behavior addition. Happy to hold it until after 7.0, or adjust the approach (for example gating behind an initialization option instead of capability detection) if you would consider it earlier.
Disclosure per CONTRIBUTING.md: this patch was authored with AI assistance (Claude Code). I have read and understand the change and will respond to review feedback myself.