From 254081462c21d686b5fe612e5032c2718a12f6b8 Mon Sep 17 00:00:00 2001 From: Cesc Date: Fri, 21 Aug 2026 10:57:36 +0700 Subject: [PATCH] fix(memory): apply Docker localhost rewrite in embedding provider constructor Model listing and chat provider construction rewrite localhost to host.docker.internal via config.DockerLocalhost() when running inside Docker, but all three memory.NewOpenAIEmbeddingProvider call sites (verify-embedding handler, runtime memory embedding) passed the raw api_base through. In Docker the container resolved localhost to itself, so Ollama Verify Embedding and runtime embedding failed with connection refused while the model dropdown loaded fine. Centralize the rewrite inside the constructor so no call site can miss it, add a SetInDockerForTest hook for environment-independent regression tests, and pin Docker detection off in httptest-based suites. Fixes #1519 --- cmd/gateway_agents_test.go | 5 +++ internal/config/runtime.go | 17 ++++++++++ internal/config/runtime_test.go | 36 +++++++++++++++++++++ internal/memory/embeddings.go | 6 ++++ internal/memory/embeddings_provider_test.go | 32 ++++++++++++++++++ 5 files changed, 96 insertions(+) create mode 100644 internal/config/runtime_test.go diff --git a/cmd/gateway_agents_test.go b/cmd/gateway_agents_test.go index a72a3d9b..993cd715 100644 --- a/cmd/gateway_agents_test.go +++ b/cmd/gateway_agents_test.go @@ -9,6 +9,7 @@ import ( "github.com/google/uuid" + "github.com/nextlevelbuilder/goclaw/internal/config" "github.com/nextlevelbuilder/goclaw/internal/store" "github.com/nextlevelbuilder/goclaw/internal/tools" ) @@ -92,6 +93,10 @@ func TestSubagentExecTool_NilStoreIsSafe(t *testing.T) { func captureEmbeddingRequest(t *testing.T, es *store.EmbeddingSettings) map[string]any { t.Helper() + // Pin Docker detection off so the loopback httptest URL is not rewritten + // to host.docker.internal when this suite runs inside a container. + t.Cleanup(config.SetInDockerForTest(false)) + var requestBody map[string]any server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if err := json.NewDecoder(r.Body).Decode(&requestBody); err != nil { diff --git a/internal/config/runtime.go b/internal/config/runtime.go index 75e94b32..4b315f35 100644 --- a/internal/config/runtime.go +++ b/internal/config/runtime.go @@ -4,16 +4,33 @@ import ( "os" "strings" "sync" + "sync/atomic" ) var ( dockerOnce sync.Once dockerCached bool + + // inDockerForTest overrides Docker detection. Production code MUST never + // set this; use SetInDockerForTest exclusively from *_test.go. + inDockerForTest atomic.Pointer[bool] ) +// SetInDockerForTest forces InDocker to return v, bypassing the /.dockerenv +// probe. Returns a restore func that reinstates the previous override (or real +// detection when none was active). Test-only. +func SetInDockerForTest(v bool) func() { + prev := inDockerForTest.Load() + inDockerForTest.Store(&v) + return func() { inDockerForTest.Store(prev) } +} + // InDocker returns true when running inside a Docker container. // Result is cached after the first call. func InDocker() bool { + if override := inDockerForTest.Load(); override != nil { + return *override + } dockerOnce.Do(func() { _, err := os.Stat("/.dockerenv") dockerCached = err == nil diff --git a/internal/config/runtime_test.go b/internal/config/runtime_test.go new file mode 100644 index 00000000..7ad0b23e --- /dev/null +++ b/internal/config/runtime_test.go @@ -0,0 +1,36 @@ +package config + +import "testing" + +func TestDockerLocalhostRewritesLoopbackInDocker(t *testing.T) { + restore := SetInDockerForTest(true) + defer restore() + + cases := []struct { + name string + in string + want string + }{ + {"localhost", "http://localhost:11434/v1", "http://host.docker.internal:11434/v1"}, + {"loopback ip", "http://127.0.0.1:11434/v1", "http://host.docker.internal:11434/v1"}, + {"non-loopback unchanged", "https://api.openai.com/v1", "https://api.openai.com/v1"}, + {"empty", "", ""}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := DockerLocalhost(tc.in); got != tc.want { + t.Fatalf("DockerLocalhost(%q) = %q, want %q", tc.in, got, tc.want) + } + }) + } +} + +func TestDockerLocalhostPassthroughOutsideDocker(t *testing.T) { + restore := SetInDockerForTest(false) + defer restore() + + in := "http://localhost:11434/v1" + if got := DockerLocalhost(in); got != in { + t.Fatalf("DockerLocalhost(%q) = %q, want unchanged outside Docker", in, got) + } +} diff --git a/internal/memory/embeddings.go b/internal/memory/embeddings.go index 362873e1..ee3ea087 100644 --- a/internal/memory/embeddings.go +++ b/internal/memory/embeddings.go @@ -11,6 +11,8 @@ import ( "net/http" "strings" "time" + + "github.com/nextlevelbuilder/goclaw/internal/config" ) // ContentHash returns a short SHA256 hex digest of the content (first 16 bytes). @@ -156,6 +158,10 @@ func NewOpenAIEmbeddingProvider(name, apiKey, apiURL, model string) *OpenAIEmbed if model == "" { model = "text-embedding-3-small" } + // Rewrite loopback hosts to host.docker.internal when running inside Docker, + // centralized here so no call site (verify handler, runtime memory embedding) + // can reach the container itself instead of a host service like Ollama. + apiURL = config.DockerLocalhost(apiURL) return &OpenAIEmbeddingProvider{ name: name, diff --git a/internal/memory/embeddings_provider_test.go b/internal/memory/embeddings_provider_test.go index 3b991b30..4efe6f98 100644 --- a/internal/memory/embeddings_provider_test.go +++ b/internal/memory/embeddings_provider_test.go @@ -5,8 +5,11 @@ import ( "encoding/json" "net/http" "net/http/httptest" + "os" "testing" "time" + + "github.com/nextlevelbuilder/goclaw/internal/config" ) func TestOpenAIEmbeddingProviderRestoresResponseOrder(t *testing.T) { @@ -50,3 +53,32 @@ func TestOpenAIEmbeddingProviderUsesBoundedHTTPClient(t *testing.T) { t.Fatalf("HTTP timeout = %v, want 60s", provider.httpClient) } } + +func TestOpenAIEmbeddingProviderRewritesLoopbackInDocker(t *testing.T) { + restore := config.SetInDockerForTest(true) + defer restore() + + provider := NewOpenAIEmbeddingProvider("ollama", "", "http://localhost:11434/v1", "nomic-embed-text") + if provider.apiURL != "http://host.docker.internal:11434/v1" { + t.Fatalf("apiURL = %q, want Docker localhost rewrite applied in constructor", provider.apiURL) + } +} + +func TestOpenAIEmbeddingProviderKeepsLoopbackOutsideDocker(t *testing.T) { + restore := config.SetInDockerForTest(false) + defer restore() + + provider := NewOpenAIEmbeddingProvider("ollama", "", "http://localhost:11434/v1", "nomic-embed-text") + if provider.apiURL != "http://localhost:11434/v1" { + t.Fatalf("apiURL = %q, want unchanged outside Docker", provider.apiURL) + } +} + +// TestMain pins Docker detection off so the httptest-based Embed tests above +// stay deterministic even when the test binary itself runs inside a container. +func TestMain(m *testing.M) { + restore := config.SetInDockerForTest(false) + code := m.Run() + restore() + os.Exit(code) +}