Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions docs/lsp.md
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,20 @@ touched only intermittently. Override them by editing the
`lsp.NewRouter(...).With...` chain in your build if you need a longer
warm pool or a tighter memory bound.

### Workspace roots

Each cached provider is keyed by `(spec, workspace)`, and the workspace
is what the language server is initialised against **and chdir'd into**.
It always comes from the tracked repo's own recorded path — never from
the directory the daemon process happened to be launched in. A daemon
tracking several repos is normally started from none of them, so a
workspace derived from its launch cwd would name a directory that exists
nowhere and fail the spawn.

The router therefore requires an absolute, existing directory and
rejects anything else before spawning. `gortex daemon --detach` can be
started from anywhere; no repo needs to be the daemon's cwd.

## Enrichment cost model

The resolution path (use 1 at the top of this page) runs as a batch
Expand Down Expand Up @@ -418,6 +432,12 @@ for repositories you trust.
boot but the subprocess failed to initialise (commonly a missing
dependency such as `node` for `pyright`, or a workspace-config
mismatch). The error surfaces the LSP server's stderr.
- **`workspace ... is not usable as a working directory` error:** the
caller supplied a workspace root that names no directory. The router
refuses it rather than spawning the server there, because that value
becomes the subprocess's working directory. A rejection is local to
the one workspace — the spec stays available for every other tracked
repo.
- **Server keeps restarting:** the idle reaper closed it, then the
next request re-spawned. Increase `WithIdleTimeout` if this hurts
warm-cache benchmarks.
Expand Down
92 changes: 77 additions & 15 deletions internal/mcp/tools_lsp.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,15 @@ import (
"context"
"errors"
"fmt"
"os"
"path/filepath"
"strings"
"sort"
"time"

"github.com/mark3labs/mcp-go/mcp"

"github.com/zzet/gortex/internal/graph"
"github.com/zzet/gortex/internal/pathkey"
"github.com/zzet/gortex/internal/semantic"
"github.com/zzet/gortex/internal/semantic/lsp"
)
Expand Down Expand Up @@ -46,7 +48,11 @@ func (s *Server) enrichNodeOnDemand(node *graph.Node) {
if err != nil || provider == nil {
return
}
_, _ = provider.EnrichNode(s.graph, s.workspaceRootFor(absPath), node)
root, err := s.workspaceRootFor(absPath)
if err != nil {
return
}
_, _ = provider.EnrichNode(s.graph, root, node)
}

// confirmSymbolRefsOnDemand faults in a callable symbol's INCOMING references
Expand Down Expand Up @@ -75,7 +81,11 @@ func (s *Server) confirmSymbolRefsOnDemand(node *graph.Node) {
if err != nil || provider == nil {
return
}
_, _ = provider.ConfirmSymbolRefs(s.graph, s.workspaceRootFor(absPath), node)
root, err := s.workspaceRootFor(absPath)
if err != nil {
return
}
_, _ = provider.ConfirmSymbolRefs(s.graph, root, node)
// Record after the attempt (even on 0/err) so a query doesn't re-spawn the
// server every call; a file re-index clears staleness through the normal
// restub path. Durable-ledger + retry semantics are the fuller version.
Expand Down Expand Up @@ -378,7 +388,10 @@ func (s *Server) lspProviderForPath(path string) (*lsp.Provider, string, error)
// in a multi-repo daemon, each rooted correctly. A
// shared (spec, defaultWorkspace) reuse would corrupt
// rootURI for files outside the first-spawned repo.
root := s.workspaceRootFor(abs)
root, err := s.workspaceRootFor(abs)
if err != nil {
return nil, abs, err
}
lp, err := r.ForSpecWorkspace(spec, root)
if err != nil {
return nil, abs, fmt.Errorf("router spawn %s: %w", spec.Name, err)
Expand All @@ -402,7 +415,10 @@ func (s *Server) lspProviderForPath(path string) (*lsp.Provider, string, error)
continue
}
// Lazy spawn — Provider.EnsureClient is idempotent.
root := s.workspaceRootFor(abs)
root, err := s.workspaceRootFor(abs)
if err != nil {
return nil, abs, err
}
if err := lp.EnsureClient(root); err != nil {
return nil, abs, fmt.Errorf("ensure client %s: %w", spec.Name, err)
}
Expand Down Expand Up @@ -455,30 +471,76 @@ func (s *Server) absolutePath(path string) (string, error) {
return filepath.Join(root, path), nil
}
}
// A repo-relative fragment the prefix join could not claim (an
// unprefixed graph path in a multi-repo daemon) still belongs to one of
// the tracked repos. Anchor it there before considering filepath.Abs,
// which would join it onto the daemon's own launch directory and mint a
// path that exists nowhere.
if abs := s.anchorOnTrackedRepo(path); abs != "" {
return abs, nil
}
if abs, err := filepath.Abs(path); err == nil {
return abs, nil
}
return path, nil
}

// workspaceRootFor returns the workspace root the LSP server should
// be initialised with for the given file. Falls back to the file's
// directory when no indexer root is available.
func (s *Server) workspaceRootFor(absPath string) string {
// anchorOnTrackedRepo joins a repo-relative path onto each tracked repo
// root and returns the first candidate that exists on disk. Roots are
// walked in sorted-prefix order so the answer stays deterministic when the
// same relative path resolves under more than one tracked repo.
func (s *Server) anchorOnTrackedRepo(rel string) string {
roots := s.collectRepoRoots("")
if len(roots) == 0 {
return ""
}
prefixes := make([]string, 0, len(roots))
for prefix := range roots {
prefixes = append(prefixes, prefix)
}
sort.Strings(prefixes)
for _, prefix := range prefixes {
candidate := filepath.Join(roots[prefix], rel)
if _, err := os.Stat(candidate); err == nil {
return candidate
}
}
return ""
}

// workspaceRootFor returns the workspace root the LSP server should be
// initialised with — and chdir'd into — for the given file: the tracked
// repo that contains it.
//
// It never derives a root from the daemon process's own working
// directory. A daemon tracking several repos is normally launched from
// none of them, and handing a language server a directory synthesised
// from that cwd fails the spawn with an opaque chdir error, silently
// dropping that language's enrichment for the rest of the session.
func (s *Server) workspaceRootFor(absPath string) (string, error) {
if s.indexer != nil {
root := s.indexer.RootPath()
if root != "" && strings.HasPrefix(absPath, root) {
return root
// Component-boundary containment: a plain string prefix would let
// the root /src/repo claim a file under /src/repo-old.
if root := s.indexer.RootPath(); root != "" && pathkey.HasPathPrefix(absPath, root) {
return root, nil
}
}
if s.multiIndexer != nil {
if prefix := s.multiIndexer.RepoForFile(absPath); prefix != "" {
if meta := s.multiIndexer.GetMetadata(prefix); meta != nil {
return meta.RootPath
if meta := s.multiIndexer.GetMetadata(prefix); meta != nil && meta.RootPath != "" {
return meta.RootPath, nil
}
}
}
return filepath.Dir(absPath)
// Last resort — the file's own directory, and only when it really
// exists. An unindexed-but-real file (a scratch file an editor opened)
// is a legitimate workspace; a directory that exists nowhere is the
// mis-anchored-path bug, and must not reach a spawn.
dir := filepath.Dir(absPath)
if info, err := os.Stat(dir); err == nil && info.IsDir() {
return dir, nil
}
return "", fmt.Errorf("no tracked repo contains %s: refusing to derive an LSP workspace from the daemon's working directory", absPath)
}

// lspNoProviderResult returns a structured error for the "no LSP
Expand Down
107 changes: 107 additions & 0 deletions internal/mcp/tools_lsp_workspace_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
package mcp

// Regression cover for issue #297: in a daemon tracking several repos, the
// LSP workspace (which becomes the language server's working directory) was
// derived from the daemon process's own launch cwd rather than from the
// tracked repo's path, producing a directory that exists nowhere and an
// opaque `chdir ...` spawn failure.

import (
"os"
"path/filepath"
"testing"

"github.com/stretchr/testify/require"
)

// TestWorkspaceRootFor_UsesTrackedRepoRoot — a file inside a tracked repo
// must resolve to that repo's recorded root, whatever directory the process
// is running from.
func TestWorkspaceRootFor_UsesTrackedRepoRoot(t *testing.T) {
fx := newSharedWorkspaceServer(t, false)

for _, repo := range []string{fx.repoA, fx.repoB} {
root, err := fx.srv.workspaceRootFor(filepath.Join(repo, "main.go"))
require.NoError(t, err)
require.Equal(t, repo, root, "workspace must be the tracked repo root")
}
}

// TestWorkspaceRootFor_RejectsPathUnderNoTrackedRepo — the reported failure
// shape. A path synthesised by joining a repo-relative fragment onto the
// daemon's launch cwd belongs to no tracked repo and names no real
// directory; it must be refused rather than handed to a spawn.
func TestWorkspaceRootFor_RejectsPathUnderNoTrackedRepo(t *testing.T) {
fx := newSharedWorkspaceServer(t, false)

phantom := filepath.Join(t.TempDir(), "daemon-launch-cwd", "SomeProject", "SomeFolder", "Thing.cs")
_, err := fx.srv.workspaceRootFor(phantom)
require.Error(t, err, "a workspace under no tracked repo must not be invented")
require.Contains(t, err.Error(), phantom)
}

// TestWorkspaceRootFor_AllowsRealUntrackedDirectory — an existing directory
// outside every tracked repo (a scratch file an editor opened) is still a
// legitimate workspace. Only phantom directories are refused.
func TestWorkspaceRootFor_AllowsRealUntrackedDirectory(t *testing.T) {
fx := newSharedWorkspaceServer(t, false)

dir := t.TempDir()
require.NoError(t, os.WriteFile(filepath.Join(dir, "scratch.go"), []byte("package x\n"), 0o644))

root, err := fx.srv.workspaceRootFor(filepath.Join(dir, "scratch.go"))
require.NoError(t, err)
require.Equal(t, dir, root)
}

// TestWorkspaceRootFor_RespectsPathComponentBoundaries — the root /src/repo
// must not claim a file under a sibling /src/repo-old. A plain string-prefix
// containment check used to.
func TestWorkspaceRootFor_RespectsPathComponentBoundaries(t *testing.T) {
fx := newSharedWorkspaceServer(t, false)

sibling := fx.repoB + "-old"
require.NoError(t, os.MkdirAll(sibling, 0o755))
require.NoError(t, os.WriteFile(filepath.Join(sibling, "main.go"), []byte("package main\n"), 0o644))

root, err := fx.srv.workspaceRootFor(filepath.Join(sibling, "main.go"))
require.NoError(t, err)
require.Equal(t, sibling, root, "a name-prefix sibling must not be folded into the tracked repo")
}

// TestAbsolutePath_AnchorsUnprefixedPathOnTrackedRepo — a repo-relative
// fragment that carries no repo prefix must be anchored on a tracked repo,
// not joined onto the daemon's own working directory.
func TestAbsolutePath_AnchorsUnprefixedPathOnTrackedRepo(t *testing.T) {
fx := newSharedWorkspaceServer(t, false)

// A file that exists in exactly one tracked repo, under a nested path —
// the shape the bug report redacted as "SomeProject\SomeFolder".
nested := filepath.Join(fx.repoB, "SomeProject", "SomeFolder")
require.NoError(t, os.MkdirAll(nested, 0o755))
require.NoError(t, os.WriteFile(filepath.Join(nested, "Thing.cs"), []byte("class Thing {}\n"), 0o644))

got, err := fx.srv.absolutePath(filepath.Join("SomeProject", "SomeFolder", "Thing.cs"))
require.NoError(t, err)
require.Equal(t, filepath.Join(nested, "Thing.cs"), got)

// And the workspace derived from it is repoB's root — the whole point.
root, err := fx.srv.workspaceRootFor(got)
require.NoError(t, err)
require.Equal(t, fx.repoB, root)
}

// TestAbsolutePath_UnresolvableStaysOutOfTrackedRepos — a fragment that
// matches nothing on disk still yields something absolute for error
// reporting, but it must not be presented as belonging to a tracked repo.
func TestAbsolutePath_UnresolvableStaysOutOfTrackedRepos(t *testing.T) {
fx := newSharedWorkspaceServer(t, false)

got, err := fx.srv.absolutePath(filepath.Join("NoSuchProject", "NoSuchFolder", "Nope.cs"))
require.NoError(t, err)
require.True(t, filepath.IsAbs(got))

// The downstream workspace derivation is what must refuse it.
_, werr := fx.srv.workspaceRootFor(got)
require.Error(t, werr)
}
7 changes: 7 additions & 0 deletions internal/semantic/lsp/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -2726,6 +2726,13 @@ func (p *Provider) Client() *Client { return p.client }
// LSP server (idempotent) so callers that want diagnostics or code
// actions outside an Enrich pass can prime the connection on demand.
func (p *Provider) EnsureClient(workspaceRoot string) error {
// filepath.Abs("") is the process's working directory. For a daemon
// tracking many repos that is a directory belonging to none of them, so
// an omitted root must be an error rather than a server rooted wherever
// the daemon was launched.
if workspaceRoot == "" {
return fmt.Errorf("lsp %s: no workspace root supplied; pass the tracked repo's own path", p.command)
}
abs, err := filepath.Abs(workspaceRoot)
if err != nil {
return err
Expand Down
Loading
Loading