mirror of
https://github.com/tiennm99/goclaw.git
synced 2026-10-11 03:13:24 +00:00
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
This commit is contained in:
1 parent
7c4716e021
commit
254081462c
5 files changed
+96
No files matched your search
@@ -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 {
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
@@ -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,
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
Reference in new issue
Block a user