When gopls is running in headless mode (as for its MCP server), it needs a file watcher (i.e. fsnotify).
It's OK if this file watcher is not perfectly accurate.
Related: - #67995 - #67529 - #52284
I think we should finally take this on. The good thing about headless mode is that it is experimental and doesn't conflict with ordinary state changes from the client. However, based on what we learn, we can consider integrating the file watcher elsewhere.
For the initial implementation, we don't need to care about the complicated algebra of glob patterns that gopls computes (largely to work around client limitations). Let's just focus on watching *.{go,mod,sum,work}
. We can then produce synthetic didChangeWatchedFile
notifications (or equivalent).
This is liable to be nontrivial and deal with the gnarly bits of the gopls session construction. For whomever takes this on (if not me): let's discuss the plan before getting hands dirty.
CC @adonovan @madelinekalil @h9jiang
Comment From: gabyhelp
Related Issues
(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)
Comment From: h9jiang
Taking a step back from the headless mode and the idea of MCP server, what I'm thinking is, the file system watcher should be built based on the concept of LSP. Meaning, we setup file watch only for the workspace gopls is analyzing. (e.g. if I open x/tools workspace in vscode, the lsp session is all about x/tools, so there is no need to set up file watcher over any other directory)
The way LSP identify workspace is through either workspaceFolder or initialParam's rootUri
or rootPath
. The second approach is deprecated.
I did a local experiments with how VSCode is reporting didChangeWatchedFile
. If I change a x/tools code, the notification is sent over to gopls follow
[Trace - 1:09:22 PM] Sending notification 'workspace/didChangeWatchedFiles'.
Params: {
"changes": [
{
"uri": "file:///.../tools/gopls/internal/server/server.go",
"type": 2
},
{
"uri": "file:///.../tools/gopls/internal/server/server.go",
"type": 2
}
]
}
If I change standard library code "context.Context", no notification is being sent to gopls.
Maybe there is some missing piece about gopls I do not understand. My naive thought is:
- When receiving initialParam
, set up a file system watcher per workspaceFolder
URI.
- The file system watcher will listen to file events / dir events for *.{go,mod,sum,work}
.
- The file system watcher will convert the file events / dir events to LSP TextDoc Sync request.
Feel like those file watchers (as a group) are one LSP client that only send file {open, create, delete, change}
to the LSP server.
Things get more complicated if we want to support the proxy (forwarder) where either the forwarder or the daemon need to handle the listen and forward the request to the daemon gopls. (Seems to beyond the scope.)
Let me know what you think. My understanding of LSP is pretty naive. Please correct me if I'm wrong.
Also, I'm more than happy to work on this. :D
Comment From: findleyr
@h9jiang yes that's approximately right. Though we'd effectively convert it to the didChangeWatchedFiles
notification.
A couple comments: 1. At what level to we integrate the notification? If you try to trace a watched file change notification in gopls, you will see that it is astoundingly complicated (a chain of similar operations like didChangeWatchedFiles...didModifyFiles..didModifyFiles...clone at different levels of the stack). 2. The LSP protocol has server requesting file watching glob patterns, and clients implementing the file watching. For our headless use-case, this is OK, because there is no client, so there is nothing to conflict with. 3. How can we test? One use case would be to hook this up to the fake.Client of our integration tests.
Comment From: gopherbot
Change https://go.dev/cl/684696 mentions this issue: gopls/internal/filewatcher: introduce filewatcher using fsnotify
Comment From: gopherbot
Change https://go.dev/cl/686596 mentions this issue: gopls/internal/filewatcher: add stress test for file watcher
Comment From: gopherbot
Change https://go.dev/cl/686838 mentions this issue: gopls/internal/filewatcher: refactor filewatcher to pass in handler func
Comment From: gopherbot
Change https://go.dev/cl/687917 mentions this issue: gopls/internal/filewatcher: fix for windows deadlock
Comment From: gopherbot
Change https://go.dev/cl/688215 mentions this issue: gopls/internal/filewatcher: retry watch registration upon failure
Comment From: gopherbot
Change https://go.dev/cl/692155 mentions this issue: gopls/internal/filewatcher: rename handleEvent to convertEvent
Comment From: gopherbot
Change https://go.dev/cl/693657 mentions this issue: gopls/internal/filewatcher: backfill events after watching a directory