mirror of
https://github.com/tiennm99/goclaw.git
synced 2026-10-11 12:18:59 +00:00
fix(agent): filter per-tenant disabled tools from system prompt (#1396)
The system prompt's Tooling section previously listed tools even when they were disabled for a tenant and already stripped from the API tools parameter, confusing the LLM into thinking it could use them. Co-authored-by: Bruno Clermont <bruno.clermont@gmail.com>
This commit is contained in:
1 parent
aa15317d5b
commit
6bafe54b08
5 files changed
+67
-20
No files matched your search
@@ -36,6 +36,7 @@ func wireHTTP(stores *store.Stores, defaultWorkspace, dataDir, bundledSkillsDir
|
||||
agentsH = httpapi.NewAgentsHandler(stores.Agents, stores.Providers, providerReg, stores.DB, stores.Tracing, defaultWorkspace, msgBus, summoner, isOwner)
|
||||
agentsH.SetImportStores(stores.Memory, stores.KnowledgeGraph)
|
||||
agentsH.SetDataDir(dataDir)
|
||||
agentsH.SetDisabledToolsStore(stores.BuiltinToolTenantCfgs)
|
||||
if stores.SecureCLI != nil && stores.SecureCLIGrants != nil {
|
||||
if agentCreds, ok := stores.SecureCLI.(store.SecureCLIAgentCredentialStore); ok {
|
||||
agentsH.SetGatewayOperatorBootstrap(stores.SecureCLI, stores.SecureCLIGrants, agentCreds, gatewayAddr)
|
||||
|
||||
@@ -21,16 +21,28 @@ func filterBootstrapTools(toolNames []string) []string {
|
||||
// filteredToolNames returns tool names after applying policy filters.
|
||||
// Used for system prompt so denied tools don't appear in ## Tooling section.
|
||||
func (l *Loop) filteredToolNames() []string {
|
||||
var names []string
|
||||
if l.toolPolicy == nil {
|
||||
return l.tools.List()
|
||||
}
|
||||
defs := l.toolPolicy.FilterTools(l.tools, l.id, l.provider.Name(), l.agentToolPolicy, nil, false, false)
|
||||
names := make([]string, 0, len(defs))
|
||||
for _, d := range defs {
|
||||
if d.Function != nil {
|
||||
names = append(names, d.Function.Name)
|
||||
names = l.tools.List()
|
||||
} else {
|
||||
defs := l.toolPolicy.FilterTools(l.tools, l.id, l.provider.Name(), l.agentToolPolicy, nil, false, false)
|
||||
names = make([]string, 0, len(defs))
|
||||
for _, d := range defs {
|
||||
if d.Function != nil {
|
||||
names = append(names, d.Function.Name)
|
||||
}
|
||||
}
|
||||
}
|
||||
// Per-tenant tool exclusions: remove tools disabled for this agent's tenant.
|
||||
if len(l.disabledTools) > 0 {
|
||||
filtered := names[:0]
|
||||
for _, name := range names {
|
||||
if !l.disabledTools[name] {
|
||||
filtered = append(filtered, name)
|
||||
}
|
||||
}
|
||||
names = filtered
|
||||
}
|
||||
return names
|
||||
}
|
||||
|
||||
|
||||
@@ -53,7 +53,7 @@ type PreviewDeps struct {
|
||||
// allow/deny/alsoAllow pipeline (including global deny) for preview tool
|
||||
// names. nil = skip policy filtering (only skill_manage gating and alias
|
||||
// exclusion apply).
|
||||
ToolPolicy *tools.PolicyEngine
|
||||
ToolPolicy *tools.PolicyEngine
|
||||
SkillsLoader interface {
|
||||
BuildPinnedSummary(ctx context.Context, names []string) string
|
||||
BuildSummary(ctx context.Context, allowList []string) string
|
||||
@@ -62,7 +62,9 @@ type PreviewDeps struct {
|
||||
// When set, MCP tool descriptions are populated from configured servers
|
||||
// even if those servers are not currently loaded in the tool registry.
|
||||
MCPLister MCPPreviewLister
|
||||
DataDir string // for team workspace path construction
|
||||
// DisabledTools is the per-tenant set of disabled tool names (nil/empty = none disabled).
|
||||
DisabledTools map[string]bool
|
||||
DataDir string // for team workspace path construction
|
||||
}
|
||||
|
||||
// PreviewResult holds the output of BuildPreviewPrompt.
|
||||
@@ -173,6 +175,17 @@ func BuildPreviewPrompt(ctx context.Context, ag *store.AgentData, mode PromptMod
|
||||
}
|
||||
}
|
||||
|
||||
// --- Per-tenant disabled tools (matches loop_tool_filter.go:105-117) ---
|
||||
if len(deps.DisabledTools) > 0 {
|
||||
filtered := make([]string, 0, len(toolNames))
|
||||
for _, n := range toolNames {
|
||||
if !deps.DisabledTools[n] {
|
||||
filtered = append(filtered, n)
|
||||
}
|
||||
}
|
||||
toolNames = filtered
|
||||
}
|
||||
|
||||
// --- MCP tool descriptions (matches loop_history_supplement.go:44-58) ---
|
||||
// mcpToolParams tracks real parameter schemas alongside descriptions, keyed
|
||||
// by the same RegisteredName. Populated when a real schema is available
|
||||
|
||||
+19
-11
@@ -32,17 +32,18 @@ type AgentsHandler struct {
|
||||
providerReg *providers.Registry
|
||||
db *sql.DB
|
||||
tracingStore store.TracingStore
|
||||
memoryStore store.MemoryStore // for import (nil = disabled)
|
||||
kgStore store.KnowledgeGraphStore // for import (nil = disabled)
|
||||
episodicStore store.EpisodicStore // for import (nil in SQLite/lite builds)
|
||||
vaultStore store.VaultStore // for vault import (nil = disabled)
|
||||
toolsReg ToolPreviewLister // for system prompt preview tool resolution (nil = fallback)
|
||||
toolPE *tools.PolicyEngine // for system prompt preview tool policy resolution (nil = skip policy filtering)
|
||||
skillsLoader SkillPreviewBuilder // for system prompt preview pinned skills (nil = skip)
|
||||
skillAccessStore store.SkillAccessStore // for system prompt preview skill filtering (nil = skip)
|
||||
teamStore store.TeamStore // for system prompt preview team context (nil = skip)
|
||||
agentLinkStore store.AgentLinkStore // for system prompt preview delegation targets (nil = skip)
|
||||
mcpPreviewMgr agent.MCPPreviewLister // for store-based MCP tool preview (nil = skip)
|
||||
memoryStore store.MemoryStore // for import (nil = disabled)
|
||||
kgStore store.KnowledgeGraphStore // for import (nil = disabled)
|
||||
episodicStore store.EpisodicStore // for import (nil in SQLite/lite builds)
|
||||
vaultStore store.VaultStore // for vault import (nil = disabled)
|
||||
toolsReg ToolPreviewLister // for system prompt preview tool resolution (nil = fallback)
|
||||
toolPE *tools.PolicyEngine // for system prompt preview tool policy resolution (nil = skip policy filtering)
|
||||
skillsLoader SkillPreviewBuilder // for system prompt preview pinned skills (nil = skip)
|
||||
skillAccessStore store.SkillAccessStore // for system prompt preview skill filtering (nil = skip)
|
||||
teamStore store.TeamStore // for system prompt preview team context (nil = skip)
|
||||
agentLinkStore store.AgentLinkStore // for system prompt preview delegation targets (nil = skip)
|
||||
mcpPreviewMgr agent.MCPPreviewLister // for store-based MCP tool preview (nil = skip)
|
||||
disabledToolsStore store.BuiltinToolTenantConfigStore // for per-tenant disabled tool filtering in preview (nil = skip)
|
||||
secureCLI store.SecureCLIStore
|
||||
secureCLIGrants store.SecureCLIAgentGrantStore
|
||||
secureCLIAgentCreds store.SecureCLIAgentCredentialStore
|
||||
@@ -128,6 +129,13 @@ func (h *AgentsHandler) SetPreviewMCPManager(lister agent.MCPPreviewLister) {
|
||||
h.mcpPreviewMgr = lister
|
||||
}
|
||||
|
||||
// SetDisabledToolsStore attaches the tenant config store for per-tenant disabled
|
||||
// tool filtering in system prompt preview. nil is safe — no per-tenant filtering
|
||||
// is applied.
|
||||
func (h *AgentsHandler) SetDisabledToolsStore(dts store.BuiltinToolTenantConfigStore) {
|
||||
h.disabledToolsStore = dts
|
||||
}
|
||||
|
||||
// SetPreviewStores attaches team + agent link stores for system prompt preview.
|
||||
func (h *AgentsHandler) SetPreviewStores(ts store.TeamStore, als store.AgentLinkStore, sas store.SkillAccessStore) {
|
||||
h.teamStore = ts
|
||||
|
||||
@@ -92,6 +92,18 @@ func (h *AgentsHandler) handleSystemPromptPreview(w http.ResponseWriter, r *http
|
||||
// Build preview prompt — reuses same BuildSystemPrompt() as LLM pipeline.
|
||||
// Runtime-only fields (channel, peer kind, credentials) are zero-valued;
|
||||
// BuildSystemPrompt nil-checks every field so these sections are simply skipped.
|
||||
// Load per-tenant disabled tools for this agent's tenant.
|
||||
var disabledTools map[string]bool
|
||||
if h.disabledToolsStore != nil && ag.TenantID != uuid.Nil {
|
||||
if disabled, err := h.disabledToolsStore.ListDisabled(ctx, ag.TenantID); err == nil && len(disabled) > 0 {
|
||||
disabledTools = make(map[string]bool, len(disabled))
|
||||
for _, name := range disabled {
|
||||
disabledTools[name] = true
|
||||
}
|
||||
slog.Debug("handleSystemPromptPreview.disabled_tools", "agent_id", ag.ID, "tenant", ag.TenantID, "disabled", len(disabled))
|
||||
}
|
||||
}
|
||||
|
||||
slog.Debug("handleSystemPromptPreview.mcp_lister", "agent_id", ag.ID, "mcp_lister_nil", h.mcpPreviewMgr == nil)
|
||||
result := agent.BuildPreviewPrompt(ctx, ag, mode, r.URL.Query().Get("user_id"), agent.PreviewDeps{
|
||||
AgentStore: h.agents,
|
||||
@@ -103,6 +115,7 @@ func (h *AgentsHandler) handleSystemPromptPreview(w http.ResponseWriter, r *http
|
||||
SkillsLoader: h.skillsLoader,
|
||||
SkillAccessStore: h.skillAccessStore,
|
||||
MCPLister: h.mcpPreviewMgr,
|
||||
DisabledTools: disabledTools,
|
||||
DataDir: h.dataDir,
|
||||
})
|
||||
|
||||
|
||||
Reference in new issue
Block a user