fix(agent): ensure isolated agents inherit manually registered tools to prevent test hangs
This commit is contained in:
stevef
parent
de03f78b69
commit
6d7b603cb7
21 changed files with 494 additions and 109 deletions
2
Makefile
2
Makefile
|
|
@ -249,7 +249,7 @@ vet: generate
|
|||
|
||||
## test: Test Go code
|
||||
test: generate
|
||||
@$(GO) test $(GOFLAGS) $$($(GO) list $(GOFLAGS) ./... | grep -v github.com/sipeed/picoclaw/web/)
|
||||
@$(GO) test $(GOFLAGS) -p 1 $$($(GO) list $(GOFLAGS) ./... | grep -v github.com/sipeed/picoclaw/web/) -timeout 120s
|
||||
@cd web && make test
|
||||
|
||||
## fmt: Format Go code
|
||||
|
|
|
|||
|
|
@ -43,7 +43,7 @@ func NewSkillsCommand() *cobra.Command {
|
|||
globalDir := filepath.Dir(internal.GetConfigPath())
|
||||
globalSkillsDir := filepath.Join(globalDir, "skills")
|
||||
builtinSkillsDir := filepath.Join(globalDir, "picoclaw", "skills")
|
||||
d.skillsLoader = skills.NewSkillsLoader(d.workspace, globalSkillsDir, builtinSkillsDir, nil, false)
|
||||
d.skillsLoader = skills.NewSkillsLoader(d.workspace, d.workspace, globalSkillsDir, builtinSkillsDir, nil, false)
|
||||
|
||||
return nil
|
||||
},
|
||||
|
|
|
|||
|
|
@ -67,6 +67,38 @@ PicoClaw stores data in your configured workspace (default: `~/.picoclaw/workspa
|
|||
|
||||
> **Note:** Changes to `AGENT.md`, `SOUL.md`, `USER.md` and `memory/MEMORY.md` are automatically detected at runtime via file modification time (mtime) tracking. You do **not** need to restart the gateway after editing these files — the agent picks up the new content on the next request.
|
||||
|
||||
### 🔒 Multi-Tenant Agent Isolation
|
||||
|
||||
PicoClaw supports safe multi-tenancy on shared infrastructure (e.g., Azure deployments). It dynamically isolates each chat session into its own private sub-workspace to prevent data collisions and ensure privacy between different users/callers (like n8n, Foundation Agents, etc.).
|
||||
|
||||
#### Isolation Strategy
|
||||
|
||||
When an incoming message includes a **ChatID** (passed in the `/chat` API or extracted from internal channels), PicoClaw automatically activates **Tenant Isolation**:
|
||||
|
||||
1. **Isolated Workspace:** The agent's operations are restricted to `workspace/sessions/{isolationID}/workspace`.
|
||||
2. **Isolated Memory:** Long-term memory (`MEMORY.md`) is stored and read from the isolated session path.
|
||||
3. **Isolated Tools:** Tools like `read_file` and `write_file` are automatically pointed to the isolated workspace, preventing any tenant from accessing another's files or the global base workspace.
|
||||
|
||||
#### Tenant Identification (Inbound Integration)
|
||||
|
||||
PicoClaw automatically detects the **ChatID** for isolation from several sources:
|
||||
|
||||
1. **API Headers (Automatic):** It checks for common tenant-identifying headers from API Gateways:
|
||||
- `X-PicoClaw-Chat-ID`: Custom header for manual control.
|
||||
- `Ocp-Apim-Subscription-Id`: Automatically captures the **Azure APIM Subscription ID** as the tenant identifier.
|
||||
2. **API Body:** The JSON payload for `/chat` can include a `chat_id` (or `session_id`) field.
|
||||
3. **Channel Context:** Channels like Microsoft Teams, Telegram, and Discord automatically pass their respective `ChatID`.
|
||||
|
||||
**What happens if no ID is present?**
|
||||
If no `ChatID` is detected, the request is routed to the **Global Agent** context, which uses the root workspace. This is the default for standalone single-user deployments. For secure multi-tenancy on shared infrastructure, ensuring a persistent `ChatID` is passed from your API Gateway or client is highly recommended.
|
||||
|
||||
#### Path Resolution
|
||||
|
||||
- **Global Agents:** Agents initialized at startup (without a specific session) use the root workspace.
|
||||
- **Session Agents:** Every request with a `chatID` creates a transient isolated agent instance that "routes" all file and memory operations into its session-specific subdirectory.
|
||||
|
||||
This mechanism is transparent to the end-user and the AI agent itself, ensuring a secure and portable multi-user environment out-of-the-box.
|
||||
|
||||
### Skill Sources
|
||||
|
||||
By default, skills are loaded from:
|
||||
|
|
|
|||
|
|
@ -22,6 +22,7 @@ import (
|
|||
|
||||
type ContextBuilder struct {
|
||||
workspace string
|
||||
baseWorkspace string
|
||||
skillsLoader *skills.SkillsLoader
|
||||
memory *MemoryStore
|
||||
toolDiscoveryBM25 bool
|
||||
|
|
@ -69,7 +70,11 @@ func getGlobalConfigDir() string {
|
|||
return filepath.Join(home, pkg.DefaultPicoClawHome)
|
||||
}
|
||||
|
||||
func NewContextBuilder(workspace string) *ContextBuilder {
|
||||
func NewContextBuilder(workspace string, baseWorkspace string) *ContextBuilder {
|
||||
// If isolationID logic is needed, it should be handled by the caller
|
||||
// ensuring workspace and baseWorkspace are correctly distinct.
|
||||
os.MkdirAll(workspace, 0o755)
|
||||
|
||||
// builtin skills: skills directory in current project
|
||||
// Use the skills/ directory under the current working directory
|
||||
builtinSkillsDir := strings.TrimSpace(os.Getenv(config.EnvBuiltinSkills))
|
||||
|
|
@ -80,9 +85,10 @@ func NewContextBuilder(workspace string) *ContextBuilder {
|
|||
globalSkillsDir := filepath.Join(getGlobalConfigDir(), "skills")
|
||||
|
||||
return &ContextBuilder{
|
||||
workspace: workspace,
|
||||
skillsLoader: skills.NewSkillsLoader(workspace, globalSkillsDir, builtinSkillsDir, nil, false),
|
||||
memory: NewMemoryStore(workspace),
|
||||
workspace: workspace,
|
||||
baseWorkspace: baseWorkspace,
|
||||
skillsLoader: skills.NewSkillsLoader(workspace, baseWorkspace, globalSkillsDir, builtinSkillsDir, nil, false),
|
||||
memory: NewMemoryStore(workspace),
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -470,7 +476,13 @@ func (cb *ContextBuilder) LoadBootstrapFiles() string {
|
|||
|
||||
if agentDefinition.Source != AgentDefinitionSourceAgent {
|
||||
filePath := filepath.Join(cb.workspace, "IDENTITY.md")
|
||||
if data, err := os.ReadFile(filePath); err == nil {
|
||||
data, err := os.ReadFile(filePath)
|
||||
if err != nil && cb.baseWorkspace != "" && cb.baseWorkspace != cb.workspace {
|
||||
// Fallback to base workspace
|
||||
filePath = filepath.Join(cb.baseWorkspace, "IDENTITY.md")
|
||||
data, err = os.ReadFile(filePath)
|
||||
}
|
||||
if err == nil {
|
||||
fmt.Fprintf(&sb, "## %s\n\n%s\n\n", "IDENTITY.md", data)
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -41,7 +41,7 @@ func TestSingleSystemMessage(t *testing.T) {
|
|||
})
|
||||
defer os.RemoveAll(tmpDir)
|
||||
|
||||
cb := NewContextBuilder(tmpDir)
|
||||
cb := NewContextBuilder(tmpDir, tmpDir)
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
|
|
@ -132,7 +132,7 @@ func TestBuildMessages_CurrentSenderDynamicContext(t *testing.T) {
|
|||
})
|
||||
defer os.RemoveAll(tmpDir)
|
||||
|
||||
cb := NewContextBuilder(tmpDir)
|
||||
cb := NewContextBuilder(tmpDir, tmpDir)
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
|
|
@ -221,7 +221,7 @@ func TestMtimeAutoInvalidation(t *testing.T) {
|
|||
tmpDir := setupWorkspace(t, map[string]string{tt.file: tt.contentV1})
|
||||
defer os.RemoveAll(tmpDir)
|
||||
|
||||
cb := NewContextBuilder(tmpDir)
|
||||
cb := NewContextBuilder(tmpDir, tmpDir)
|
||||
|
||||
sp1 := cb.BuildSystemPromptWithCache()
|
||||
|
||||
|
|
@ -257,7 +257,7 @@ func TestMtimeAutoInvalidation(t *testing.T) {
|
|||
tmpDir := setupWorkspace(t, nil)
|
||||
defer os.RemoveAll(tmpDir)
|
||||
|
||||
cb := NewContextBuilder(tmpDir)
|
||||
cb := NewContextBuilder(tmpDir, tmpDir)
|
||||
_ = cb.BuildSystemPromptWithCache() // populate cache
|
||||
|
||||
// Touch skills directory (simulate new skill installed)
|
||||
|
|
@ -284,7 +284,7 @@ func TestExplicitInvalidateCache(t *testing.T) {
|
|||
})
|
||||
defer os.RemoveAll(tmpDir)
|
||||
|
||||
cb := NewContextBuilder(tmpDir)
|
||||
cb := NewContextBuilder(tmpDir, tmpDir)
|
||||
|
||||
sp1 := cb.BuildSystemPromptWithCache()
|
||||
cb.InvalidateCache()
|
||||
|
|
@ -312,7 +312,7 @@ func TestCacheStability(t *testing.T) {
|
|||
})
|
||||
defer os.RemoveAll(tmpDir)
|
||||
|
||||
cb := NewContextBuilder(tmpDir)
|
||||
cb := NewContextBuilder(tmpDir, tmpDir)
|
||||
|
||||
results := make([]string, 5)
|
||||
for i := range results {
|
||||
|
|
@ -361,7 +361,7 @@ func TestNewFileCreationInvalidatesCache(t *testing.T) {
|
|||
tmpDir := setupWorkspace(t, nil)
|
||||
defer os.RemoveAll(tmpDir)
|
||||
|
||||
cb := NewContextBuilder(tmpDir)
|
||||
cb := NewContextBuilder(tmpDir, tmpDir)
|
||||
|
||||
// Populate cache — file does not exist yet
|
||||
sp1 := cb.BuildSystemPromptWithCache()
|
||||
|
|
@ -406,7 +406,7 @@ Original content.`
|
|||
})
|
||||
defer os.RemoveAll(tmpDir)
|
||||
|
||||
cb := NewContextBuilder(tmpDir)
|
||||
cb := NewContextBuilder(tmpDir, tmpDir)
|
||||
|
||||
// Populate cache
|
||||
sp1 := cb.BuildSystemPromptWithCache()
|
||||
|
|
@ -467,7 +467,7 @@ description: global-v1
|
|||
t.Fatal(err)
|
||||
}
|
||||
|
||||
cb := NewContextBuilder(tmpDir)
|
||||
cb := NewContextBuilder(tmpDir, tmpDir)
|
||||
sp1 := cb.BuildSystemPromptWithCache()
|
||||
if !strings.Contains(sp1, "global-v1") {
|
||||
t.Fatal("expected initial prompt to contain global skill description")
|
||||
|
|
@ -527,7 +527,7 @@ description: builtin-v1
|
|||
t.Fatal(err)
|
||||
}
|
||||
|
||||
cb := NewContextBuilder(tmpDir)
|
||||
cb := NewContextBuilder(tmpDir, tmpDir)
|
||||
sp1 := cb.BuildSystemPromptWithCache()
|
||||
if !strings.Contains(sp1, "builtin-v1") {
|
||||
t.Fatal("expected initial prompt to contain builtin skill description")
|
||||
|
|
@ -574,7 +574,7 @@ description: delete-me-v1
|
|||
})
|
||||
defer os.RemoveAll(tmpDir)
|
||||
|
||||
cb := NewContextBuilder(tmpDir)
|
||||
cb := NewContextBuilder(tmpDir, tmpDir)
|
||||
sp1 := cb.BuildSystemPromptWithCache()
|
||||
if !strings.Contains(sp1, "delete-me-v1") {
|
||||
t.Fatal("expected initial prompt to contain skill description")
|
||||
|
|
@ -614,7 +614,7 @@ func TestConcurrentBuildSystemPromptWithCache(t *testing.T) {
|
|||
})
|
||||
defer os.RemoveAll(tmpDir)
|
||||
|
||||
cb := NewContextBuilder(tmpDir)
|
||||
cb := NewContextBuilder(tmpDir, tmpDir)
|
||||
|
||||
const goroutines = 20
|
||||
const iterations = 50
|
||||
|
|
@ -677,7 +677,7 @@ func TestEmptyWorkspaceBaselineDetectsNewFiles(t *testing.T) {
|
|||
tmpDir := setupWorkspace(t, nil)
|
||||
defer os.RemoveAll(tmpDir)
|
||||
|
||||
cb := NewContextBuilder(tmpDir)
|
||||
cb := NewContextBuilder(tmpDir, tmpDir)
|
||||
|
||||
// Build cache — all tracked files are absent, maxMtime falls back to epoch.
|
||||
sp1 := cb.BuildSystemPromptWithCache()
|
||||
|
|
@ -718,7 +718,7 @@ func BenchmarkBuildMessagesWithCache(b *testing.B) {
|
|||
os.WriteFile(filepath.Join(tmpDir, name), []byte(strings.Repeat("Content.\n", 10)), 0o644)
|
||||
}
|
||||
|
||||
cb := NewContextBuilder(tmpDir)
|
||||
cb := NewContextBuilder(tmpDir, tmpDir)
|
||||
history := []providers.Message{
|
||||
{Role: "user", Content: "previous message"},
|
||||
{Role: "assistant", Content: "previous response"},
|
||||
|
|
|
|||
|
|
@ -73,7 +73,25 @@ type AgentContextDefinition struct {
|
|||
// structured files are absent, it falls back to the legacy AGENTS.md layout so
|
||||
// the current runtime can transition incrementally.
|
||||
func (cb *ContextBuilder) LoadAgentDefinition() AgentContextDefinition {
|
||||
return loadAgentDefinition(cb.workspace)
|
||||
def := loadAgentDefinition(cb.workspace)
|
||||
if def.Source == "" && cb.baseWorkspace != "" && cb.baseWorkspace != cb.workspace {
|
||||
// Fallback to base workspace if nothing found in isolated workspace
|
||||
baseDef := loadAgentDefinition(cb.baseWorkspace)
|
||||
if baseDef.Source != "" {
|
||||
// Inherit Agent and Source from base, but keep Tenant's User/Soul if they exist
|
||||
if def.Agent == nil {
|
||||
def.Agent = baseDef.Agent
|
||||
def.Source = baseDef.Source
|
||||
}
|
||||
if def.Soul == nil {
|
||||
def.Soul = baseDef.Soul
|
||||
}
|
||||
if def.User == nil {
|
||||
def.User = baseDef.User
|
||||
}
|
||||
}
|
||||
}
|
||||
return def
|
||||
}
|
||||
|
||||
func loadAgentDefinition(workspace string) AgentContextDefinition {
|
||||
|
|
|
|||
|
|
@ -34,7 +34,7 @@ Act directly and use tools first.
|
|||
})
|
||||
defer cleanupWorkspace(t, tmpDir)
|
||||
|
||||
cb := NewContextBuilder(tmpDir)
|
||||
cb := NewContextBuilder(tmpDir, tmpDir)
|
||||
definition := cb.LoadAgentDefinition()
|
||||
|
||||
if definition.Source != AgentDefinitionSourceAgent {
|
||||
|
|
@ -86,7 +86,7 @@ func TestLoadAgentDefinitionFallsBackToLegacyAgentsMarkdown(t *testing.T) {
|
|||
})
|
||||
defer cleanupWorkspace(t, tmpDir)
|
||||
|
||||
cb := NewContextBuilder(tmpDir)
|
||||
cb := NewContextBuilder(tmpDir, tmpDir)
|
||||
definition := cb.LoadAgentDefinition()
|
||||
|
||||
if definition.Source != AgentDefinitionSourceAgents {
|
||||
|
|
@ -113,7 +113,7 @@ func TestLoadAgentDefinitionLoadsWorkspaceUserMarkdown(t *testing.T) {
|
|||
})
|
||||
defer cleanupWorkspace(t, tmpDir)
|
||||
|
||||
cb := NewContextBuilder(tmpDir)
|
||||
cb := NewContextBuilder(tmpDir, tmpDir)
|
||||
definition := cb.LoadAgentDefinition()
|
||||
|
||||
if definition.User == nil {
|
||||
|
|
@ -142,7 +142,7 @@ Keep going.
|
|||
})
|
||||
defer cleanupWorkspace(t, tmpDir)
|
||||
|
||||
cb := NewContextBuilder(tmpDir)
|
||||
cb := NewContextBuilder(tmpDir, tmpDir)
|
||||
definition := cb.LoadAgentDefinition()
|
||||
|
||||
if definition.Agent == nil {
|
||||
|
|
@ -178,7 +178,7 @@ Follow the body prompt.
|
|||
})
|
||||
defer cleanupWorkspace(t, tmpDir)
|
||||
|
||||
cb := NewContextBuilder(tmpDir)
|
||||
cb := NewContextBuilder(tmpDir, tmpDir)
|
||||
bootstrap := cb.LoadBootstrapFiles()
|
||||
|
||||
if !strings.Contains(bootstrap, "Follow the body prompt") {
|
||||
|
|
@ -209,7 +209,7 @@ func TestLoadBootstrapFilesIncludesWorkspaceUserMarkdown(t *testing.T) {
|
|||
})
|
||||
defer cleanupWorkspace(t, tmpDir)
|
||||
|
||||
cb := NewContextBuilder(tmpDir)
|
||||
cb := NewContextBuilder(tmpDir, tmpDir)
|
||||
bootstrap := cb.LoadBootstrapFiles()
|
||||
|
||||
if !strings.Contains(bootstrap, "Shared profile") {
|
||||
|
|
@ -228,7 +228,7 @@ func TestStructuredAgentIgnoresIdentityChanges(t *testing.T) {
|
|||
})
|
||||
defer cleanupWorkspace(t, tmpDir)
|
||||
|
||||
cb := NewContextBuilder(tmpDir)
|
||||
cb := NewContextBuilder(tmpDir, tmpDir)
|
||||
|
||||
promptV1 := cb.BuildSystemPromptWithCache()
|
||||
if strings.Contains(promptV1, "Legacy identity") {
|
||||
|
|
@ -265,7 +265,7 @@ func TestStructuredAgentUserChangesInvalidateCache(t *testing.T) {
|
|||
})
|
||||
defer cleanupWorkspace(t, tmpDir)
|
||||
|
||||
cb := NewContextBuilder(tmpDir)
|
||||
cb := NewContextBuilder(tmpDir, tmpDir)
|
||||
|
||||
promptV1 := cb.BuildSystemPromptWithCache()
|
||||
if !strings.Contains(promptV1, "Initial workspace preferences") {
|
||||
|
|
|
|||
|
|
@ -275,7 +275,7 @@ func TestAgentLoop_EmitsSteeringAndSkippedToolEvents(t *testing.T) {
|
|||
|
||||
resultCh := make(chan string, 1)
|
||||
go func() {
|
||||
resp, _ := al.ProcessDirectWithChannel(context.Background(), "do something", "test-session", "test", "chat1")
|
||||
resp, _ := al.ProcessDirectWithChannel(context.Background(), "do something", "test-session", "test", "direct")
|
||||
resultCh <- resp
|
||||
}()
|
||||
|
||||
|
|
|
|||
|
|
@ -56,8 +56,9 @@ func NewAgentInstance(
|
|||
defaults *config.AgentDefaults,
|
||||
cfg *config.Config,
|
||||
provider providers.LLMProvider,
|
||||
isolationID string,
|
||||
) *AgentInstance {
|
||||
workspace := resolveAgentWorkspace(agentCfg, defaults)
|
||||
workspace := resolveAgentWorkspace(agentCfg, defaults, isolationID)
|
||||
os.MkdirAll(workspace, 0o755)
|
||||
|
||||
model := resolveAgentModel(agentCfg, defaults)
|
||||
|
|
@ -99,11 +100,15 @@ func NewAgentInstance(
|
|||
toolsRegistry.Register(tools.NewAppendFileTool(workspace, restrict, allowWritePaths))
|
||||
}
|
||||
|
||||
sessionsDir := filepath.Join(workspace, "sessions")
|
||||
// Use main agent workspace (no isolation) for sessions so that session history
|
||||
// persists across transient instances. The isolated workspace is only for file tools.
|
||||
mainWorkspace := resolveOriginalAgentWorkspace(agentCfg, defaults)
|
||||
sessionsDir := filepath.Join(mainWorkspace, "sessions")
|
||||
sessions := initSessionStore(sessionsDir)
|
||||
|
||||
mcpDiscoveryActive := cfg.Tools.MCP.Enabled && cfg.Tools.MCP.Discovery.Enabled
|
||||
contextBuilder := NewContextBuilder(workspace).
|
||||
baseWorkspace := mainWorkspace
|
||||
contextBuilder := NewContextBuilder(workspace, baseWorkspace).
|
||||
WithToolDiscovery(
|
||||
mcpDiscoveryActive && cfg.Tools.MCP.Discovery.UseBM25,
|
||||
mcpDiscoveryActive && cfg.Tools.MCP.Discovery.UseRegex,
|
||||
|
|
@ -211,17 +216,27 @@ func NewAgentInstance(
|
|||
}
|
||||
|
||||
// resolveAgentWorkspace determines the workspace directory for an agent.
|
||||
func resolveAgentWorkspace(agentCfg *config.AgentConfig, defaults *config.AgentDefaults) string {
|
||||
func resolveAgentWorkspace(agentCfg *config.AgentConfig, defaults *config.AgentDefaults, isolationID string) string {
|
||||
base := ""
|
||||
if agentCfg != nil && strings.TrimSpace(agentCfg.Workspace) != "" {
|
||||
return expandHome(strings.TrimSpace(agentCfg.Workspace))
|
||||
base = expandHome(strings.TrimSpace(agentCfg.Workspace))
|
||||
} else if agentCfg == nil || agentCfg.Default || agentCfg.ID == "" || routing.NormalizeAgentID(agentCfg.ID) == "main" {
|
||||
base = expandHome(defaults.Workspace)
|
||||
} else {
|
||||
// For named agents without explicit workspace, use default workspace with agent ID suffix
|
||||
id := routing.NormalizeAgentID(agentCfg.ID)
|
||||
base = filepath.Join(expandHome(defaults.Workspace), "..", "workspace-"+id)
|
||||
}
|
||||
// Use the configured default workspace (respects PICOCLAW_HOME)
|
||||
if agentCfg == nil || agentCfg.Default || agentCfg.ID == "" || routing.NormalizeAgentID(agentCfg.ID) == "main" {
|
||||
return expandHome(defaults.Workspace)
|
||||
|
||||
if isolationID != "" && isolationID != "direct" {
|
||||
return filepath.Join(base, "sessions", isolationID, "workspace")
|
||||
}
|
||||
// For named agents without explicit workspace, use default workspace with agent ID suffix
|
||||
id := routing.NormalizeAgentID(agentCfg.ID)
|
||||
return filepath.Join(expandHome(defaults.Workspace), "..", "workspace-"+id)
|
||||
return base
|
||||
}
|
||||
|
||||
// resolveOriginalAgentWorkspace determines the original workspace directory for an agent without isolation.
|
||||
func resolveOriginalAgentWorkspace(agentCfg *config.AgentConfig, defaults *config.AgentDefaults) string {
|
||||
return resolveAgentWorkspace(agentCfg, defaults, "")
|
||||
}
|
||||
|
||||
// resolveAgentModel resolves the primary model for an agent.
|
||||
|
|
|
|||
|
|
@ -33,7 +33,7 @@ func TestNewAgentInstance_UsesDefaultsTemperatureAndMaxTokens(t *testing.T) {
|
|||
cfg.Agents.Defaults.Temperature = &configuredTemp
|
||||
|
||||
provider := &mockProvider{}
|
||||
agent := NewAgentInstance(nil, &cfg.Agents.Defaults, cfg, provider)
|
||||
agent := NewAgentInstance(nil, &cfg.Agents.Defaults, cfg, provider, "")
|
||||
|
||||
if agent.MaxTokens != 1234 {
|
||||
t.Fatalf("MaxTokens = %d, want %d", agent.MaxTokens, 1234)
|
||||
|
|
@ -65,7 +65,7 @@ func TestNewAgentInstance_DefaultsTemperatureWhenZero(t *testing.T) {
|
|||
cfg.Agents.Defaults.Temperature = &configuredTemp
|
||||
|
||||
provider := &mockProvider{}
|
||||
agent := NewAgentInstance(nil, &cfg.Agents.Defaults, cfg, provider)
|
||||
agent := NewAgentInstance(nil, &cfg.Agents.Defaults, cfg, provider, "")
|
||||
|
||||
if agent.Temperature != 0.0 {
|
||||
t.Fatalf("Temperature = %f, want %f", agent.Temperature, 0.0)
|
||||
|
|
@ -91,7 +91,7 @@ func TestNewAgentInstance_DefaultsTemperatureWhenUnset(t *testing.T) {
|
|||
}
|
||||
|
||||
provider := &mockProvider{}
|
||||
agent := NewAgentInstance(nil, &cfg.Agents.Defaults, cfg, provider)
|
||||
agent := NewAgentInstance(nil, &cfg.Agents.Defaults, cfg, provider, "")
|
||||
|
||||
if agent.Temperature != 0.7 {
|
||||
t.Fatalf("Temperature = %f, want %f", agent.Temperature, 0.7)
|
||||
|
|
@ -150,7 +150,7 @@ func TestNewAgentInstance_ResolveCandidatesFromModelListAlias(t *testing.T) {
|
|||
}
|
||||
|
||||
provider := &mockProvider{}
|
||||
agent := NewAgentInstance(nil, &cfg.Agents.Defaults, cfg, provider)
|
||||
agent := NewAgentInstance(nil, &cfg.Agents.Defaults, cfg, provider, "")
|
||||
|
||||
if len(agent.Candidates) != 1 {
|
||||
t.Fatalf("len(Candidates) = %d, want 1", len(agent.Candidates))
|
||||
|
|
@ -205,7 +205,7 @@ func TestNewAgentInstance_AllowsMediaTempDirForReadListAndExec(t *testing.T) {
|
|||
},
|
||||
}
|
||||
|
||||
agent := NewAgentInstance(nil, &cfg.Agents.Defaults, cfg, &mockProvider{})
|
||||
agent := NewAgentInstance(nil, &cfg.Agents.Defaults, cfg, &mockProvider{}, "")
|
||||
|
||||
readTool, ok := agent.Tools.Get("read_file")
|
||||
if !ok {
|
||||
|
|
@ -268,7 +268,7 @@ func TestNewAgentInstance_InvalidExecConfigDoesNotExit(t *testing.T) {
|
|||
},
|
||||
}
|
||||
|
||||
agent := NewAgentInstance(nil, &cfg.Agents.Defaults, cfg, &mockProvider{})
|
||||
agent := NewAgentInstance(nil, &cfg.Agents.Defaults, cfg, &mockProvider{}, "")
|
||||
if agent == nil {
|
||||
t.Fatal("expected agent instance, got nil")
|
||||
}
|
||||
|
|
@ -281,3 +281,31 @@ func TestNewAgentInstance_InvalidExecConfigDoesNotExit(t *testing.T) {
|
|||
t.Fatal("read_file tool should still be registered")
|
||||
}
|
||||
}
|
||||
func TestNewAgentInstance_IsolatedWorkspace(t *testing.T) {
|
||||
tmpDir := t.TempDir()
|
||||
cfg := &config.Config{
|
||||
Agents: config.AgentsConfig{
|
||||
Defaults: config.AgentDefaults{
|
||||
Workspace: tmpDir,
|
||||
ModelName: "test-model",
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
isolationID := "user-123"
|
||||
agent := NewAgentInstance(nil, &cfg.Agents.Defaults, cfg, &mockProvider{}, isolationID)
|
||||
|
||||
expectedWorkspace := filepath.Join(tmpDir, "sessions", isolationID, "workspace")
|
||||
if agent.Workspace != expectedWorkspace {
|
||||
t.Fatalf("Workspace = %q, want %q", agent.Workspace, expectedWorkspace)
|
||||
}
|
||||
|
||||
// Verify the directory exists
|
||||
info, err := os.Stat(agent.Workspace)
|
||||
if err != nil {
|
||||
t.Fatalf("os.Stat(agent.Workspace) failed: %v", err)
|
||||
}
|
||||
if !info.IsDir() {
|
||||
t.Fatal("agent.Workspace is not a directory")
|
||||
}
|
||||
}
|
||||
|
|
|
|||
122
pkg/agent/isolation_tools_test.go
Normal file
122
pkg/agent/isolation_tools_test.go
Normal file
|
|
@ -0,0 +1,122 @@
|
|||
package agent
|
||||
|
||||
import (
|
||||
"context"
|
||||
"os"
|
||||
"testing"
|
||||
|
||||
"github.com/sipeed/picoclaw/pkg/bus"
|
||||
"github.com/sipeed/picoclaw/pkg/config"
|
||||
"github.com/sipeed/picoclaw/pkg/providers"
|
||||
"github.com/sipeed/picoclaw/pkg/tools"
|
||||
)
|
||||
|
||||
type isolationMockTool struct {
|
||||
name string
|
||||
}
|
||||
|
||||
func (m *isolationMockTool) Name() string { return m.name }
|
||||
func (m *isolationMockTool) Description() string { return "mock tool" }
|
||||
func (m *isolationMockTool) Parameters() map[string]any {
|
||||
return map[string]any{"type": "object", "properties": map[string]any{}}
|
||||
}
|
||||
func (m *isolationMockTool) Execute(ctx context.Context, args map[string]any) *tools.ToolResult {
|
||||
return tools.SilentResult("executed")
|
||||
}
|
||||
|
||||
func TestIsolationLacksManualTools(t *testing.T) {
|
||||
tmpDir, err := os.MkdirTemp("", "picoclaw-isolation-*")
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to create temp dir: %v", err)
|
||||
}
|
||||
defer os.RemoveAll(tmpDir)
|
||||
|
||||
cfg := &config.Config{}
|
||||
cfg.Agents.Defaults.Workspace = tmpDir
|
||||
cfg.Agents.Defaults.ModelName = "test-model"
|
||||
|
||||
msgBus := bus.NewMessageBus()
|
||||
provider := &isolationMockProvider{}
|
||||
al := NewAgentLoop(cfg, msgBus, provider)
|
||||
|
||||
tool := &isolationMockTool{name: "my_custom_tool"}
|
||||
al.RegisterTool(tool)
|
||||
|
||||
// chatID "direct" does NOT use isolation
|
||||
resp, err := al.ProcessDirectWithChannel(context.Background(), "hello", "session1", "cli", "direct")
|
||||
if err != nil {
|
||||
t.Errorf("ProcessDirectWithChannel failed: %v", err)
|
||||
}
|
||||
if resp != "Found tool" {
|
||||
t.Errorf("Direct response: %s, want Found tool", resp)
|
||||
}
|
||||
|
||||
// chatID "chat1" DOES use isolation - transient agent instance is created
|
||||
resp, err = al.ProcessDirectWithChannel(context.Background(), "hello", "session1", "cli", "chat1")
|
||||
if err != nil {
|
||||
t.Errorf("ProcessDirectWithChannel (isolated) failed: %v", err)
|
||||
}
|
||||
if resp != "Found tool" {
|
||||
t.Errorf("Isolated response: %s, want Found tool (fixed)", resp)
|
||||
}
|
||||
}
|
||||
|
||||
func TestManualToolsPreservedAfterReload(t *testing.T) {
|
||||
tmpDir, err := os.MkdirTemp("", "picoclaw-reload-*")
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to create temp dir: %v", err)
|
||||
}
|
||||
defer os.RemoveAll(tmpDir)
|
||||
|
||||
cfg := &config.Config{}
|
||||
cfg.Agents.Defaults.Workspace = tmpDir
|
||||
cfg.Agents.Defaults.ModelName = "test-model"
|
||||
|
||||
msgBus := bus.NewMessageBus()
|
||||
provider := &isolationMockProvider{}
|
||||
al := NewAgentLoop(cfg, msgBus, provider)
|
||||
|
||||
tool := &isolationMockTool{name: "my_custom_tool"}
|
||||
al.RegisterTool(tool)
|
||||
|
||||
// Reload with same config and provider - should preserve manual tools
|
||||
err = al.ReloadProviderAndConfig(context.Background(), provider, cfg)
|
||||
if err != nil {
|
||||
t.Fatalf("Reload failed: %v", err)
|
||||
}
|
||||
|
||||
// Check if tool is still visible to the new registry
|
||||
resp, err := al.ProcessDirectWithChannel(context.Background(), "hello", "session1", "cli", "direct")
|
||||
if err != nil {
|
||||
t.Errorf("ProcessDirectWithChannel failed: %v", err)
|
||||
}
|
||||
if resp != "Found tool" {
|
||||
t.Errorf("Response after reload: %s, want Found tool", resp)
|
||||
}
|
||||
}
|
||||
|
||||
type isolationMockProvider struct{}
|
||||
|
||||
func (m *isolationMockProvider) Chat(
|
||||
ctx context.Context,
|
||||
msgs []providers.Message,
|
||||
tools []providers.ToolDefinition,
|
||||
model string,
|
||||
opts map[string]any,
|
||||
) (*providers.LLMResponse, error) {
|
||||
found := false
|
||||
for _, t := range tools {
|
||||
if t.Function.Name == "my_custom_tool" {
|
||||
found = true
|
||||
break
|
||||
}
|
||||
}
|
||||
if found {
|
||||
return &providers.LLMResponse{Content: "Found tool"}, nil
|
||||
}
|
||||
return &providers.LLMResponse{Content: "Tool NOT found"}, nil
|
||||
}
|
||||
|
||||
func (m *isolationMockProvider) GetDefaultModel() string {
|
||||
return "mock"
|
||||
}
|
||||
|
|
@ -58,11 +58,20 @@ type AgentLoop struct {
|
|||
steering *steeringQueue
|
||||
pendingSkills sync.Map
|
||||
mu sync.RWMutex
|
||||
manualTools []tools.Tool
|
||||
|
||||
// Concurrent turn management (from HEAD)
|
||||
activeTurnStates sync.Map // key: sessionKey (string), value: *turnState
|
||||
subTurnCounter atomic.Int64 // Counter for generating unique SubTurn IDs
|
||||
|
||||
// Agent instance caching for multi-user isolation
|
||||
// Each unique chatID gets its own agent instance to maintain state/model selection
|
||||
agentCache sync.Map // key: channel:chatID, value: *AgentInstance
|
||||
agentCacheMu sync.RWMutex
|
||||
agentCacheTTL time.Duration // How long to keep cached agents alive
|
||||
agentCleaner *time.Ticker // Periodic cleanup of stale cached agents
|
||||
lastCacheCheck sync.Map // key: channel:chatID, value: time.Time (last access time)
|
||||
|
||||
// Turn tracking (from Incoming)
|
||||
turnSeq atomic.Uint64
|
||||
activeRequests sync.WaitGroup
|
||||
|
|
@ -100,7 +109,7 @@ const (
|
|||
defaultResponse = "The model returned an empty response. This may indicate a provider error or token limit."
|
||||
toolLimitResponse = "I've reached `max_tool_iterations` without a final response. Increase `max_tool_iterations` in config.json if this task needs more tool steps."
|
||||
handledToolResponseSummary = "Requested output delivered via tool attachment."
|
||||
sessionKeyAgentPrefix = "agent:"
|
||||
sessionKeyAgentPrefix = "agent::"
|
||||
metadataKeyAccountID = "account_id"
|
||||
metadataKeyGuildID = "guild_id"
|
||||
metadataKeyTeamID = "team_id"
|
||||
|
|
@ -163,6 +172,13 @@ func registerSharedTools(
|
|||
continue
|
||||
}
|
||||
|
||||
// Re-register manual tools first so they can be overwritten by core shared tools if needed
|
||||
al.mu.RLock()
|
||||
for _, tool := range al.manualTools {
|
||||
agent.Tools.Register(tool)
|
||||
}
|
||||
al.mu.RUnlock()
|
||||
|
||||
if cfg.Tools.IsToolEnabled("web") {
|
||||
searchTool, err := tools.NewWebSearchTool(tools.WebSearchToolOptions{
|
||||
BraveAPIKeys: cfg.Tools.Web.Brave.APIKeys.Values(),
|
||||
|
|
@ -664,7 +680,7 @@ func (al *AgentLoop) buildContinuationTarget(msg bus.InboundMessage) (*continuat
|
|||
}
|
||||
|
||||
return &continuationTarget{
|
||||
SessionKey: resolveScopeKey(route, msg.SessionKey),
|
||||
SessionKey: resolveScopeKey(route, msg.SessionKey, msg.ChatID, route.AgentID),
|
||||
Channel: msg.Channel,
|
||||
ChatID: msg.ChatID,
|
||||
}, nil
|
||||
|
|
@ -919,6 +935,21 @@ func (al *AgentLoop) RegisterTool(tool tools.Tool) {
|
|||
agent.Tools.Register(tool)
|
||||
}
|
||||
}
|
||||
|
||||
al.mu.Lock()
|
||||
defer al.mu.Unlock()
|
||||
// Check for duplicates by name and overwrite
|
||||
found := false
|
||||
for i, t := range al.manualTools {
|
||||
if t.Name() == tool.Name() {
|
||||
al.manualTools[i] = tool
|
||||
found = true
|
||||
break
|
||||
}
|
||||
}
|
||||
if !found {
|
||||
al.manualTools = append(al.manualTools, tool)
|
||||
}
|
||||
}
|
||||
|
||||
func (al *AgentLoop) SetChannelManager(cm *channels.Manager) {
|
||||
|
|
@ -1298,11 +1329,63 @@ func (al *AgentLoop) processMessage(ctx context.Context, msg bus.InboundMessage)
|
|||
return al.processSystemMessage(ctx, msg)
|
||||
}
|
||||
|
||||
route, agent, routeErr := al.resolveMessageRoute(msg)
|
||||
route, baseAgent, routeErr := al.resolveMessageRoute(msg)
|
||||
if routeErr != nil {
|
||||
return "", routeErr
|
||||
}
|
||||
|
||||
agent := baseAgent
|
||||
isolationID := msg.ChatID
|
||||
if isolationID != "" && isolationID != "direct" {
|
||||
// Check agent instance cache first (keyed by channel:chatID)
|
||||
cacheKey := msg.Channel + ":" + isolationID
|
||||
if cached, ok := al.agentCache.Load(cacheKey); ok {
|
||||
agent = cached.(*AgentInstance)
|
||||
// Update last access time for TTL tracking
|
||||
al.lastCacheCheck.Store(cacheKey, time.Now())
|
||||
|
||||
logger.InfoCF("agent", "Reusing cached agent instance", map[string]any{
|
||||
"agent_id": agent.ID,
|
||||
"cache_key": cacheKey,
|
||||
"isolation_id": isolationID,
|
||||
})
|
||||
} else {
|
||||
// Create a transient isolated instance for this chat session
|
||||
// This ensures workspace, memory, and sessions are private to the chat_id.
|
||||
|
||||
// Determine the original config for this agent to preserve its specialized prompt/skills
|
||||
var ac *config.AgentConfig
|
||||
for i := range al.cfg.Agents.List {
|
||||
if routing.NormalizeAgentID(al.cfg.Agents.List[i].ID) == route.AgentID {
|
||||
ac = &al.cfg.Agents.List[i]
|
||||
break
|
||||
}
|
||||
}
|
||||
|
||||
// Create a new instance with the isolationID
|
||||
// NewAgentInstance uses isolationID to sub-path the workspace
|
||||
agent = NewAgentInstance(ac, &al.cfg.Agents.Defaults, al.cfg, baseAgent.Provider, isolationID)
|
||||
|
||||
// Set its ID to match the routed agent so prompts and logs match
|
||||
agent.ID = route.AgentID
|
||||
|
||||
// Re-register shared tools (web, message, spawn) to this transient agent
|
||||
// We pass a mini-registry containing only this agent
|
||||
registerSharedTools(al, al.cfg, al.bus, &AgentRegistry{agents: map[string]*AgentInstance{agent.ID: agent}}, baseAgent.Provider)
|
||||
|
||||
// Cache this agent instance per chat session
|
||||
al.agentCache.Store(cacheKey, agent)
|
||||
al.lastCacheCheck.Store(cacheKey, time.Now())
|
||||
|
||||
logger.InfoCF("agent", "Created isolated transient agent", map[string]any{
|
||||
"agent_id": agent.ID,
|
||||
"cache_key": cacheKey,
|
||||
"isolation_id": isolationID,
|
||||
"workspace": agent.Workspace,
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// Reset message-tool state for this round so we don't skip publishing due to a previous round.
|
||||
if tool, ok := agent.Tools.Get("message"); ok {
|
||||
if resetter, ok := tool.(interface{ ResetSentInRound() }); ok {
|
||||
|
|
@ -1311,7 +1394,8 @@ func (al *AgentLoop) processMessage(ctx context.Context, msg bus.InboundMessage)
|
|||
}
|
||||
|
||||
// Resolve session key from route, while preserving explicit agent-scoped keys.
|
||||
scopeKey := resolveScopeKey(route, msg.SessionKey)
|
||||
// If caller provides a session key, respect it. Otherwise, derive from chatID for isolation.
|
||||
scopeKey := resolveScopeKey(route, msg.SessionKey, msg.ChatID, agent.ID)
|
||||
sessionKey := scopeKey
|
||||
|
||||
logger.InfoCF("agent", "Routed message",
|
||||
|
|
@ -1377,10 +1461,19 @@ func (al *AgentLoop) resolveMessageRoute(msg bus.InboundMessage) (routing.Resolv
|
|||
return route, agent, nil
|
||||
}
|
||||
|
||||
func resolveScopeKey(route routing.ResolvedRoute, msgSessionKey string) string {
|
||||
func resolveScopeKey(route routing.ResolvedRoute, msgSessionKey, chatID, agentID string) string {
|
||||
// 1. If caller explicitly provides a session key with agent prefix, use it as-is
|
||||
if msgSessionKey != "" && strings.HasPrefix(msgSessionKey, sessionKeyAgentPrefix) {
|
||||
return msgSessionKey
|
||||
}
|
||||
|
||||
// 2. If a unique chatID is provided, use it to create an isolated session per chat
|
||||
// This ensures each Teams conversation (or any unique chat) has separate session history
|
||||
if chatID != "" && chatID != "direct" {
|
||||
return fmt.Sprintf("%s:%s:%s", sessionKeyAgentPrefix, agentID, chatID)
|
||||
}
|
||||
|
||||
// 3. Fall back to route's default session key
|
||||
return route.SessionKey
|
||||
}
|
||||
|
||||
|
|
@ -1394,7 +1487,7 @@ func (al *AgentLoop) resolveSteeringTarget(msg bus.InboundMessage) (string, stri
|
|||
return "", "", false
|
||||
}
|
||||
|
||||
return resolveScopeKey(route, msg.SessionKey), agent.ID, true
|
||||
return resolveScopeKey(route, msg.SessionKey, msg.ChatID, agent.ID), agent.ID, true
|
||||
}
|
||||
|
||||
func (al *AgentLoop) requeueInboundMessage(msg bus.InboundMessage) error {
|
||||
|
|
|
|||
|
|
@ -64,7 +64,7 @@ func (al *AgentLoop) EnsureMCPInitialized(ctx context.Context) error {
|
|||
return nil
|
||||
}
|
||||
|
||||
if al.cfg.Tools.MCP.Servers == nil || len(al.cfg.Tools.MCP.Servers) == 0 {
|
||||
if len(al.cfg.Tools.MCP.Servers) == 0 {
|
||||
logger.WarnCF("agent", "MCP is enabled but no servers are configured, skipping MCP initialization", nil)
|
||||
return nil
|
||||
}
|
||||
|
|
|
|||
|
|
@ -654,7 +654,7 @@ func TestProcessMessage_MediaToolHandledSkipsFollowUpLLMAndFinalText(t *testing.
|
|||
if err != nil {
|
||||
t.Fatalf("resolveMessageRoute() error = %v", err)
|
||||
}
|
||||
sessionKey := resolveScopeKey(route, "")
|
||||
sessionKey := resolveScopeKey(route, "", "chat1", route.AgentID)
|
||||
history := defaultAgent.Sessions.GetHistory(sessionKey)
|
||||
if len(history) == 0 {
|
||||
t.Fatal("expected session history to be saved")
|
||||
|
|
@ -1343,11 +1343,8 @@ func TestProcessMessage_UsesRouteSessionKey(t *testing.T) {
|
|||
},
|
||||
}
|
||||
|
||||
route := al.registry.ResolveRoute(routing.RouteInput{
|
||||
Channel: msg.Channel,
|
||||
Peer: extractPeer(msg),
|
||||
})
|
||||
sessionKey := route.SessionKey
|
||||
// With chatID isolation, session key is derived from chatID
|
||||
sessionKey := fmt.Sprintf("agent:::main:%s", msg.ChatID)
|
||||
|
||||
defaultAgent := al.registry.GetDefaultAgent()
|
||||
if defaultAgent == nil {
|
||||
|
|
@ -1945,7 +1942,7 @@ func TestAgentLoop_ToolLimitUsesDedicatedFallback(t *testing.T) {
|
|||
al := NewAgentLoop(cfg, msgBus, provider)
|
||||
al.RegisterTool(&toolLimitTestTool{})
|
||||
|
||||
response, err := al.ProcessDirectWithChannel(context.Background(), "hello", "tool-limit", "test", "chat1")
|
||||
response, err := al.ProcessDirectWithChannel(context.Background(), "hello", "tool-limit", "test", "direct")
|
||||
if err != nil {
|
||||
t.Fatalf("ProcessDirectWithChannel failed: %v", err)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -33,14 +33,15 @@ func NewAgentRegistry(
|
|||
ID: "main",
|
||||
Default: true,
|
||||
}
|
||||
instance := NewAgentInstance(implicitAgent, &cfg.Agents.Defaults, cfg, provider)
|
||||
instance := NewAgentInstance(implicitAgent, &cfg.Agents.Defaults, cfg, provider, "")
|
||||
registry.agents["main"] = instance
|
||||
logger.InfoCF("agent", "Created implicit main agent (no agents.list configured)", nil)
|
||||
} else {
|
||||
for i := range agentConfigs {
|
||||
ac := &agentConfigs[i]
|
||||
id := routing.NormalizeAgentID(ac.ID)
|
||||
instance := NewAgentInstance(ac, &cfg.Agents.Defaults, cfg, provider)
|
||||
instance := NewAgentInstance(ac, &cfg.Agents.Defaults, cfg, provider, "")
|
||||
|
||||
registry.agents[id] = instance
|
||||
logger.InfoCF("agent", "Registered agent",
|
||||
map[string]any{
|
||||
|
|
|
|||
|
|
@ -298,7 +298,7 @@ func TestAgentLoop_Continue_NoMessages(t *testing.T) {
|
|||
t.Fatal("expected provider to be initialized")
|
||||
}
|
||||
|
||||
resp, err := al.Continue(context.Background(), "test-session", "test", "chat1")
|
||||
resp, err := al.Continue(context.Background(), "test-session", "test", "direct")
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
|
|
@ -331,7 +331,7 @@ func TestAgentLoop_Continue_WithMessages(t *testing.T) {
|
|||
|
||||
al.Steer(providers.Message{Role: "user", Content: "new direction"})
|
||||
|
||||
resp, err := al.Continue(context.Background(), "test-session", "test", "chat1")
|
||||
resp, err := al.Continue(context.Background(), "test-session", "test", "direct")
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
|
|
@ -367,7 +367,7 @@ func TestDrainBusToSteering_RequeuesDifferentScopeMessage(t *testing.T) {
|
|||
activeMsg := bus.InboundMessage{
|
||||
Channel: "telegram",
|
||||
SenderID: "user1",
|
||||
ChatID: "chat1",
|
||||
ChatID: "direct",
|
||||
Content: "active turn",
|
||||
Peer: bus.Peer{
|
||||
Kind: "direct",
|
||||
|
|
@ -701,7 +701,7 @@ func TestAgentLoop_Steering_SkipsRemainingTools(t *testing.T) {
|
|||
"do something",
|
||||
"test-session",
|
||||
"test",
|
||||
"chat1",
|
||||
"direct",
|
||||
)
|
||||
resultCh <- result{resp, err}
|
||||
}()
|
||||
|
|
@ -783,7 +783,7 @@ func TestAgentLoop_Steering_InitialPoll(t *testing.T) {
|
|||
"initial message",
|
||||
"test-session",
|
||||
"test",
|
||||
"chat1",
|
||||
"direct",
|
||||
)
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
|
|
@ -843,7 +843,7 @@ func TestAgentLoop_Run_AutoContinuesLateSteeringMessage(t *testing.T) {
|
|||
first := bus.InboundMessage{
|
||||
Channel: "test",
|
||||
SenderID: "user1",
|
||||
ChatID: "chat1",
|
||||
ChatID: "direct",
|
||||
Content: "first message",
|
||||
Peer: bus.Peer{
|
||||
Kind: "direct",
|
||||
|
|
@ -853,7 +853,7 @@ func TestAgentLoop_Run_AutoContinuesLateSteeringMessage(t *testing.T) {
|
|||
late := bus.InboundMessage{
|
||||
Channel: "test",
|
||||
SenderID: "user1",
|
||||
ChatID: "chat1",
|
||||
ChatID: "direct",
|
||||
Content: "late append",
|
||||
Peer: bus.Peer{
|
||||
Kind: "direct",
|
||||
|
|
@ -970,7 +970,7 @@ func TestAgentLoop_Steering_DirectResponseContinuesWithQueuedMessage(t *testing.
|
|||
"initial request",
|
||||
sessionKey,
|
||||
"test",
|
||||
"chat1",
|
||||
"direct",
|
||||
)
|
||||
resultCh <- struct {
|
||||
resp string
|
||||
|
|
@ -1073,7 +1073,7 @@ func TestAgentLoop_Continue_PreservesSteeringMedia(t *testing.T) {
|
|||
t.Fatalf("Steer failed: %v", err)
|
||||
}
|
||||
|
||||
resp, err := al.Continue(context.Background(), sessionKey, "test", "chat1")
|
||||
resp, err := al.Continue(context.Background(), sessionKey, "test", "direct")
|
||||
if err != nil {
|
||||
t.Fatalf("Continue failed: %v", err)
|
||||
}
|
||||
|
|
@ -1184,7 +1184,7 @@ func TestAgentLoop_InterruptGraceful_UsesTerminalNoToolCall(t *testing.T) {
|
|||
"do something",
|
||||
sessionKey,
|
||||
"test",
|
||||
"chat1",
|
||||
"direct",
|
||||
)
|
||||
resultCh <- result{resp: resp, err: err}
|
||||
}()
|
||||
|
|
@ -1202,7 +1202,7 @@ func TestAgentLoop_InterruptGraceful_UsesTerminalNoToolCall(t *testing.T) {
|
|||
if active.SessionKey != sessionKey {
|
||||
t.Fatalf("expected active session %q, got %q", sessionKey, active.SessionKey)
|
||||
}
|
||||
if active.Channel != "test" || active.ChatID != "chat1" {
|
||||
if active.Channel != "test" || active.ChatID != "direct" {
|
||||
t.Fatalf("unexpected active turn target: %#v", active)
|
||||
}
|
||||
|
||||
|
|
@ -1349,7 +1349,7 @@ func TestAgentLoop_InterruptHard_RestoresSession(t *testing.T) {
|
|||
"do work",
|
||||
sessionKey,
|
||||
"test",
|
||||
"chat1",
|
||||
"direct",
|
||||
)
|
||||
resultCh <- result{resp: resp, err: err}
|
||||
}()
|
||||
|
|
@ -1518,7 +1518,7 @@ func TestAgentLoop_Steering_SkippedToolsHaveErrorResults(t *testing.T) {
|
|||
resultCh := make(chan string, 1)
|
||||
go func() {
|
||||
resp, _ := al.ProcessDirectWithChannel(
|
||||
context.Background(), "go", "test-session", "test", "chat1",
|
||||
context.Background(), "go", "test-session", "test", "direct",
|
||||
)
|
||||
resultCh <- resp
|
||||
}()
|
||||
|
|
|
|||
|
|
@ -160,11 +160,14 @@ func Run(debug bool, homePath, configPath string, allowEmptyStartup bool) error
|
|||
|
||||
// Setup synchronous /chat endpoint handler
|
||||
if cfg.Gateway.ChatEnabled {
|
||||
runningServices.HealthServer.SetChatFunc(func(ctx context.Context, message, sessionID string) (string, error) {
|
||||
runningServices.HealthServer.SetChatFunc(func(ctx context.Context, message, sessionID, chatID string) (string, error) {
|
||||
if sessionID == "" {
|
||||
sessionID = "http-chat"
|
||||
}
|
||||
return agentLoop.ProcessDirectWithChannel(ctx, message, sessionID, "http", "chat")
|
||||
if chatID == "" {
|
||||
chatID = "chat"
|
||||
}
|
||||
return agentLoop.ProcessDirectWithChannel(ctx, message, sessionID, "http", chatID)
|
||||
})
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -17,6 +17,7 @@ import (
|
|||
type ChatRequest struct {
|
||||
Message string `json:"message"`
|
||||
SessionID string `json:"session_id,omitempty"`
|
||||
ChatID string `json:"chat_id,omitempty"` // Alias for session_id to match PicoClaw terminology
|
||||
}
|
||||
|
||||
// ChatResponse is the JSON response from /chat.
|
||||
|
|
@ -41,7 +42,7 @@ type Server struct {
|
|||
checks map[string]Check
|
||||
startTime time.Time
|
||||
reloadFunc func() error
|
||||
chatFunc func(ctx context.Context, message, sessionID string) (string, error)
|
||||
chatFunc func(ctx context.Context, message, sessionID, chatID string) (string, error)
|
||||
apiKey string
|
||||
chatResults map[string]*chatStatus
|
||||
chatResultsMu sync.RWMutex
|
||||
|
|
@ -153,7 +154,7 @@ func (s *Server) SetReloadFunc(fn func() error) {
|
|||
// fn receives the user message and an optional session ID and must return the
|
||||
// agent's reply (or an error). It is called synchronously inside the HTTP
|
||||
// handler, so the write timeout on the server governs the maximum duration.
|
||||
func (s *Server) SetChatFunc(fn func(ctx context.Context, message, sessionID string) (string, error)) {
|
||||
func (s *Server) SetChatFunc(fn func(ctx context.Context, message, sessionID, chatID string) (string, error)) {
|
||||
s.mu.Lock()
|
||||
defer s.mu.Unlock()
|
||||
s.chatFunc = fn
|
||||
|
|
@ -331,6 +332,56 @@ func (s *Server) handlePostChat(w http.ResponseWriter, r *http.Request) {
|
|||
}
|
||||
|
||||
sessionID := req.SessionID
|
||||
if sessionID == "" && req.ChatID != "" {
|
||||
sessionID = req.ChatID
|
||||
}
|
||||
|
||||
chatID := req.ChatID
|
||||
if chatID == "" {
|
||||
// Try to extract ChatID/TenantID from common headers
|
||||
// These are ordered by specificity/reliability
|
||||
headers := []string{
|
||||
"X-PicoClaw-Chat-ID",
|
||||
"X-User-ID",
|
||||
"X-Session-ID",
|
||||
"X-MS-CLIENT-PRINCIPAL-ID", // Azure App Service / Container Apps (EasyAuth)
|
||||
"X-MS-CLIENT-PRINCIPAL-NAME", // Azure App Service Email/Username
|
||||
"Ocp-Apim-Subscription-Id", // Azure APIM (if configured)
|
||||
}
|
||||
|
||||
for _, h := range headers {
|
||||
if val := r.Header.Get(h); val != "" {
|
||||
chatID = val
|
||||
break
|
||||
}
|
||||
}
|
||||
|
||||
// Fallback to SessionID if provided in body, otherwise empty (global)
|
||||
if chatID == "" {
|
||||
chatID = req.SessionID
|
||||
}
|
||||
}
|
||||
|
||||
if chatID != "" {
|
||||
logger.InfoCF("api", "Resolved isolation ID for request", map[string]any{
|
||||
"chat_id": chatID,
|
||||
"session_id": sessionID,
|
||||
})
|
||||
} else {
|
||||
// Log all headers for debugging (excluding sensitive ones)
|
||||
headers := make(map[string]string)
|
||||
for k, v := range r.Header {
|
||||
if k == "Authorization" || k == "X-Api-Key" || k == "Ocp-Apim-Subscription-Key" {
|
||||
headers[k] = "REDACTED"
|
||||
} else if len(v) > 0 {
|
||||
headers[k] = v[0]
|
||||
}
|
||||
}
|
||||
logger.DebugCF("api", "Chat request received without explicit ChatID. Checking headers...", map[string]any{
|
||||
"headers": headers,
|
||||
})
|
||||
}
|
||||
|
||||
if sessionID == "" {
|
||||
sessionID = fmt.Sprintf("chat-%d", time.Now().UnixNano())
|
||||
}
|
||||
|
|
@ -348,7 +399,7 @@ func (s *Server) handlePostChat(w http.ResponseWriter, r *http.Request) {
|
|||
// which will be cancelled when this request finishes.
|
||||
ctx := context.Background()
|
||||
logger.Debugf("Starting async chat for session %s", sessionID)
|
||||
reply, err := chatFunc(ctx, req.Message, sessionID)
|
||||
reply, err := chatFunc(ctx, req.Message, sessionID, chatID)
|
||||
|
||||
s.chatResultsMu.Lock()
|
||||
defer s.chatResultsMu.Unlock()
|
||||
|
|
|
|||
|
|
@ -59,18 +59,19 @@ func (info SkillInfo) validate() error {
|
|||
}
|
||||
|
||||
type SkillsLoader struct {
|
||||
workspace string
|
||||
workspaceSkills string // workspace skills (project-level)
|
||||
globalSkills string // global skills (~/.picoclaw/skills)
|
||||
builtinSkills string // builtin skills
|
||||
whitelist []string
|
||||
whitelistEnabled bool
|
||||
workspace string
|
||||
workspaceSkills string // workspace skills (project-level)
|
||||
baseWorkspaceSkills string // fallback workspace skills (if isolated)
|
||||
globalSkills string // global skills (~/.picoclaw/skills)
|
||||
builtinSkills string // builtin skills
|
||||
whitelist []string
|
||||
whitelistEnabled bool
|
||||
}
|
||||
|
||||
// SkillRoots returns all unique skill root directories used by this loader.
|
||||
// The order follows resolution priority: workspace > global > builtin.
|
||||
func (sl *SkillsLoader) SkillRoots() []string {
|
||||
roots := []string{sl.workspaceSkills, sl.globalSkills, sl.builtinSkills}
|
||||
roots := []string{sl.workspaceSkills, sl.baseWorkspaceSkills, sl.globalSkills, sl.builtinSkills}
|
||||
seen := make(map[string]struct{}, len(roots))
|
||||
out := make([]string, 0, len(roots))
|
||||
|
||||
|
|
@ -92,18 +93,20 @@ func (sl *SkillsLoader) SkillRoots() []string {
|
|||
|
||||
func NewSkillsLoader(
|
||||
workspace string,
|
||||
baseWorkspace string,
|
||||
globalSkills string,
|
||||
builtinSkills string,
|
||||
whitelist []string,
|
||||
whitelistEnabled bool,
|
||||
) *SkillsLoader {
|
||||
return &SkillsLoader{
|
||||
workspace: workspace,
|
||||
workspaceSkills: filepath.Join(workspace, "skills"),
|
||||
globalSkills: globalSkills, // ~/.picoclaw/skills
|
||||
builtinSkills: builtinSkills,
|
||||
whitelist: whitelist,
|
||||
whitelistEnabled: whitelistEnabled,
|
||||
workspace: workspace,
|
||||
workspaceSkills: filepath.Join(workspace, "skills"),
|
||||
baseWorkspaceSkills: filepath.Join(baseWorkspace, "skills"),
|
||||
globalSkills: globalSkills, // ~/.picoclaw/skills
|
||||
builtinSkills: builtinSkills,
|
||||
whitelist: whitelist,
|
||||
whitelistEnabled: whitelistEnabled,
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -173,8 +176,9 @@ func (sl *SkillsLoader) ListSkills() []SkillInfo {
|
|||
}
|
||||
}
|
||||
|
||||
// Priority: workspace > global > builtin
|
||||
// Priority: workspace > base workspace > global > builtin
|
||||
addSkills(sl.workspaceSkills, "workspace")
|
||||
addSkills(sl.baseWorkspaceSkills, "shared")
|
||||
addSkills(sl.globalSkills, "global")
|
||||
addSkills(sl.builtinSkills, "builtin")
|
||||
|
||||
|
|
@ -204,6 +208,14 @@ func (sl *SkillsLoader) LoadSkill(name string) (string, bool) {
|
|||
}
|
||||
// ...
|
||||
|
||||
// 1b. load from base workspace skills (fallback if isolated)
|
||||
if sl.baseWorkspaceSkills != "" && sl.baseWorkspaceSkills != sl.workspaceSkills {
|
||||
skillFile := filepath.Join(sl.baseWorkspaceSkills, name, "SKILL.md")
|
||||
if content, err := os.ReadFile(skillFile); err == nil {
|
||||
return sl.stripFrontmatter(string(content)), true
|
||||
}
|
||||
}
|
||||
|
||||
// 2. then load from global skills (~/.picoclaw/skills)
|
||||
if sl.globalSkills != "" {
|
||||
skillFile := filepath.Join(sl.globalSkills, name, "SKILL.md")
|
||||
|
|
|
|||
|
|
@ -155,7 +155,7 @@ func TestListSkillsWorkspaceOverridesGlobal(t *testing.T) {
|
|||
createSkillDir(t, filepath.Join(ws, "skills"), "my-skill", "my-skill", "workspace version")
|
||||
createSkillDir(t, global, "my-skill", "my-skill", "global version")
|
||||
|
||||
sl := NewSkillsLoader(ws, global, "", nil, false)
|
||||
sl := NewSkillsLoader(ws, ws, global, "", nil, false)
|
||||
skills := sl.ListSkills()
|
||||
|
||||
assert.Len(t, skills, 1)
|
||||
|
|
@ -172,7 +172,7 @@ func TestListSkillsGlobalOverridesBuiltin(t *testing.T) {
|
|||
createSkillDir(t, global, "my-skill", "my-skill", "global version")
|
||||
createSkillDir(t, builtin, "my-skill", "my-skill", "builtin version")
|
||||
|
||||
sl := NewSkillsLoader(ws, global, builtin, nil, false)
|
||||
sl := NewSkillsLoader(ws, ws, global, builtin, nil, false)
|
||||
skills := sl.ListSkills()
|
||||
|
||||
assert.Len(t, skills, 1)
|
||||
|
|
@ -189,7 +189,7 @@ func TestListSkillsMetadataNameDedup(t *testing.T) {
|
|||
createSkillDir(t, filepath.Join(ws, "skills"), "dir-a", "shared-name", "workspace version")
|
||||
createSkillDir(t, global, "dir-b", "shared-name", "global version")
|
||||
|
||||
sl := NewSkillsLoader(ws, global, "", nil, false)
|
||||
sl := NewSkillsLoader(ws, ws, global, "", nil, false)
|
||||
skills := sl.ListSkills()
|
||||
|
||||
assert.Len(t, skills, 1)
|
||||
|
|
@ -207,7 +207,7 @@ func TestListSkillsMultipleDistinctSkills(t *testing.T) {
|
|||
createSkillDir(t, global, "skill-b", "skill-b", "desc b")
|
||||
createSkillDir(t, builtin, "skill-c", "skill-c", "desc c")
|
||||
|
||||
sl := NewSkillsLoader(ws, global, builtin, nil, false)
|
||||
sl := NewSkillsLoader(ws, ws, global, builtin, nil, false)
|
||||
skills := sl.ListSkills()
|
||||
|
||||
assert.Len(t, skills, 3)
|
||||
|
|
@ -230,7 +230,7 @@ func TestListSkillsInvalidSkillSkipped(t *testing.T) {
|
|||
// Valid skill
|
||||
createSkillDir(t, global, "good-skill", "good-skill", "desc")
|
||||
|
||||
sl := NewSkillsLoader(ws, global, "", nil, false)
|
||||
sl := NewSkillsLoader(ws, ws, global, "", nil, false)
|
||||
skills := sl.ListSkills()
|
||||
|
||||
assert.Len(t, skills, 1)
|
||||
|
|
@ -243,7 +243,7 @@ func TestListSkillsEmptyAndNonexistentDirs(t *testing.T) {
|
|||
emptyDir := filepath.Join(tmp, "empty")
|
||||
require.NoError(t, os.MkdirAll(emptyDir, 0o755))
|
||||
|
||||
sl := NewSkillsLoader(ws, emptyDir, filepath.Join(tmp, "nonexistent"), nil, false)
|
||||
sl := NewSkillsLoader(ws, ws, emptyDir, filepath.Join(tmp, "nonexistent"), nil, false)
|
||||
skills := sl.ListSkills()
|
||||
|
||||
assert.Empty(t, skills)
|
||||
|
|
@ -259,7 +259,7 @@ func TestListSkillsDirWithoutSkillMD(t *testing.T) {
|
|||
// Valid skill alongside
|
||||
createSkillDir(t, global, "real-skill", "real-skill", "desc")
|
||||
|
||||
sl := NewSkillsLoader(ws, global, "", nil, false)
|
||||
sl := NewSkillsLoader(ws, ws, global, "", nil, false)
|
||||
skills := sl.ListSkills()
|
||||
|
||||
assert.Len(t, skills, 1)
|
||||
|
|
@ -333,7 +333,7 @@ func TestSkillRootsTrimsWhitespaceAndDedups(t *testing.T) {
|
|||
global := filepath.Join(tmp, "global")
|
||||
builtin := filepath.Join(tmp, "builtin")
|
||||
|
||||
sl := NewSkillsLoader(workspace, " "+global+" ", "\t"+builtin+"\n", nil, false)
|
||||
sl := NewSkillsLoader(workspace, workspace, " "+global+" ", "\t"+builtin+"\n", nil, false)
|
||||
roots := sl.SkillRoots()
|
||||
|
||||
assert.Equal(t, []string{
|
||||
|
|
@ -429,14 +429,14 @@ func TestListSkillsWithWhitelist(t *testing.T) {
|
|||
createSkillDir(t, builtin, "skill-c", "skill-c", "desc c")
|
||||
|
||||
t.Run("allow-one", func(t *testing.T) {
|
||||
sl := NewSkillsLoader(ws, global, builtin, []string{"skill-a"}, true)
|
||||
sl := NewSkillsLoader(ws, ws, global, builtin, []string{"skill-a"}, true)
|
||||
skills := sl.ListSkills()
|
||||
assert.Len(t, skills, 1)
|
||||
assert.Equal(t, "skill-a", skills[0].Name)
|
||||
})
|
||||
|
||||
t.Run("allow-two", func(t *testing.T) {
|
||||
sl := NewSkillsLoader(ws, global, builtin, []string{"skill-a", "skill-c"}, true)
|
||||
sl := NewSkillsLoader(ws, ws, global, builtin, []string{"skill-a", "skill-c"}, true)
|
||||
skills := sl.ListSkills()
|
||||
assert.Len(t, skills, 2)
|
||||
names := []string{skills[0].Name, skills[1].Name}
|
||||
|
|
@ -445,19 +445,19 @@ func TestListSkillsWithWhitelist(t *testing.T) {
|
|||
})
|
||||
|
||||
t.Run("allow-none", func(t *testing.T) {
|
||||
sl := NewSkillsLoader(ws, global, builtin, []string{"non-existent"}, true)
|
||||
sl := NewSkillsLoader(ws, ws, global, builtin, []string{"non-existent"}, true)
|
||||
skills := sl.ListSkills()
|
||||
assert.Empty(t, skills)
|
||||
})
|
||||
|
||||
t.Run("empty-whitelist-allows-all", func(t *testing.T) {
|
||||
sl := NewSkillsLoader(ws, global, builtin, []string{}, false)
|
||||
sl := NewSkillsLoader(ws, ws, global, builtin, []string{}, false)
|
||||
skills := sl.ListSkills()
|
||||
assert.Len(t, skills, 3)
|
||||
})
|
||||
|
||||
t.Run("nil-whitelist-allows-all", func(t *testing.T) {
|
||||
sl := NewSkillsLoader(ws, global, builtin, nil, false)
|
||||
sl := NewSkillsLoader(ws, ws, global, builtin, nil, false)
|
||||
skills := sl.ListSkills()
|
||||
assert.Len(t, skills, 3)
|
||||
})
|
||||
|
|
|
|||
|
|
@ -191,6 +191,7 @@ func (h *Handler) handleDeleteSkill(w http.ResponseWriter, r *http.Request) {
|
|||
|
||||
func newSkillsLoader(workspace string) *skills.SkillsLoader {
|
||||
return skills.NewSkillsLoader(
|
||||
workspace,
|
||||
workspace,
|
||||
filepath.Join(globalConfigDir(), "skills"),
|
||||
builtinSkillsDir(),
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue