fix(lsp): root language servers at the tracked repo, not the daemon cwd - #382
Merged
Conversation
A daemon tracking several repos is normally launched from none of them. Three paths let that launch directory leak into an LSP workspace, which becomes the language server subprocess's working directory: - Server.absolutePath fell back to filepath.Abs for a repo-relative graph path the prefix join could not claim, joining the fragment onto the daemon's cwd and minting a path that exists nowhere. - Server.workspaceRootFor then returned filepath.Dir of that phantom path, and matched the indexer root with a plain string prefix, so /src/repo could claim a file under /src/repo-old. - The router's default workspace was seeded from os.Getwd() whenever cfg.Index was empty — always, in daemon mode. The spawn failed with a bare `chdir <phantom>: no such file or directory`, and markSpawnFailed then disabled that spec for every other tracked repo for the rest of the session, so one mis-anchored query silently dropped a whole language's enrichment daemon-wide. absolutePath now anchors an unclaimed relative path on a tracked repo root, workspaceRootFor resolves the containing tracked repo on component boundaries and refuses to invent a workspace from a nonexistent directory, and the daemon's router default stays empty so a caller that omits the per-repo root fails loudly. The router validates the workspace before spawning, keeping the failure local to the one bad workspace instead of poisoning the spec. Fixes #297
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #297.
Validation
Reproduced and confirmed. With the fix reverted, the new test resolves a repo-relative fragment to
<process cwd>/SomeProject/SomeFolder/Thing.cs— exactly the shape reported (C:\Users\someuser\SomeProject\SomeFolder). The reporter's inference was right: a repo-relative path fragment gets joined onto the daemon's launch directory instead of the tracked repo's root.Root cause
Three paths let the daemon's launch cwd leak into an LSP workspace, and that workspace becomes the language server subprocess's working directory (
SpawnTransport.Startsetscmd.Dir).Server.absolutePath— for a repo-relative graph path the multi-indexer's prefix join couldn't claim, it fell through tofilepath.Abs, which joins onto the process cwd.Server.workspaceRootFor— returnedfilepath.Dirof that phantom path unconditionally. It also matched the indexer root with a plainstrings.HasPrefix, so/src/repocould claim a file under/src/repo-old.serverstack.NewSharedServer— seeded the router's default workspace fromos.Getwd()whenevercfg.Indexwas empty, which is always in daemon mode. (filepath.Abs("")returns the cwd too, soNewRouter("")had the same problem.)The spawn then failed with a bare
chdir <phantom>: no such file or directory.markSpawnFailedtreats that as "this binary can't launch" and disables the spec for every tracked repo for the rest of the session — which is why one mis-anchored query silently dropped C# enrichment daemon-wide, and why restarting the daemon inside the repo appeared to fix it.Fix
absolutePathanchors an unclaimed relative path on a tracked repo root (stat-checked, sorted-prefix order for determinism) before consideringfilepath.Abs.workspaceRootForreturns(string, error): it resolves the containing tracked repo on path-component boundaries (pathkey.HasPathPrefix), and refuses to invent a workspace from a directory that doesn't exist. An existing untracked directory (a scratch file an editor opened) is still accepted.absWorkspacepreserves the empty string through every absolutisation site.Router.forSpecWorkspacevalidates the workspace before spawning — non-empty, absolute, an existing directory — so the failure is diagnosable and, critically, never reachesmarkSpawnFailed. A bad workspace stays one repo's problem.Provider.EnsureClientrejects an empty workspace root for the same reason.A daemon can now be started from anywhere; no repo needs to be its cwd.
Tests
internal/semantic/lsp/router_workspace_test.goandinternal/mcp/tools_lsp_workspace_test.go— 13 new cases covering the tracked-root resolution, the phantom-path refusal, component-boundary containment, the empty/relative workspace guards, and the spec-not-poisoned invariant. The behavioural ones were confirmed to fail against the pre-fix code.Verification
go build ./...,go vet ./...,golangci-lint runon the touched packages, andgo test -raceoninternal/mcp,internal/semantic/...,internal/serverstack,internal/indexer— all clean.Not addressed
markSpawnFailedstill disables a spec daemon-wide for a genuine launch failure (missing runtime dependency, broken toolchain shim). That is documented as intentional, and the validation gate now keeps workspace-shaped failures out of it. Narrowing it further is a separate call.