From 749ce833a75f049fdb1ebce17ad15c649816e8e0 Mon Sep 17 00:00:00 2001 From: Jordan Coin Jackson Date: Sun, 11 Oct 2026 21:49:17 +0200 Subject: [PATCH 1/2] fix(mcp): expand ~ with os.UserHomeDir; run tests on Windows in CI Four sites joined os.Getenv("HOME") to expand "~/". HOME is normally unset on Windows, so "~/proj" became "proj" and filepath.Abs resolved it against the server's cwd: either a spurious error or, worse, a confident analysis of the wrong project. One helper, expandHome, now resolves "~", "~/" and "~\" through os.UserHomeDir and returns an error naming the path when no home directory is known. Tests set HOME and USERPROFILE together (setHome) so they prove the same behaviour on every OS; the old HOME-only tests passed on Windows by supplying the variable Windows would not have. CI gains one Windows test job (newest Go) so a whole platform is no longer asserted by a build. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01NRaBgKRAZMX9hjbkkefA7T --- .github/workflows/ci.yml | 7 +++++++ mcp/main.go | 33 ++++++++++++++++++++++----------- mcp/main_test.go | 34 +++++++++++++++++++++++++++++++++- mcp/surface_hygiene_test.go | 4 ++-- mcp/topology.go | 7 +++---- 5 files changed, 67 insertions(+), 18 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2efdffa..1489c3b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -15,6 +15,11 @@ jobs: matrix: os: [ubuntu-latest, macos-latest] go-version: ['1.24', '1.25', '1.26'] + include: + # One Windows test job: before #139 Windows only built, never ran + # tests, so a whole platform's correctness was asserted by a build. + - os: windows-latest + go-version: '1.26' steps: - uses: actions/checkout@v4 @@ -30,6 +35,8 @@ jobs: pip install ast-grep-cli elif [ "$RUNNER_OS" = "macOS" ]; then brew install ast-grep + elif [ "$RUNNER_OS" = "Windows" ]; then + pip install ast-grep-cli fi shell: bash diff --git a/mcp/main.go b/mcp/main.go index 7d873a4..ad8859c 100644 --- a/mcp/main.go +++ b/mcp/main.go @@ -399,9 +399,27 @@ func mustSchemaFor[T any]() *jsonschema.Schema { return schema } +// expandHome resolves a leading "~" against the user's home directory. +// os.UserHomeDir reads USERPROFILE on Windows, where HOME is normally unset; +// joining os.Getenv("HOME") there produced a relative path that filepath.Abs +// resolved against the server's cwd, so "~/proj" could silently analyse the +// wrong project. When no home is known the caller gets an error naming the +// path instead of a guess. +func expandHome(path string) (string, error) { + if path != "~" && !strings.HasPrefix(path, "~/") && !strings.HasPrefix(path, `~\`) { + return path, nil + } + home, err := os.UserHomeDir() + if err != nil { + return "", fmt.Errorf("cannot expand %q: %w", path, err) + } + return filepath.Join(home, strings.TrimLeft(path[1:], `/\`)), nil +} + func validateProjectPath(path string) (string, *mcp.CallToolResult) { - if strings.HasPrefix(path, "~/") { - path = filepath.Join(os.Getenv("HOME"), path[2:]) + path, err := expandHome(path) + if err != nil { + return "", errorResult("Invalid path: " + err.Error()) } absPath, err := filepath.Abs(path) if err != nil { @@ -642,7 +660,7 @@ type EmptyInput struct{} func handleStatus(ctx context.Context, req *mcp.CallToolRequest, input EmptyInput) (*mcp.CallToolResult, any, error) { cwd, _ := os.Getwd() - home := os.Getenv("HOME") + home, _ := os.UserHomeDir() // Check active watchers watchersMu.RLock() @@ -693,14 +711,7 @@ func handleListProjects(ctx context.Context, req *mcp.CallToolRequest, input Lis if cancelled := cancellationResult(ctx, "Project discovery"); cancelled != nil { return cancelled, nil, nil } - // Expand ~ to home directory - path := input.Path - if strings.HasPrefix(path, "~/") { - home := os.Getenv("HOME") - path = filepath.Join(home, path[2:]) - } - - absPath, invalid := validateProjectPath(path) + absPath, invalid := validateProjectPath(input.Path) if invalid != nil { return invalid, nil, nil } diff --git a/mcp/main_test.go b/mcp/main_test.go index f696555..d13317c 100644 --- a/mcp/main_test.go +++ b/mcp/main_test.go @@ -579,9 +579,41 @@ func resultText(t *testing.T, res *mcp.CallToolResult) string { return text.Text } +// setHome points os.UserHomeDir at dir on every OS: HOME on Unix, USERPROFILE +// on Windows. Tests that set only HOME passed on Windows by supplying the +// variable Windows would not have, which is how #139 stayed invisible. +func setHome(t *testing.T, dir string) { + t.Helper() + t.Setenv("HOME", dir) + t.Setenv("USERPROFILE", dir) +} + +func TestExpandHome(t *testing.T) { + home := t.TempDir() + setHome(t, home) + for _, in := range []string{"~/x", `~\x`} { + got, err := expandHome(in) + if err != nil || got != filepath.Join(home, "x") { + t.Fatalf("expandHome(%q) = %q, %v; want %q", in, got, err, filepath.Join(home, "x")) + } + } + if got, err := expandHome("~"); err != nil || got != home { + t.Fatalf("expandHome(~) = %q, %v; want %q", got, err, home) + } + if got, err := expandHome("plain/x"); err != nil || got != "plain/x" { + t.Fatalf("expandHome(plain/x) = %q, %v; want unchanged", got, err) + } + // No home at all: an error naming the path, never a cwd-relative guess. + t.Setenv("HOME", "") + t.Setenv("USERPROFILE", "") + if got, err := expandHome("~/x"); err == nil || !strings.Contains(err.Error(), "~/x") { + t.Fatalf("expandHome without home = %q, %v; want error naming the path", got, err) + } +} + func TestValidateProjectPathExpandsHomeAndRejectsInvalidRoots(t *testing.T) { home := t.TempDir() - t.Setenv("HOME", home) + setHome(t, home) proj := filepath.Join(home, "proj") if err := os.MkdirAll(filepath.Join(proj, ".git"), 0o755); err != nil { t.Fatal(err) diff --git a/mcp/surface_hygiene_test.go b/mcp/surface_hygiene_test.go index f4ab382..a49dd15 100644 --- a/mcp/surface_hygiene_test.go +++ b/mcp/surface_hygiene_test.go @@ -262,7 +262,7 @@ func TestFileContextUsesNativeKeysAndKeepsProjectRootsRequestLocal(t *testing.T) func TestFindFileResolvesHomeAndKeepsConfigRootsRequestLocal(t *testing.T) { home := t.TempDir() - t.Setenv("HOME", home) + setHome(t, home) alpha := filepath.Join(home, "alpha") beta := filepath.Join(home, "beta") writeFindProject(t, alpha, `{"only":["go"]}`) @@ -471,7 +471,7 @@ func TestValidateProjectPathRejectsInaccessiblePath(t *testing.T) { t.Fatalf("unexpected rejection: %q", resultText(t, result)) } home := t.TempDir() - t.Setenv("HOME", home) + setHome(t, home) if err := os.MkdirAll(filepath.Join(home, "proj"), 0o755); err != nil { t.Fatal(err) } diff --git a/mcp/topology.go b/mcp/topology.go index d5dfefe..6ec24a1 100644 --- a/mcp/topology.go +++ b/mcp/topology.go @@ -3,10 +3,8 @@ package codemapmcp import ( "context" "encoding/json" - "os" "path/filepath" "sort" - "strings" "codemap/internal/projectpath" "codemap/limits" @@ -145,8 +143,9 @@ func handleGetModuleContext(ctx context.Context, req *mcp.CallToolRequest, input } func topologyProjectRoot(path string) (string, error) { - if strings.HasPrefix(path, "~/") { - path = filepath.Join(os.Getenv("HOME"), path[2:]) + path, err := expandHome(path) + if err != nil { + return "", err } selection, err := projectpath.Select(path) if err != nil { From 7a9d233724297605a2c9c2bdca9050a1758e97e8 Mon Sep 17 00:00:00 2001 From: Jordan Coin Jackson Date: Sun, 11 Oct 2026 22:05:48 +0200 Subject: [PATCH 2/2] ci: make the Windows test job informational until #214 is empty Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01NRaBgKRAZMX9hjbkkefA7T --- .github/workflows/ci.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1489c3b..ec391fc 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -10,6 +10,7 @@ jobs: test: name: Test runs-on: ${{ matrix.os }} + continue-on-error: ${{ matrix.experimental == true }} strategy: fail-fast: false matrix: @@ -18,8 +19,11 @@ jobs: include: # One Windows test job: before #139 Windows only built, never ran # tests, so a whole platform's correctness was asserted by a build. + # Informational until #214 (124 pre-existing Windows failures, + # grouped by cause there) is empty; it still reports on every PR. - os: windows-latest go-version: '1.26' + experimental: true steps: - uses: actions/checkout@v4