Skip to content

Commit db98e47

Browse files
fix(gateway): re-apply tool rate limiter after system_configs overlay (#1111)
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.
1 parent b5d5fce commit db98e47

2 files changed

Lines changed: 48 additions & 0 deletions

File tree

cmd/gateway.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,18 @@ func runGateway() {
226226
slog.Info("system_configs applied to in-memory config", "keys", len(sysConfigs))
227227
}
228228
}
229+
230+
// Re-apply tool rate limiter using DB-overlaid config. setupToolRegistry
231+
// initialised the limiter from the JSON5 default before ApplySystemConfigs
232+
// ran, so DB-driven changes to tools.rate_limit_per_hour were lost. Replace
233+
// the limiter object now that cfg reflects the DB value. Safe: server has
234+
// not started, no in-flight tool calls.
235+
if cfg.Tools.RateLimitPerHour > 0 {
236+
toolsReg.SetRateLimiter(tools.NewToolRateLimiter(cfg.Tools.RateLimitPerHour))
237+
slog.Info("tool rate limiting reapplied from system_configs", "per_hour", cfg.Tools.RateLimitPerHour)
238+
} else {
239+
toolsReg.SetRateLimiter(nil)
240+
}
229241
setupMemoryEmbeddings(pgStores, providerRegistry)
230242
usageCapSvc := usagecaps.NewService(pgStores.UsageCaps, pgStores.Providers)
231243

internal/tools/registry_test.go

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,42 @@ func TestRegistry_ExecuteWithContext_RateLimiting(t *testing.T) {
168168
}
169169
}
170170

171+
// SetRateLimiter must be re-callable so that startup code can swap the limiter
172+
// after DB-overlaid config is applied (cmd/gateway.go re-applies once
173+
// system_configs has overlaid the JSON5 default).
174+
func TestRegistry_SetRateLimiter_ReplacesPriorLimiter(t *testing.T) {
175+
reg := NewRegistry()
176+
reg.SetRateLimiter(NewToolRateLimiter(1)) // simulate JSON5 default
177+
reg.Register(&mockTool{name: "tool"})
178+
179+
// Replace with higher limit (simulates DB overlay = 5)
180+
reg.SetRateLimiter(NewToolRateLimiter(5))
181+
182+
for i := range 5 {
183+
result := reg.ExecuteWithContext(context.Background(), "tool", nil,
184+
"", "", "", "session-replace", nil)
185+
if result.IsError {
186+
t.Errorf("call %d should succeed under new 5/h limit: %s", i, result.ForLLM)
187+
}
188+
}
189+
result := reg.ExecuteWithContext(context.Background(), "tool", nil,
190+
"", "", "", "session-replace", nil)
191+
if !result.IsError {
192+
t.Error("6th call should hit the 5/h limit")
193+
}
194+
195+
// Disable rate limiting via nil — verifies the gateway path that disables
196+
// the limiter when cfg.Tools.RateLimitPerHour <= 0.
197+
reg.SetRateLimiter(nil)
198+
for i := range 10 {
199+
result := reg.ExecuteWithContext(context.Background(), "tool", nil,
200+
"", "", "", "session-replace", nil)
201+
if result.IsError {
202+
t.Errorf("call %d after nil limiter should be unbounded", i)
203+
}
204+
}
205+
}
206+
171207
func TestRegistry_ExecuteWithContext_NoRateLimitWithoutSessionKey(t *testing.T) {
172208
reg := NewRegistry()
173209
reg.SetRateLimiter(NewToolRateLimiter(1))

0 commit comments

Comments
 (0)