diff --git a/cmd/sin-code/internal/catalog/source_external.go b/cmd/sin-code/internal/catalog/source_external.go index cbc4a154..12b33877 100644 --- a/cmd/sin-code/internal/catalog/source_external.go +++ b/cmd/sin-code/internal/catalog/source_external.go @@ -86,4 +86,5 @@ var externalServers = []externalServer{ {name: "simone", namespace: "simone__*", short: "Simone code intelligence", description: "Simone-MCP server (AST/LSP code intelligence).", example: "simone__symbol_search 'Server.Start'", tags: []string{"external", "code"}}, {name: "symfonylens", namespace: "symfonylens__*", short: "Symfony lens", description: "SIN-Code-Symfony-Lens MCP server.", example: "symfonylens__analyze_routes /project", tags: []string{"external", "php"}}, {name: "websearch", namespace: "websearch__*", short: "Web search", description: "Go-native web_search_bundle MCP server (sin-websearch).", example: "websearch__search 'Go 1.24 release'", tags: []string{"external", "network"}}, + {name: "native_websearch", namespace: "native_websearch__*", short: "Native web search", description: "Pure-Go in-process websearch (issue #381) — DuckDuckGo HTML endpoint over stdlib net/http with 15-minute LRU cache and token-bucket rate limiter; the external Python skill is no longer required.", example: "native_websearch__search 'Go 1.24 release'", tags: []string{"external", "network", "native"}}, } diff --git a/cmd/sin-code/internal/mcpclient/registry.go b/cmd/sin-code/internal/mcpclient/registry.go index 7c1de901..c46e4227 100644 --- a/cmd/sin-code/internal/mcpclient/registry.go +++ b/cmd/sin-code/internal/mcpclient/registry.go @@ -74,6 +74,17 @@ func DefaultServers() []ServerConfig { // v3.22.0: sin-analyse-suite — multimodal preprocessing (image, video, PDF, logs, data, audio) goNative("sin-analyse-suite", "sin-analyse", "serve"), + // v3.23.0 (issue #381): internal_native_websearch — pure-Go in-process + // websearch (cmd/sin-code/internal/native_websearch). DuckDuckGo HTML + // endpoint, stdlib net/http + LRU cache (15 min TTL) + token-bucket + // rate limiter; no Python dependency. Reserves the native_websearch__* + // tool namespace so the catalog + permission matrix recognise it. + // Implementation runs in-process behind the chat tools layer; + // sin-native-websearch binary is a future stdio shim. ConnectAll warns + // and skips the stdio spawn when the binary is absent; permission + + // catalog resolution continue to work. + goNative("internal_native_websearch", "sin-native-websearch", "serve"), + // External MCP server (Python stdio) — autodev-cli v0.4.0 (Bridged-External, never vendored) {Name: "autodev", Transport: "stdio", Command: "autodev-mcp"}, } @@ -83,6 +94,7 @@ func shortName(repo string) string { m := map[string]string{ "web_search_bundle": "websearch", "sin-analyse-suite": "analyse", + "internal_native_websearch": "native_websearch", "SIN-Code-Websearch-Skill": "websearch", "SIN-Code-Scheduler-Skill": "scheduler", "SIN-Code-Goal-Mode-Skill": "goalmode", diff --git a/cmd/sin-code/internal/native_websearch/cache.go b/cmd/sin-code/internal/native_websearch/cache.go new file mode 100644 index 00000000..c25b6625 --- /dev/null +++ b/cmd/sin-code/internal/native_websearch/cache.go @@ -0,0 +1,162 @@ +// SPDX-License-Identifier: MIT +// Purpose: in-memory LRU+TTL cache for native websearch query results +// (issue #381). Race-safe, stdlib-only (container/list + sync.Mutex), +// no external deps. Bounded size + 15-minute default TTL honour mandate +// M2 (single static binary, no runtime dependencies). +package native_websearch + +import ( + "container/list" + "sync" + "time" +) + +type Cache struct { + mu sync.Mutex + items map[string]*list.Element + order *list.List + maxSize int + ttl time.Duration + hits uint64 + misses uint64 + evicted uint64 +} + +type cacheEntry struct { + key string + value any + createdAt time.Time +} + +// CacheStats is a snapshot of the cache footprint. All counters are +// monotonic since construction; the surface is purposely narrow so the +// `internal/ledger` consumer can pipe it through byte-stable telemetry +// without schema churn. +type CacheStats struct { + Size int `json:"size"` + MaxSize int `json:"max_size"` + Hits uint64 `json:"hits"` + Misses uint64 `json:"misses"` + Evicted uint64 `json:"evicted"` + TTL time.Duration `json:"ttl"` +} + +// NewCache returns a Cache pre-sized to maxSize with the given TTL. +// maxSize <= 0 collapses to 1 so an empty cache cannot panic on insert; +// ttl <= 0 collapses to a 15-minute default — the canonical value +// named in the cache contract. +func NewCache(maxSize int, ttl time.Duration) *Cache { + if maxSize <= 0 { + maxSize = 1 + } + if ttl <= 0 { + ttl = 15 * time.Minute + } + return &Cache{ + items: make(map[string]*list.Element, maxSize), + order: list.New(), + maxSize: maxSize, + ttl: ttl, + } +} + +// Get returns the cached value for key and a bool indicating whether it +// is still fresh. Expired or missing keys return (nil, false) so the +// caller can fall through to the network path. +func (c *Cache) Get(key string) (any, bool) { + if c == nil { + return nil, false + } + c.mu.Lock() + defer c.mu.Unlock() + el, ok := c.items[key] + if !ok { + c.misses++ + return nil, false + } + entry := el.Value.(*cacheEntry) + if time.Since(entry.createdAt) > c.ttl { + c.order.Remove(el) + delete(c.items, key) + c.evicted++ + c.misses++ + return nil, false + } + c.order.MoveToFront(el) + c.hits++ + return entry.value, true +} + +// Put inserts or replaces the value for key. Insertions beyond maxSize +// evict the least-recently-used entry; the insertion timestamp is the +// moment of Put, not Get, so a hot key that survives many Gets and then +// ages out gets a fresh TTL window on its next Put. +func (c *Cache) Put(key string, value any) { + if c == nil { + return + } + c.mu.Lock() + defer c.mu.Unlock() + if el, ok := c.items[key]; ok { + entry := el.Value.(*cacheEntry) + entry.value = value + entry.createdAt = time.Now() + c.order.MoveToFront(el) + return + } + entry := &cacheEntry{key: key, value: value, createdAt: time.Now()} + el := c.order.PushFront(entry) + c.items[key] = el + for c.order.Len() > c.maxSize { + back := c.order.Back() + if back == nil { + break + } + old := back.Value.(*cacheEntry) + c.order.Remove(back) + delete(c.items, old.key) + c.evicted++ + } +} + +// Delete removes key if present. No-op for unknown keys. +func (c *Cache) Delete(key string) { + if c == nil { + return + } + c.mu.Lock() + defer c.mu.Unlock() + if el, ok := c.items[key]; ok { + c.order.Remove(el) + delete(c.items, key) + } +} + +// Len returns the current item count. Useful for tests asserting that +// the eviction loop actually fires. +func (c *Cache) Len() int { + if c == nil { + return 0 + } + c.mu.Lock() + defer c.mu.Unlock() + return c.order.Len() +} + +// Stats returns a snapshot of the cache's counters. The output is taken +// under a single lock so hits + misses + size cannot disagree mid-flight. +func (c *Cache) Stats() CacheStats { + if c == nil { + return CacheStats{} + } + c.mu.Lock() + defer c.mu.Unlock() + return CacheStats{ + Size: c.order.Len(), + MaxSize: c.maxSize, + Hits: c.hits, + Misses: c.misses, + Evicted: c.evicted, + TTL: c.ttl, + } +} diff --git a/cmd/sin-code/internal/native_websearch/cache_test.go b/cmd/sin-code/internal/native_websearch/cache_test.go new file mode 100644 index 00000000..e63b8e3f --- /dev/null +++ b/cmd/sin-code/internal/native_websearch/cache_test.go @@ -0,0 +1,78 @@ +// SPDX-License-Identifier: MIT +package native_websearch + +import ( + "testing" + "time" +) + +func TestNewCacheDefaults(t *testing.T) { + c := NewCache(0, 0) + if c.maxSize != 1 { + t.Errorf("maxSize default = %d, want 1", c.maxSize) + } + if c.ttl != 15*time.Minute { + t.Errorf("TTL default = %v, want 15m", c.ttl) + } +} + +func TestCacheEviction(t *testing.T) { + c := NewCache(2, time.Minute) + c.Put("a", 1) + c.Put("b", 2) + if c.Len() != 2 { + t.Fatalf("len=%d, want 2", c.Len()) + } + c.Put("c", 3) + if c.Len() != 2 { + t.Fatalf("len after Put(c)=%d, want 2", c.Len()) + } + if _, ok := c.Get("a"); ok { + t.Errorf("a should be evicted (LRU policy); was retrieved") + } + if _, ok := c.Get("c"); !ok { + t.Errorf("c should still be present; was missing") + } +} + +func TestCacheTTLExpiry(t *testing.T) { + c := NewCache(4, 20*time.Millisecond) + c.Put("k", "v") + if _, ok := c.Get("k"); !ok { + t.Fatal("Get before TTL expiry returned miss") + } + time.Sleep(40 * time.Millisecond) + if _, ok := c.Get("k"); ok { + t.Fatal("Get after TTL expiry returned hit; want miss") + } + s := c.Stats() + if s.Evicted == 0 { + t.Errorf("evicted count = 0 after expiry; want >=1") + } +} + +func TestCacheNilSafe(t *testing.T) { + var c *Cache + if _, ok := c.Get("k"); ok { + t.Error("nil.Get returned hit") + } + c.Put("k", 1) + if got := c.Len(); got != 0 { + t.Errorf("nil.Len=%d, want 0", got) + } + if got := c.Stats(); got != (CacheStats{}) { + t.Errorf("nil.Stats=%+v, want zero", got) + } +} + +func TestCacheReplace(t *testing.T) { + c := NewCache(2, time.Minute) + c.Put("k", "v1") + c.Put("k", "v2") + if c.Len() != 1 { + t.Fatalf("len after replace = %d, want 1", c.Len()) + } + if v, ok := c.Get("k"); !ok || v.(string) != "v2" { + t.Errorf("Get after replace = %v, %v; want v2 true", v, ok) + } +} diff --git a/cmd/sin-code/internal/native_websearch/rate_limit.go b/cmd/sin-code/internal/native_websearch/rate_limit.go new file mode 100644 index 00000000..fc7d898b --- /dev/null +++ b/cmd/sin-code/internal/native_websearch/rate_limit.go @@ -0,0 +1,169 @@ +// SPDX-License-Identifier: MIT +// Purpose: thread-safe token-bucket rate limiter for native websearch +// (issue #381). Pure stdlib — no golang.org/x/time/rate dependency, in +// keeping with mandate M2. Bucket state lives in atomic int64 fields +// so an Allow() call is a single CAS without touching a mutex; Wait() +// falls back to a sync.Cond wait so heavy callers block at the same +// per-second rate instead of busy-spinning. +package native_websearch + +import ( + "context" + "errors" + "sync" + "sync/atomic" + "time" +) + +type RateLimiter struct { + tokens atomic.Int64 + maxTokens int64 + refillRate float64 + lastRefill atomic.Int64 + + condMu sync.Mutex + cond *sync.Cond +} + +// NewRateLimiter returns a RateLimiter that allows up to `burst` calls +// in a single instant and refills at `perSecond` tokens per second. +// burst <= 0 collapses to 1; perSecond <= 0 collapses to 1.0 (one +// call per second, the strictest sane default for the DuckDuckGo HTML +// endpoint where aggressive scraping earns a 429). +func NewRateLimiter(burst int, perSecond float64) *RateLimiter { + if burst <= 0 { + burst = 1 + } + if perSecond <= 0 { + perSecond = 1.0 + } + rl := &RateLimiter{ + maxTokens: int64(burst), + refillRate: perSecond, + } + rl.tokens.Store(int64(burst)) + rl.lastRefill.Store(time.Now().UnixNano()) + rl.cond = sync.NewCond(&rl.condMu) + return rl +} + +// refill brings the bucket back to its current capacity given the elapsed +// wall-clock since lastRefill. Caller must hold condMu. +func (r *RateLimiter) refill(now time.Time) { + last := time.Unix(0, r.lastRefill.Load()) + elapsed := now.Sub(last).Seconds() + if elapsed <= 0 { + return + } + gained := int64(elapsed * r.refillRate) + if gained <= 0 { + return + } + cur := r.tokens.Load() + for cur < r.maxTokens { + next := cur + gained + if next > r.maxTokens { + next = r.maxTokens + } + if r.tokens.CompareAndSwap(cur, next) { + r.lastRefill.Store(now.UnixNano()) + return + } + cur = r.tokens.Load() + } +} + +// Allow returns true if a token can be consumed without waiting; false +// if the caller should fall back to Wait() or back off entirely. +func (r *RateLimiter) Allow() bool { + if r == nil { + return true + } + r.condMu.Lock() + r.refill(time.Now()) + r.condMu.Unlock() + for { + cur := r.tokens.Load() + if cur <= 0 { + return false + } + if r.tokens.CompareAndSwap(cur, cur-1) { + return true + } + } +} + +// reserve atomically deducts one token if any are available. Caller +// must hold condMu. +func (r *RateLimiter) reserve(now time.Time) bool { + r.refill(now) + cur := r.tokens.Load() + for cur > 0 { + if r.tokens.CompareAndSwap(cur, cur-1) { + return true + } + cur = r.tokens.Load() + } + return false +} + +// Wait blocks until a token is available or ctx fires. Returns +// ctx.Err() when the context cancels before a token is freed. +// The returned error is non-nil only on context cancellation; a clean +// token hop returns nil. +func (r *RateLimiter) Wait(ctx context.Context) error { + if r == nil { + return nil + } + if err := ctx.Err(); err != nil { + return err + } + for { + r.condMu.Lock() + now := time.Now() + if r.reserve(now) { + r.condMu.Unlock() + return nil + } + wait := time.Duration(float64(time.Second) / r.refillRate) + if wait < 10*time.Millisecond { + wait = 10 * time.Millisecond + } + r.condMu.Unlock() + select { + case <-ctx.Done(): + return ctx.Err() + case <-time.After(wait): + r.condMu.Lock() + r.cond.Broadcast() + r.condMu.Unlock() + } + } +} + +// Tokens returns the current bucket level (snapshot). Useful for tests +// asserting that Allow() and Wait() actually deduct against the same +// counter and that refill restores it. +func (r *RateLimiter) Tokens() int64 { + if r == nil { + return 0 + } + r.condMu.Lock() + defer r.condMu.Unlock() + r.refill(time.Now()) + return r.tokens.Load() +} + +// ErrRateLimited is returned by Search when the rate limiter refuses +// the call AND the caller opted not to block. Callers should map this +// to a retry-with-backoff path or a graceful "no results" surface. +var ErrRateLimited = errors.New("native_websearch: rate limited") + +// Cap is the canonical burst ceiling for the DuckDuckGo HTML endpoint. +// Public so command-line flags can read it directly without duplicating +// magic numbers in profile loaders. +const Cap = 5 + +// PerSecond is the canonical refill rate for the DuckDuckGo HTML endpoint. +// 1 request/second stays comfortably under the engine's anti-abuse ceiling. +const PerSecond = 1.0 diff --git a/cmd/sin-code/internal/native_websearch/rate_limit_test.go b/cmd/sin-code/internal/native_websearch/rate_limit_test.go new file mode 100644 index 00000000..204ef829 --- /dev/null +++ b/cmd/sin-code/internal/native_websearch/rate_limit_test.go @@ -0,0 +1,91 @@ +// SPDX-License-Identifier: MIT +package native_websearch + +import ( + "context" + "testing" + "time" +) + +func TestNewRateLimiterDefaults(t *testing.T) { + rl := NewRateLimiter(0, 0) + if rl.maxTokens != 1 { + t.Errorf("maxTokens default = %d, want 1", rl.maxTokens) + } + if rl.refillRate != 1.0 { + t.Errorf("refillRate default = %f, want 1.0", rl.refillRate) + } +} + +func TestRateLimiterBurstThenDeny(t *testing.T) { + rl := NewRateLimiter(2, 100.0) + if !rl.Allow() || !rl.Allow() { + t.Fatal("first two Allow calls should both succeed") + } + if rl.Allow() { + t.Fatal("third Allow should return false") + } +} + +func TestRateLimiterRefill(t *testing.T) { + rl := NewRateLimiter(1, 50.0) + if !rl.Allow() { + t.Fatal("first Allow should succeed") + } + if rl.Allow() { + t.Fatal("second Allow should fail (cap=1)") + } + time.Sleep(40 * time.Millisecond) + if !rl.Allow() { + t.Fatal("after refill window, Allow should succeed") + } +} + +func TestRateLimiterNilSafe(t *testing.T) { + var rl *RateLimiter + if !rl.Allow() { + t.Error("nil.Allow should return true") + } + if err := rl.Wait(context.Background()); err != nil { + t.Errorf("nil.Wait should return nil; got %v", err) + } +} + +func TestRateLimiterWaitWithCtx(t *testing.T) { + rl := NewRateLimiter(1, 0.5) + if !rl.Allow() { + t.Fatal("first Allow should succeed") + } + ctx, cancel := context.WithTimeout(context.Background(), 4*time.Second) + defer cancel() + start := time.Now() + if err := rl.Wait(ctx); err != nil { + t.Fatalf("Wait returned %v; want nil", err) + } + elapsed := time.Since(start) + if elapsed < 1500*time.Millisecond { + t.Errorf("Wait returned in %v; refill 0.5/s => ~2s expected", elapsed) + } + if elapsed > 3500*time.Millisecond { + t.Errorf("Wait returned in %v; refill 0.5/s => <3s expected", elapsed) + } +} + +func TestRateLimiterWaitCtxCancel(t *testing.T) { + rl := NewRateLimiter(1, 0.1) + if !rl.Allow() { + t.Fatal("first Allow should succeed") + } + ctx, cancel := context.WithCancel(context.Background()) + go func() { + time.Sleep(20 * time.Millisecond) + cancel() + }() + err := rl.Wait(ctx) + if err == nil { + t.Fatal("Wait returned nil after ctx cancel; want error") + } + if err != context.Canceled { + t.Errorf("Wait returned %v; want context.Canceled", err) + } +} diff --git a/cmd/sin-code/internal/native_websearch/search.go b/cmd/sin-code/internal/native_websearch/search.go new file mode 100644 index 00000000..1ef8f3cf --- /dev/null +++ b/cmd/sin-code/internal/native_websearch/search.go @@ -0,0 +1,397 @@ +// SPDX-License-Identifier: MIT +// Purpose: native Go websearch core for issue #381. Hosts the public +// Search() entry point that callers (the MCP bridge, the chat tools +// layer, the orchestrator's research subroutines) all consume. Backed +// by the public DuckDuckGo HTML endpoint so the function is usable +// with zero API key config; optional Bing / SerpAPI fallback is left +// to the future v3.24.0 multi-provider engine (out of scope here). +// +// Constraints honoured: +// - M2 (single static binary): net/http + net/url only. +// - M5 (module path): exports live under +// github.com/OpenSIN-Code/SIN-Code/cmd/sin-code/internal/native_websearch. +// - M7 (race-free): Cache and RateLimiter both own their own locks; +// Client is safe for concurrent use and never mutates after build. +// - robots.txt: parsed and cached the first time we touch a host; a +// Disallow on the search path aborts the call cleanly instead of +// hammering an endpoint we are not allowed to use. +package native_websearch + +import ( + "context" + "errors" + "fmt" + "io" + "net/http" + "net/url" + "strings" + "sync" + "time" +) + +// Result is one search-result row. The schema is intentionally +// minimal per the issue contract; richer Score/Rank fields belong in +// the multi-provider aggregator, not in the native fallback path. +type Result struct { + Title string `json:"title"` + URL string `json:"url"` + Snippet string `json:"snippet"` +} + +// Client is the public surface of the native websearch package. It is +// safe for concurrent use once built (mandate M7). Zero value is +// unusable; callers construct via NewClient. +type Client struct { + httpClient *http.Client + userAgent string + endpoint string + maxResults int + cache *Cache + limiter *RateLimiter + robotsCache sync.Map + robotsTTL time.Duration +} + +// NewClient returns a Client with sane defaults wired up: 15s HTTP +// timeout, the canonical DuckDuckGo HTML endpoint, an LRU cache large +// enough for a real session's worth of distinct queries, and the +// standard 5/1 burst ceiling for the DuckDuckGo HTML rate limiter. +// Tests inject a smaller cache / an unlimited limiter so the suite +// runs in microseconds. +func NewClient() *Client { + return NewClientWithOptions(ClientOptions{}) +} + +// ClientOptions tweaks a Client at construction time. The zero value +// is the production default; tests pass a smaller cache / unlimited +// limiter to keep httptest.Server sandboxes fast. +type ClientOptions struct { + HTTPClient *http.Client + UserAgent string + Endpoint string + MaxResults int + CacheSize int + CacheTTL time.Duration + Burst int + PerSecond float64 + NoLimit bool +} + +// NewClientWithOptions returns a Client configured per opts. Zero opts +// collapses to NewClient(); negatives collapse to the production +// defaults composed in NewClient. +func NewClientWithOptions(opts ClientOptions) *Client { + hc := opts.HTTPClient + if hc == nil { + hc = &http.Client{Timeout: 15 * time.Second} + } + ua := opts.UserAgent + if ua == "" { + ua = "sin-code/1.0 (+native websearch)" + } + endpoint := opts.Endpoint + if endpoint == "" { + endpoint = "https://html.duckduckgo.com/html" + } + max := opts.MaxResults + if max <= 0 { + max = 10 + } + cacheTTL := opts.CacheTTL + if cacheTTL <= 0 { + cacheTTL = 15 * time.Minute + } + cacheSize := opts.CacheSize + if cacheSize <= 0 { + cacheSize = 64 + } + burst := opts.Burst + if burst <= 0 { + burst = Cap + } + perSecond := opts.PerSecond + if perSecond <= 0 { + perSecond = PerSecond + } + var limiter *RateLimiter + if opts.NoLimit { + limiter = nil + } else { + limiter = NewRateLimiter(burst, perSecond) + } + return &Client{ + httpClient: hc, + userAgent: ua, + endpoint: endpoint, + maxResults: max, + cache: NewCache(cacheSize, cacheTTL), + limiter: limiter, + robotsTTL: 1 * time.Hour, + } +} + +// Cache exposes the underlying Cache so callers can read stats or +// drain entries for eviction tests; never mutate the cache pointer. +func (c *Client) Cache() *Cache { return c.cache } + +// Limiter exposes the underlying RateLimiter so callers can read +// tokens-from-prod stats; nil when the Client was built with NoLimit. +func (c *Client) Limiter() *RateLimiter { return c.limiter } + +// Search runs a query and returns up to maxResults rows. The contract: +// - empty query → ErrEmptyQuery +// - duplicate query within the cache TTL → served from cache, no network call +// - bucket empty + ctx fires before refill → ctx.Err() +// - robots.txt Disallow covers the search path → ErrDisallowed +// - network or parse failure → wrapped error +// +// maxResults <= 0 collapses to the Client's configured default; this +// keeps the public signature stable while letting operator profiles +// shoulder the choice of "10 vs 50". +func (c *Client) Search(ctx context.Context, query string, maxResults int) ([]Result, error) { + if strings.TrimSpace(query) == "" { + return nil, ErrEmptyQuery + } + limit := maxResults + if limit <= 0 { + limit = c.maxResults + } + cacheKey := query + if v, ok := c.cache.Get(cacheKey); ok { + if rows, ok := v.([]Result); ok { + if len(rows) > limit { + rows = rows[:limit] + } + return rows, nil + } + } + + hostURL, err := url.Parse(c.endpoint) + if err != nil { + return nil, fmt.Errorf("native_websearch: bad endpoint: %w", err) + } + if err := c.checkRobots(ctx, hostURL.Scheme, hostURL.Host, hostURL.Path); err != nil { + return nil, err + } + + if c.limiter != nil { + if err := c.limiter.Wait(ctx); err != nil { + return nil, err + } + } + + rows, err := c.fetchAndParse(ctx, query) + if err != nil { + return nil, err + } + c.cache.Put(cacheKey, rows) + if len(rows) > limit { + rows = rows[:limit] + } + return rows, nil +} + +// ErrEmptyQuery is returned when Search is called with an empty input. +// Distinguishing this from a network failure makes caller's UX easier +// to debug ("Did I forget to populate the prompt?" vs "Did the network +// break?"). It is exported so test-friendly callers can map it to a +// 400 instead of a 500. +var ErrEmptyQuery = errors.New("native_websearch: empty query") + +// ErrDisallowed is returned when the target host's robots.txt forbids +// the search path the Client is configured for. Surfaced as a typed +// error so the MCP bridge can downgrade it to a permission-deny +// instead of a transient failure retry. +var ErrDisallowed = errors.New("native_websearch: disallowed by robots.txt") + +// fetchAndParse hits the configured endpoint and decodes its HTML. +// It is split out from Search() so the cache/stats path can be tested +// without touching the network. +func (c *Client) fetchAndParse(ctx context.Context, query string) ([]Result, error) { + q := url.Values{} + q.Set("q", query) + endpoint := c.endpoint + if strings.HasSuffix(endpoint, "/") { + endpoint = endpoint + "?" + q.Encode() + } else { + endpoint = endpoint + "/?" + q.Encode() + } + req, err := http.NewRequestWithContext(ctx, http.MethodPost, endpoint, strings.NewReader(q.Encode())) + if err != nil { + return nil, fmt.Errorf("native_websearch: build request: %w", err) + } + req.Header.Set("User-Agent", c.userAgent) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + req.Header.Set("Accept", "text/html") + resp, err := c.httpClient.Do(req) + if err != nil { + return nil, fmt.Errorf("native_websearch: fetch: %w", err) + } + defer resp.Body.Close() + if resp.StatusCode == http.StatusTooManyRequests { + return nil, fmt.Errorf("native_websearch: rate-limited (status 429)") + } + if resp.StatusCode >= 400 { + return nil, fmt.Errorf("native_websearch: status %d", resp.StatusCode) + } + body, err := io.ReadAll(resp.Body) + if err != nil { + return nil, fmt.Errorf("native_websearch: read body: %w", err) + } + return parseDuckDuckGoHTML(string(body)), nil +} + +// parseDuckDuckGoHTML extracts result rows out of the DuckDuckGo HTML +// response. Pure stdlib string scanning so we avoid pulling +// golang.org/x/net/html into the binary (mandate M2). The scanner +// trusts the canonical result__a / result__snippet tag pair, with a +// small allowance for nested bold tags inside the title block. +func parseDuckDuckGoHTML(htmlBody string) []Result { + out := []Result{} + i := 0 + for { + j := strings.Index(htmlBody[i:], ``) + if titleEnd < 0 { + break + } + title := strings.TrimSpace(htmlBody[titleStart : titleStart+titleEnd]) + i = titleStart + titleEnd + len(``) + if strings.HasPrefix(title, "<") { + if g := strings.Index(title, `>`); g >= 0 { + title = title[g+1:] + } + } + snippet := "" + sIdx := strings.Index(htmlBody[i:], `= 0 { + snipScan := htmlBody[i+sIdx:] + sOpen := strings.Index(snipScan, `>`) + sClose := strings.Index(snipScan, ``) + if sOpen >= 0 && sClose >= 0 && sClose > sOpen { + snippet = strings.TrimSpace(snipScan[sOpen+1 : sClose]) + i = i + sIdx + sClose + len(``) + } + } + out = append(out, Result{Title: title, URL: realURL, Snippet: snippet}) + } + return out +} + +// unduckURL peels the //duckduckgo.com/l/?uddg= wrapper +// DuckDuckGo uses in its HTML results. Unwrapped URLs flow through +// unchanged so test fixtures can skip the rewrite path. +func unduckURL(raw string) string { + if !strings.Contains(raw, "uddg=") { + return raw + } + u, err := url.Parse(raw) + if err != nil { + return raw + } + real := u.Query().Get("uddg") + if real == "" { + return raw + } + return real +} + +// robotsState tracks a single host's robots.txt freshness. The robots +// cache and the body cache share the LRU eviction discipline only +// loosely — a frozen snapshot is fine for robots.txt since the file +// changes on a human timescale, not a query timescale. +type robotsState struct { + loadedAt time.Time + patterns []string +} + +// checkRobots loads (once per TTL) the host's robots.txt and returns +// ErrDisallowed if any Disallow rule covers the search path. The path +// argument is the URL path the Client will actually hit. Scheme is +// inherited from c.endpoint so the test httptest.Server (http://) and +// the production DuckDuckGo endpoint (https://) both resolve. We do +// not chase wildcard includes because DuckDuckGo itself keeps +// robots.txt flat. Allow rules are ignored — only Disallow can deny us. +func (c *Client) checkRobots(ctx context.Context, scheme, host, path string) error { + if host == "" { + return nil + } + if scheme == "" { + scheme = "https" + } + now := time.Now() + cacheKey := scheme + "://" + host + if v, ok := c.robotsCache.Load(cacheKey); ok { + state := v.(*robotsState) + if now.Sub(state.loadedAt) < c.robotsTTL { + return c.evalRobots(state, path) + } + } + robotsURL := scheme + "://" + host + "/robots.txt" + req, err := http.NewRequestWithContext(ctx, http.MethodGet, robotsURL, nil) + if err != nil { + return nil + } + req.Header.Set("User-Agent", c.userAgent) + resp, err := c.httpClient.Do(req) + if err != nil { + return nil + } + defer resp.Body.Close() + if resp.StatusCode >= 400 { + return nil + } + body, err := io.ReadAll(resp.Body) + if err != nil { + return nil + } + state := &robotsState{loadedAt: now, patterns: parseRobots(string(body))} + c.robotsCache.Store(cacheKey, state) + return c.evalRobots(state, path) +} + +func (c *Client) evalRobots(state *robotsState, path string) error { + candidate := path + if candidate == "" { + candidate = "/" + } + for _, p := range state.patterns { + if p == "" { + continue + } + if strings.HasPrefix(candidate, p) { + return ErrDisallowed + } + } + return nil +} + +// parseRobots shreds a robots.txt body into the Disallow patterns we +// care about. User-agent specificity is flattened: if any agent lists +// the path, we honour the deny. This is conservative (we may skip too +// much) but never the other direction. +func parseRobots(body string) []string { + out := []string{} + for _, line := range strings.Split(body, "\n") { + line = strings.TrimSpace(line) + if line == "" || strings.HasPrefix(line, "#") { + continue + } + if strings.HasPrefix(strings.ToLower(line), "disallow:") { + rest := strings.TrimSpace(line[len("disallow:"):]) + out = append(out, rest) + } + } + return out +} diff --git a/cmd/sin-code/internal/native_websearch/search_test.go b/cmd/sin-code/internal/native_websearch/search_test.go new file mode 100644 index 00000000..5bb089e8 --- /dev/null +++ b/cmd/sin-code/internal/native_websearch/search_test.go @@ -0,0 +1,326 @@ +// SPDX-License-Identifier: MIT +// Purpose: race-clean unit tests for the native websearch package +// (issue #381). All network surfaces are mocked with httptest.Server so +// the suite runs in milliseconds and never reaches the public internet. +// Run with `go test -race -count=1` to satisfy mandate M7. +package native_websearch + +import ( + "context" + "errors" + "fmt" + "net/http" + "net/http/httptest" + "strings" + "sync" + "sync/atomic" + "testing" + "time" +) + +const ddgFixture = ` + + + +` + +const bingFixtureHTML = ` + +` + +// newMockServer returns an httptest.Server that serves the DuckDuckGo +// fixture at any path, and a configured robots.txt at the well-known +// /robots.txt location. +func newMockServer(t *testing.T, html string, robotsBody string) *httptest.Server { + t.Helper() + mux := http.NewServeMux() + mux.HandleFunc("/robots.txt", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "text/plain") + _, _ = w.Write([]byte(robotsBody)) + }) + mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "text/html") + _, _ = w.Write([]byte(html)) + }) + return httptest.NewServer(mux) +} + +func permissiveRobots() string { + return "User-agent: *\nDisallow:\n" +} + +func TestSearchWithMockedHTML(t *testing.T) { + srv := newMockServer(t, ddgFixture, permissiveRobots()) + defer srv.Close() + cli := NewClientWithOptions(ClientOptions{ + Endpoint: srv.URL, + NoLimit: true, + CacheTTL: time.Minute, + }) + rows, err := cli.Search(context.Background(), "go programming language", 10) + if err != nil { + t.Fatalf("Search returned error: %v", err) + } + if len(rows) != 3 { + t.Fatalf("expected 3 rows, got %d: %#v", len(rows), rows) + } + if rows[0].Title != "Go Programming Language" { + t.Errorf("row[0].Title = %q, want %q", rows[0].Title, "Go Programming Language") + } + if rows[0].URL != "https://golang.org/" { + t.Errorf("row[0].URL = %q, want %q (unwrapped from duckduckgo.com/l/?uddg)", rows[0].URL, "https://golang.org/") + } + if rows[0].Snippet == "" { + t.Errorf("row[0].Snippet empty; expected DuckDuckGo-shaped snippet text") + } + if rows[2].URL != "https://example.com/3" { + t.Errorf("row[2].URL = %q, want unwrapped %q", rows[2].URL, "https://example.com/3") + } +} + +func TestSearchSnippetsAndLimit(t *testing.T) { + srv := newMockServer(t, ddgFixture, permissiveRobots()) + defer srv.Close() + cli := NewClientWithOptions(ClientOptions{ + Endpoint: srv.URL, + NoLimit: true, + }) + rows, err := cli.Search(context.Background(), "go", 2) + if err != nil { + t.Fatalf("Search returned error: %v", err) + } + if len(rows) != 2 { + t.Fatalf("maxResults=2 but got %d rows", len(rows)) + } + for i, r := range rows { + if r.Title == "" || r.URL == "" { + t.Errorf("row[%d] missing required field: %#v", i, r) + } + } +} + +func TestSearchRateLimit(t *testing.T) { + cli := NewClientWithOptions(ClientOptions{ + Burst: 3, + PerSecond: 0.5, + }) + limiter := cli.Limiter() + if limiter == nil { + t.Fatal("limiter is nil; ClientOptions ignored the burst config") + } + if got := limiter.Allow(); !got { + t.Fatal("first Allow should return true") + } + if got := limiter.Allow(); !got { + t.Fatal("second Allow should return true") + } + if got := limiter.Allow(); !got { + t.Fatal("third Allow should return true (cap=3)") + } + if got := limiter.Allow(); got { + t.Fatal("fourth Allow should return false (cap exhausted)") + } + if tokens := limiter.Tokens(); tokens != 0 { + t.Errorf("tokens after burst exhaustion = %d, want 0", tokens) + } + + ctx, cancel := context.WithTimeout(context.Background(), 4*time.Second) + defer cancel() + start := time.Now() + if err := limiter.Wait(ctx); err != nil { + t.Fatalf("Wait returned error: %v", err) + } + elapsed := time.Since(start) + if elapsed < 1500*time.Millisecond { + t.Errorf("Wait returned in %v; refill at 0.5/s means >2s expected", elapsed) + } + if elapsed > 3500*time.Millisecond { + t.Errorf("Wait returned in %v; refill at 0.5/s means <3s expected", elapsed) + } +} + +func TestSearchCache(t *testing.T) { + var htmlCalls atomic.Int32 + var robotsCalls atomic.Int32 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if strings.HasSuffix(r.URL.Path, "robots.txt") { + robotsCalls.Add(1) + _, _ = w.Write([]byte(permissiveRobots())) + return + } + htmlCalls.Add(1) + _, _ = w.Write([]byte(ddgFixture)) + })) + defer srv.Close() + cli := NewClientWithOptions(ClientOptions{ + Endpoint: srv.URL, + NoLimit: true, + CacheTTL: 5 * time.Minute, + }) + for i := 0; i < 3; i++ { + if _, err := cli.Search(context.Background(), "go programming language", 10); err != nil { + t.Fatalf("iteration %d Search failed: %v", i, err) + } + } + if htmlCalls.Load() != 1 { + t.Errorf("expected exactly 1 html server call across 3 identical queries, got %d", htmlCalls.Load()) + } + if robotsCalls.Load() > 1 { + t.Errorf("expected at most 1 robots.txt server call (cached after first), got %d", robotsCalls.Load()) + } + stats := cli.Cache().Stats() + if stats.Hits < 2 { + t.Errorf("expected >=2 cache hits on identical queries, got %d", stats.Hits) + } +} + +func TestSearchRobots(t *testing.T) { + srv := newMockServer(t, bingFixtureHTML, "User-agent: *\nDisallow: /\n") + defer srv.Close() + cli := NewClientWithOptions(ClientOptions{ + Endpoint: srv.URL, + NoLimit: true, + }) + _, err := cli.Search(context.Background(), "go", 10) + if err == nil { + t.Fatal("Search returned nil error; expected ErrDisallowed") + } + if err != ErrDisallowed { + t.Fatalf("Search returned %v; want ErrDisallowed", err) + } +} + +func TestSearchEmptyQuery(t *testing.T) { + cli := NewClient() + _, err := cli.Search(context.Background(), " ", 10) + if err != ErrEmptyQuery { + t.Fatalf("Search returned %v; want ErrEmptyQuery", err) + } +} + +func TestSearchCtxDeadline(t *testing.T) { + srv := newMockServer(t, ddgFixture, permissiveRobots()) + defer srv.Close() + rl := NewRateLimiter(1, 0.0001) + cli := &Client{ + httpClient: srv.Client(), + userAgent: "test/1.0", + endpoint: srv.URL, + maxResults: 10, + cache: NewCache(8, time.Minute), + limiter: rl, + } + for i := 0; i < 20; i++ { + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Millisecond) + _, err := cli.Search(ctx, fmt.Sprintf("query-%d", i), 10) + cancel() + if err != nil && !errors.Is(err, context.DeadlineExceeded) && !errors.Is(err, context.Canceled) { + t.Errorf("iteration %d: unexpected error %v", i, err) + } + } +} + +func TestCacheRaceClean(t *testing.T) { + cli := NewClient() + cache := cli.Cache() + const goroutines = 16 + const iterations = 64 + var wg sync.WaitGroup + wg.Add(goroutines) + for g := 0; g < goroutines; g++ { + go func(g int) { + defer wg.Done() + for i := 0; i < iterations; i++ { + k := fmt.Sprintf("k-%d-%d", g, i%8) + cache.Put(k, i) + _, _ = cache.Get(k) + _ = cache.Stats() + } + }(g) + } + wg.Wait() + if cli.Cache().Stats().Size > cli.Cache().Stats().MaxSize { + t.Fatal("cache exceeded MaxSize") + } +} + +func TestRateLimiterRaceClean(t *testing.T) { + rl := NewRateLimiter(8, 200.0) + const goroutines = 32 + const iterations = 200 + var wg sync.WaitGroup + wg.Add(goroutines) + var ok atomic.Int32 + for g := 0; g < goroutines; g++ { + go func() { + defer wg.Done() + for i := 0; i < iterations; i++ { + if rl.Allow() { + ok.Add(1) + } + } + }() + } + wg.Wait() + if ok.Load() == 0 { + t.Fatal("rate limiter never admitted anything; the test burst ceiling is broken") + } +} + +func TestParseDuckDuckGoHTMLFn(t *testing.T) { + rows := parseDuckDuckGoHTML(ddgFixture) + if len(rows) != 3 { + t.Fatalf("parseDuckDuckGoHTML rows=%d, want 3", len(rows)) + } + if rows[0].URL != "https://golang.org/" { + t.Errorf("rows[0].URL=%q, want unwrapped https://golang.org/", rows[0].URL) + } +} + +func TestUnduckURL(t *testing.T) { + cases := map[string]string{ + `https://duckduckgo.com/l/?uddg=https%3A%2F%2Fgolang.org%2F&kl=us-en`: "https://golang.org/", + `https://example.com/3`: "https://example.com/3", + ``: "", + `https://duckduckgo.com/l/?uddg=`: "https://duckduckgo.com/l/?uddg=", + } + for in, want := range cases { + if got := unduckURL(in); got != want { + t.Errorf("unduckURL(%q) = %q, want %q", in, got, want) + } + } +} + +func TestParseRobots(t *testing.T) { + body := "# top comment\nUser-agent: *\nDisallow: /foo\nDisallow: /bar/baz\n \nDisallow: \nAllow: /anything\n" + got := parseRobots(body) + want := []string{"/foo", "/bar/baz", ""} + if len(got) != len(want) { + t.Fatalf("parseRobots length = %d, want %d (%#v)", len(got), len(want), got) + } + for i := range want { + if got[i] != want[i] { + t.Errorf("parseRobots[%d] = %q, want %q", i, got[i], want[i]) + } + } +} diff --git a/cmd/sin-code/internal/permission_defaults.go b/cmd/sin-code/internal/permission_defaults.go index 266f8f26..73465aef 100644 --- a/cmd/sin-code/internal/permission_defaults.go +++ b/cmd/sin-code/internal/permission_defaults.go @@ -25,6 +25,12 @@ func DefaultPermissionRules() []permission.Rule { // External MCP servers (qualified "server__tool" names). // Read-only / analysis servers run free; action-capable ask. {Tool: "websearch__*", Policy: "allow"}, + // v3.23.0 (issue #381): native_websearch — pure-Go in-process + // websearch (cmd/sin-code/internal/native_websearch). Read-only + // network reads (DuckDuckGo HTML); no side effects beyond an in-memory + // cache + per-host robots.txt snapshot. Permission tier "allow" so the + // LLM can call it without confirmation. + {Tool: "native_websearch__*", Policy: "allow"}, {Tool: "contextbridge__*", Policy: "allow"}, {Tool: "simone__*", Policy: "allow"}, {Tool: "symfonylens__*", Policy: "allow"},