ddg-search-mcp

[onokonem]

No description provided.

[AnyCPU]

1. Remove duplicate formatPerplexityResults — use Markdown() instead
2. Extract shared extractArgs helper (3x copy-paste)
3. Remove pre-handler logToolCall + guard debug logging
4. Remove dead debugWriter and apiKey fields in perplexity client
5. Remove unused SearchOptions type + tests in perplexity
6. Remove double SetContext(ctx)
7. Make config loading conditional in main.go
8. Remove nonfunctional Context() in ShutdownHandler
9. Fix misleading doc comment on ReloadableConfig.Get()
10. Wire up SearchConfig defaults from appConfig in HandleSearch

[vrybas]

appConfig any breaks type safety.

The entire TLS configuration path in buildTLSConfig() (lines 437-503) uses a chain of interface type assertions (tlsConfigGetter, tlsFields) to extract config fields.

Similarly, ReloadTLS() (lines 164-213) does the same.

The Server struct doesn't know what its config is, yet deeply depends on
its shape.

Recommendation: Define an explicit interface that mcpconfig.Config satisfies:

type AppConfig interface {k
  GetTLSConfig() *TLSSettings // concrete type, not any
}

Or simply accept *mcpconfig.Config directly — these packages are in the same module.

[vrybas]

This adds 6 getter methods (GetEnabled, GetCertFile, GetKeyFile, GetMinVersion, GetMTLSEnabled, GetMTLSCAFile) on an already-exported struct with exported fields.

These exist solely to satisfy the inline interfaces in server.go. If the types were used directly, these would be unnecessary.

[vrybas]

The serverCancel is only called when Serve returns.

But in waitForShutdown (line 173), when shutdown signal arrives, nothing cancels serverCtx — the code just waits on shutdown.Wait().

The HTTP server is never told to stop. The Serve method does

select { case <-ctx.Done(): ... }

but ctx is serverCtx, which is never cancelled from outside.

Recommendation: Cancel serverCtx when shutdown is initiated.

[vrybas]

This is fragile string matching. Use errors.Is(err, context.Canceled) or a sentinel error.

[vrybas]

Both branches return context.Background(). This method never returns a context that gets cancelled on shutdown.

If the intent is to provide a cancellable context, it should use context.WithCancel tied to the shutdown channel.

Cancelling serverCtx on shutdown solves the shutdown problem without needing this method.

[vrybas]

When DDG_SEARCH_CONFIG_FILE points to a non-existent file, the code silently skips it and uses defaults.

If a user explicitly sets config file, they expect that file to be used. Silently falling back to defaults hides misconfiguration

[vrybas]

After SIGHUP reload, the callback reads currentCfg but never updates the server’s appConfig pointer.

Since tools and ReloadTLS() read configuration from mcpServer state, they keep using the initial config loaded at startup, so runtime config changes (e.g., Perplexity enable/token, search defaults, TLS file path changes) are silently ignored.

[vrybas]

This reads default locations/environment, even when startup used --config and LoadFromFile(...).

In that case, SIGHUP reload pulls a different config source, so services launched with a custom config file can unexpectedly revert to defaults or unrelated settings.

Also note: --log-level override is currently startup-only and will also be lost on reload.

[vrybas]

This hardcodes MinVersion: tls.VersionTLS12 and never uses the configured server.tls.min_version value, even though the interface exposes GetMinVersion().

[vrybas]

Can be removed because Do() at client.go:63 calls req.SetContext(ctx) again.