diff --git a/internal/agent/loop_context.go b/internal/agent/loop_context.go index 8774c3d7..75623c90 100644 --- a/internal/agent/loop_context.go +++ b/internal/agent/loop_context.go @@ -67,10 +67,16 @@ func (l *Loop) injectContext(ctx context.Context, req *RunRequest) (contextSetup if req.SenderName != "" { ctx = store.WithSenderName(ctx, req.SenderName) } - // Inject global builtin tool settings for media tools (provider chain) + // Inject global + per-agent builtin tool settings (tier 1+3). + // Media/provider-chain tools read the merged view via BuiltinToolSettingsFromCtx. if l.builtinToolSettings != nil { ctx = tools.WithBuiltinToolSettings(ctx, l.builtinToolSettings) } + // Inject tenant-layer tool settings (tier 2). Merge with per-agent happens + // at read time — per-agent still wins at tool-name level. + if l.tenantToolSettings != nil { + ctx = tools.WithTenantToolSettings(ctx, l.tenantToolSettings) + } // Inject channel type into context for tools (e.g. message tool needs it for Zalo group routing) if req.ChannelType != "" { ctx = tools.WithToolChannelType(ctx, req.ChannelType) diff --git a/internal/agent/loop_types.go b/internal/agent/loop_types.go index 7a5f6af8..7cda95dc 100644 --- a/internal/agent/loop_types.go +++ b/internal/agent/loop_types.go @@ -160,9 +160,15 @@ type Loop struct { injectionAction string // "log", "warn" (default), "block", "off" maxMessageChars int // 0 = use default (32000) - // Global builtin tool settings (from builtin_tools table) + // Global builtin tool settings (from builtin_tools.settings table). + // Tier 3 in the overlay — tenant (tier 2) and future per-agent (tier 1) sit above. builtinToolSettings tools.BuiltinToolSettings + // Tenant-layer tool settings overlay (from builtin_tool_tenant_configs.settings). + // Tier 2 — sits above global (tier 3) and is merged at read time in + // BuiltinToolSettingsFromCtx with global winning at tool-name level. + tenantToolSettings tools.BuiltinToolSettings + // Per-tenant disabled tools (tool name → true means excluded from LLM) disabledTools map[string]bool @@ -327,9 +333,12 @@ type LoopConfig struct { InjectionAction string // "log", "warn" (default), "block", "off" MaxMessageChars int // 0 = use default (32000) - // Global builtin tool settings (from builtin_tools table) + // Global builtin tool settings (from builtin_tools table, merged with per-agent overrides) BuiltinToolSettings tools.BuiltinToolSettings + // Tenant-layer tool settings overlay (from builtin_tool_tenant_configs.settings). + TenantToolSettings tools.BuiltinToolSettings + // Per-tenant disabled tools (tool name → true means excluded) DisabledTools map[string]bool @@ -474,6 +483,7 @@ func NewLoop(cfg LoopConfig) *Loop { injectionAction: action, maxMessageChars: cfg.MaxMessageChars, builtinToolSettings: cfg.BuiltinToolSettings, + tenantToolSettings: cfg.TenantToolSettings, disabledTools: cfg.DisabledTools, reasoningConfig: cfg.ReasoningConfig, promptMode: cfg.PromptMode, diff --git a/internal/agent/resolver.go b/internal/agent/resolver.go index 0e7d7f90..9f29fe5d 100644 --- a/internal/agent/resolver.go +++ b/internal/agent/resolver.go @@ -343,7 +343,11 @@ func NewManagedResolver(deps ResolverDeps) ResolverFunc { } // Load per-tenant tool exclusions (disabled tools for this agent's tenant) - var disabledTools map[string]bool + // AND per-tenant tool settings overlay (tier 2 in the 4-tier cascade). + var ( + disabledTools map[string]bool + tenantToolSettings tools.BuiltinToolSettings + ) if deps.BuiltinToolTenantCfgs != nil && ag.TenantID != uuid.Nil { if disabled, err := deps.BuiltinToolTenantCfgs.ListDisabled(ctx, ag.TenantID); err == nil && len(disabled) > 0 { disabledTools = make(map[string]bool, len(disabled)) @@ -352,6 +356,16 @@ func NewManagedResolver(deps ResolverDeps) ResolverFunc { } slog.Debug("tenant tool exclusions", "agent", agentKey, "tenant", ag.TenantID, "disabled", len(disabled)) } + if settings, err := deps.BuiltinToolTenantCfgs.ListAllSettings(ctx, ag.TenantID); err != nil { + // Log but don't fail agent creation — fall back to global/hardcoded defaults. + slog.Warn("failed to load tenant tool settings", "agent", agentKey, "tenant", ag.TenantID, "error", err) + } else if len(settings) > 0 { + tenantToolSettings = make(tools.BuiltinToolSettings, len(settings)) + for name, raw := range settings { + tenantToolSettings[name] = []byte(raw) + } + slog.Debug("tenant tool settings loaded", "agent", agentKey, "tenant", ag.TenantID, "tools", len(tenantToolSettings)) + } } // Filter skills by visibility + agent grants. @@ -452,6 +466,7 @@ func NewManagedResolver(deps ResolverDeps) ResolverFunc { SandboxContainerDir: sandboxContainerDir, SandboxWorkspaceAccess: sandboxWorkspaceAccess, BuiltinToolSettings: builtinSettings, + TenantToolSettings: tenantToolSettings, DisabledTools: disabledTools, ReasoningConfig: store.ResolveEffectiveReasoningConfig(providerReasoningDefaults, ag.ParseReasoningConfig()), PromptMode: PromptMode(ag.ParsePromptMode()), diff --git a/internal/tools/builtin_tool_settings_merge_test.go b/internal/tools/builtin_tool_settings_merge_test.go new file mode 100644 index 00000000..6fc70804 --- /dev/null +++ b/internal/tools/builtin_tool_settings_merge_test.go @@ -0,0 +1,145 @@ +package tools + +import ( + "bytes" + "context" + "testing" + + "github.com/nextlevelbuilder/goclaw/internal/store" +) + +// ---- BuiltinToolSettingsFromCtx merge semantics (4-tier overlay) ---- + +func TestBuiltinToolSettingsFromCtx_EmptyCtx_ReturnsNil(t *testing.T) { + if got := BuiltinToolSettingsFromCtx(context.Background()); got != nil { + t.Errorf("empty ctx: want nil, got %v", got) + } +} + +func TestBuiltinToolSettingsFromCtx_PerAgentOnly(t *testing.T) { + perAgent := BuiltinToolSettings{"web_search": []byte(`{"k":"per-agent"}`)} + ctx := WithBuiltinToolSettings(context.Background(), perAgent) + + got := BuiltinToolSettingsFromCtx(ctx) + if !bytes.Equal(got["web_search"], []byte(`{"k":"per-agent"}`)) { + t.Errorf("per-agent only: got %s", got["web_search"]) + } + + // Fast path: same map returned (no copy). + if &got == &perAgent { + // maps are reference types — compare underlying via len + value + } + if len(got) != 1 { + t.Errorf("expected single entry, got %d", len(got)) + } +} + +func TestBuiltinToolSettingsFromCtx_TenantOnly(t *testing.T) { + tenant := BuiltinToolSettings{"web_search": []byte(`{"k":"tenant"}`)} + ctx := WithTenantToolSettings(context.Background(), tenant) + + got := BuiltinToolSettingsFromCtx(ctx) + if !bytes.Equal(got["web_search"], []byte(`{"k":"tenant"}`)) { + t.Errorf("tenant only: got %s", got["web_search"]) + } +} + +func TestBuiltinToolSettingsFromCtx_BothMergeTenantWinsOverGlobal(t *testing.T) { + // WithBuiltinToolSettings carries tier 3 (global defaults). + // WithTenantToolSettings carries tier 2 (tenant admin override). + // Tenant must win at tool-name level. + global := BuiltinToolSettings{ + "web_search": []byte(`{"k":"global"}`), + "web_fetch": []byte(`{"k":"global"}`), + } + tenant := BuiltinToolSettings{ + "web_search": []byte(`{"k":"tenant"}`), // overrides global + "tts": []byte(`{"k":"tenant"}`), // tenant-only + } + + ctx := WithBuiltinToolSettings(context.Background(), global) + ctx = WithTenantToolSettings(ctx, tenant) + + got := BuiltinToolSettingsFromCtx(ctx) + if len(got) != 3 { + t.Errorf("merged: want 3 entries, got %d (%v)", len(got), got) + } + // web_search: tenant wins (overrides global default). + if !bytes.Equal(got["web_search"], []byte(`{"k":"tenant"}`)) { + t.Errorf("tenant should override global for web_search, got %s", got["web_search"]) + } + // web_fetch: only global has it — global value surfaces. + if !bytes.Equal(got["web_fetch"], []byte(`{"k":"global"}`)) { + t.Errorf("web_fetch should come from global, got %s", got["web_fetch"]) + } + // tts: only tenant has it. + if !bytes.Equal(got["tts"], []byte(`{"k":"tenant"}`)) { + t.Errorf("tts should come from tenant, got %s", got["tts"]) + } +} + +func TestBuiltinToolSettingsFromCtx_RunContextFallback(t *testing.T) { + // Neither ctx key set but RunContext has settings — should fall back. + rc := &store.RunContext{ + BuiltinToolSettings: map[string][]byte{"web_search": []byte(`{"from":"rc"}`)}, + } + ctx := store.WithRunContext(context.Background(), rc) + + got := BuiltinToolSettingsFromCtx(ctx) + if !bytes.Equal(got["web_search"], []byte(`{"from":"rc"}`)) { + t.Errorf("RunContext fallback: got %s", got["web_search"]) + } +} + +func TestBuiltinToolSettingsFromCtx_BuiltinCtxKeyOverridesRunContext(t *testing.T) { + // Builtin ctx key must take precedence over RunContext fallback. + // RunContext only serves empty-both-keys case. + rc := &store.RunContext{ + BuiltinToolSettings: map[string][]byte{"web_search": []byte(`{"from":"rc"}`)}, + } + global := BuiltinToolSettings{"web_search": []byte(`{"from":"global"}`)} + + ctx := store.WithRunContext(context.Background(), rc) + ctx = WithBuiltinToolSettings(ctx, global) + + got := BuiltinToolSettingsFromCtx(ctx) + if !bytes.Equal(got["web_search"], []byte(`{"from":"global"}`)) { + t.Errorf("ctx key must override RunContext, got %s", got["web_search"]) + } +} + +// ---- TenantToolSettingsFromCtx raw accessor ---- + +func TestTenantToolSettingsFromCtx_RoundTrip(t *testing.T) { + tenant := BuiltinToolSettings{"web_search": []byte(`{"k":"tenant"}`)} + ctx := WithTenantToolSettings(context.Background(), tenant) + + got := TenantToolSettingsFromCtx(ctx) + if len(got) != 1 || !bytes.Equal(got["web_search"], tenant["web_search"]) { + t.Errorf("raw tenant round-trip failed: %v", got) + } + + // Empty ctx → nil + if TenantToolSettingsFromCtx(context.Background()) != nil { + t.Errorf("empty ctx should return nil tenant settings") + } +} + +// ---- Fast path allocation check ---- +// When only one tier is present, the merge function must return the same +// underlying map without allocating. We verify by checking equality after +// mutation — if a copy was made, the original wouldn't see the change. +// +// Note: this is a behavioral assertion, not an allocation benchmark. + +func TestBuiltinToolSettingsFromCtx_FastPath_ReturnsSameMap(t *testing.T) { + perAgent := BuiltinToolSettings{"web_search": []byte(`original`)} + ctx := WithBuiltinToolSettings(context.Background(), perAgent) + + got := BuiltinToolSettingsFromCtx(ctx) + // Mutate the returned map — if it's the same map, perAgent sees the change. + got["web_fetch"] = []byte(`mutated`) + if _, ok := perAgent["web_fetch"]; !ok { + t.Errorf("fast path should return same map, got a copy") + } +} diff --git a/internal/tools/context_keys.go b/internal/tools/context_keys.go index f6052275..2f12f68d 100644 --- a/internal/tools/context_keys.go +++ b/internal/tools/context_keys.go @@ -174,25 +174,94 @@ func RunKindFromCtx(ctx context.Context) string { // Leader agents in this mode can only relay status — mutations are blocked. const RunKindNotification = "notification" -// --- Builtin tool settings (global DB overrides) --- +// --- Builtin tool settings (3-tier overlay, tier-1 reserved) --- +// +// Tool config resolution order (most specific wins): +// 1. (reserved — future per-agent override) +// 2. Tenant override (builtin_tool_tenant_configs.settings, via WithTenantToolSettings) +// 3. Global default (builtin_tools.settings, via WithBuiltinToolSettings — resolver-loaded) +// 4. Hardcoded (tool internal default when the map has no entry) +// +// `WithBuiltinToolSettings` carries the resolver-loaded global defaults (tier 3). +// If per-agent overrides are added later, they can share this ctx key (same map) or +// introduce a dedicated key — the merge function's precedence (builtin ctx key wins +// over tenant) already reflects "more specific wins". +// +// Merge happens at tool-name level: if both tiers define `web_search`, the builtin +// ctx-key value wins wholesale (no field-level deep merge inside the JSON blob). -const ctxBuiltinToolSettings toolContextKey = "tool_builtin_settings" +const ( + ctxBuiltinToolSettings toolContextKey = "tool_builtin_settings" + ctxTenantToolSettings toolContextKey = "tool_tenant_settings" +) // BuiltinToolSettings maps tool name → settings JSON bytes. type BuiltinToolSettings map[string][]byte +// WithBuiltinToolSettings injects the per-agent + global tool settings map. +// Tier 1 + tier 3 in the overlay (coalesced by the resolver). func WithBuiltinToolSettings(ctx context.Context, settings BuiltinToolSettings) context.Context { return context.WithValue(ctx, ctxBuiltinToolSettings, settings) } +// WithTenantToolSettings injects the tenant-layer tool settings map (tier 2). +// Loaded by the resolver via BuiltinToolTenantConfigStore.ListAllSettings. +func WithTenantToolSettings(ctx context.Context, settings BuiltinToolSettings) context.Context { + return context.WithValue(ctx, ctxTenantToolSettings, settings) +} + +// TenantToolSettingsFromCtx returns the raw tenant-layer map without merging. +// Use BuiltinToolSettingsFromCtx for the merged view that tools should read. +func TenantToolSettingsFromCtx(ctx context.Context) BuiltinToolSettings { + v, _ := ctx.Value(ctxTenantToolSettings).(BuiltinToolSettings) + return v +} + +// BuiltinToolSettingsFromCtx returns the merged tool settings view. +// +// Merge semantics (current wiring — tier 1 per-agent override not yet loaded): +// - `WithBuiltinToolSettings` carries tier 3 (global defaults) +// - `WithTenantToolSettings` carries tier 2 (tenant admin override) +// - Tenant wins over global at tool-name level (no field-level deep merge) +// +// When a future phase adds tier 1 (per-agent override), it will either +// introduce a third ctx key that outranks both, OR the resolver will merge +// tier 1 into the builtin-ctx-key map before injection. In the latter case, +// semantics remain correct so long as tier 1 is layered ABOVE tenant — the +// resolver will need a split then. +// +// Fast paths: single-tier or empty merge returns the underlying map directly +// with zero allocations. Merge only runs when BOTH layers have entries. func BuiltinToolSettingsFromCtx(ctx context.Context) BuiltinToolSettings { - if v, _ := ctx.Value(ctxBuiltinToolSettings).(BuiltinToolSettings); v != nil { - return v + global, _ := ctx.Value(ctxBuiltinToolSettings).(BuiltinToolSettings) + tenant, _ := ctx.Value(ctxTenantToolSettings).(BuiltinToolSettings) + + if len(global) == 0 && len(tenant) == 0 { + // RunContext fallback (existing behavior — subagent runs inherit + // parent's BuiltinToolSettings through RunContext serialization). + if rc := store.RunContextFromCtx(ctx); rc != nil && rc.BuiltinToolSettings != nil { + return BuiltinToolSettings(rc.BuiltinToolSettings) + } + return nil } - if rc := store.RunContextFromCtx(ctx); rc != nil { - return BuiltinToolSettings(rc.BuiltinToolSettings) + + // Fast paths — no allocation when only one tier is active. + if len(tenant) == 0 { + return global } - return nil + if len(global) == 0 { + return tenant + } + + // Both tiers present: layer tenant override on top of global defaults. + merged := make(BuiltinToolSettings, len(global)+len(tenant)) + for k, v := range global { + merged[k] = v + } + for k, v := range tenant { + merged[k] = v + } + return merged } // --- Per-agent restrict_to_workspace override ---