From 943606ccf66b0dc912d8ba5fe1b2c5b1102102e1 Mon Sep 17 00:00:00 2001 From: Plateau Nguyen Date: Wed, 6 May 2026 23:51:38 +0700 Subject: [PATCH] fix(gateway): re-apply tool rate limiter after system_configs overlay setupToolRegistry creates the rate limiter from cfg.Tools.RateLimitPerHour during early bootstrap (gateway.go line 137). System_configs DB overlay runs ~50 lines later via cfg.ApplySystemConfigs (line 191), so any DB override of tools.rate_limit_per_hour was silently lost - the limiter object was already initialised from the JSON5 default. Symptom in production: editing tools.rate_limit_per_hour via system_configs table or the config HTTP API had no effect on running gateways. Operators had to inject a config.json file to change the value, defeating the DB-as-source-of-truth pattern that other tunables rely on. Re-apply the limiter after ApplySystemConfigs runs. Safe ordering: server has not started yet, no in-flight tool calls, and SetRateLimiter is a plain field assignment with no shutdown cost on the discarded limiter. nil case (rate_limit_per_hour <= 0) also handled so DB writes can disable the limiter without restart. Tests: new TestRegistry_SetRateLimiter_ReplacesPriorLimiter covers both the replace-with-higher-limit path and the nil-disables path. All existing rate limiter tests still pass. Note: the same ordering pattern likely affects other config consumed inside setupToolRegistry (tools.scrub_credentials, MCP server wiring). Out of scope for this hotfix - they need a deeper restructure. --- cmd/gateway.go | 12 +++++++++++ internal/tools/registry_test.go | 36 +++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/cmd/gateway.go b/cmd/gateway.go index 0ebb2a899c..20eb269d1f 100644 --- a/cmd/gateway.go +++ b/cmd/gateway.go @@ -192,6 +192,18 @@ func runGateway() { slog.Info("system_configs applied to in-memory config", "keys", len(sysConfigs)) } } + + // Re-apply tool rate limiter using DB-overlaid config. setupToolRegistry + // initialised the limiter from the JSON5 default before ApplySystemConfigs + // ran, so DB-driven changes to tools.rate_limit_per_hour were lost. Replace + // the limiter object now that cfg reflects the DB value. Safe: server has + // not started, no in-flight tool calls. + if cfg.Tools.RateLimitPerHour > 0 { + toolsReg.SetRateLimiter(tools.NewToolRateLimiter(cfg.Tools.RateLimitPerHour)) + slog.Info("tool rate limiting reapplied from system_configs", "per_hour", cfg.Tools.RateLimitPerHour) + } else { + toolsReg.SetRateLimiter(nil) + } setupMemoryEmbeddings(pgStores, providerRegistry) // Resolve background provider for consolidation + vault enrichment. diff --git a/internal/tools/registry_test.go b/internal/tools/registry_test.go index 1daeb39752..5e6885e973 100644 --- a/internal/tools/registry_test.go +++ b/internal/tools/registry_test.go @@ -168,6 +168,42 @@ func TestRegistry_ExecuteWithContext_RateLimiting(t *testing.T) { } } +// SetRateLimiter must be re-callable so that startup code can swap the limiter +// after DB-overlaid config is applied (cmd/gateway.go re-applies once +// system_configs has overlaid the JSON5 default). +func TestRegistry_SetRateLimiter_ReplacesPriorLimiter(t *testing.T) { + reg := NewRegistry() + reg.SetRateLimiter(NewToolRateLimiter(1)) // simulate JSON5 default + reg.Register(&mockTool{name: "tool"}) + + // Replace with higher limit (simulates DB overlay = 5) + reg.SetRateLimiter(NewToolRateLimiter(5)) + + for i := range 5 { + result := reg.ExecuteWithContext(context.Background(), "tool", nil, + "", "", "", "session-replace", nil) + if result.IsError { + t.Errorf("call %d should succeed under new 5/h limit: %s", i, result.ForLLM) + } + } + result := reg.ExecuteWithContext(context.Background(), "tool", nil, + "", "", "", "session-replace", nil) + if !result.IsError { + t.Error("6th call should hit the 5/h limit") + } + + // Disable rate limiting via nil — verifies the gateway path that disables + // the limiter when cfg.Tools.RateLimitPerHour <= 0. + reg.SetRateLimiter(nil) + for i := range 10 { + result := reg.ExecuteWithContext(context.Background(), "tool", nil, + "", "", "", "session-replace", nil) + if result.IsError { + t.Errorf("call %d after nil limiter should be unbounded", i) + } + } +} + func TestRegistry_ExecuteWithContext_NoRateLimitWithoutSessionKey(t *testing.T) { reg := NewRegistry() reg.SetRateLimiter(NewToolRateLimiter(1))