fix(agent): ensure isolated agents inherit manually registered tools to prevent test hangs

This commit is contained in:
stevef 2026-03-26 19:34:16 +01:00
parent eaf61a1e8a
commit 386a93a13c
21 changed files with 507 additions and 155 deletions

View file

@ -268,7 +268,7 @@ vet: generate
## test: Test Go code ## test: Test Go code
test: generate 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 @cd web && make test
## fmt: Format Go code ## fmt: Format Go code

View file

@ -43,7 +43,7 @@ func NewSkillsCommand() *cobra.Command {
globalDir := filepath.Dir(internal.GetConfigPath()) globalDir := filepath.Dir(internal.GetConfigPath())
globalSkillsDir := filepath.Join(globalDir, "skills") globalSkillsDir := filepath.Join(globalDir, "skills")
builtinSkillsDir := filepath.Join(globalDir, "picoclaw", "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 return nil
}, },

View file

@ -6,8 +6,6 @@
Config file: `~/.picoclaw/config.json` Config file: `~/.picoclaw/config.json`
> **Security Configuration:** For storing API keys, tokens, and other sensitive data, see the [Security Configuration Guide](security_configuration.md).
### Environment Variables ### Environment Variables
You can override default paths using environment variables. This is useful for portable installations, containerized deployments, or running picoclaw as a system service. These variables are independent and control different paths. You can override default paths using environment variables. This is useful for portable installations, containerized deployments, or running picoclaw as a system service. These variables are independent and control different paths.
@ -40,12 +38,12 @@ PICOCLAW_HOME=/srv/picoclaw PICOCLAW_CONFIG=/srv/picoclaw/main.json picoclaw gat
```json ```json
{ {
"gateway": { "gateway": {
"log_level": "warn" "log_level": "fatal"
} }
} }
``` ```
When omitted, the default is `warn`. Supported values: `debug`, `info`, `warn`, `error`, `fatal`. When omitted, the default is `fatal`. Supported values: `debug`, `info`, `warn`, `error`, `fatal`.
You can also override this with the environment variable `PICOCLAW_LOG_LEVEL`. You can also override this with the environment variable `PICOCLAW_LOG_LEVEL`.
@ -69,17 +67,37 @@ 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. > **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.
### Web launcher dashboard ### 🔒 Multi-Tenant Agent Isolation
**picoclaw-launcher** serves a browser UI that requires sign-in first. By default, the **dashboard token** and **session signing key** are **generated in memory on each start** (a new random token after every restart). Set **`PICOCLAW_LAUNCHER_TOKEN`** to pin a fixed token for that process (startup logs do not print the secret when this env var is used). 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.).
**Where to read the token**: In **console mode** (`-console`), it is printed at startup. In **tray / GUI mode**, use the tray action **Copy dashboard token**, and check **`$PICOCLAW_HOME/logs/launcher.log`** (typically `~/.picoclaw/logs/launcher.log` if `PICOCLAW_HOME` is unset) for the random token logged on startup. The login page shows hints that match how the launcher is running (including the absolute log path); **responses do not include the token itself**. #### Isolation Strategy
- **Config file**: Same directory as `config.json` (or the file pointed to by `PICOCLAW_CONFIG`). The launcher-specific file is `launcher-config.json`. When an incoming message includes a **ChatID** (passed in the `/chat` API or extracted from internal channels), PicoClaw automatically activates **Tenant Isolation**:
- **Sign-in and links**: Enter the token on the login page, or open with `?token=` when the browser is launched automatically. All responses include **`Referrer-Policy: no-referrer`** to reduce leakage of `token` via the `Referer` header.
- **Sign-out**: Use **`POST /api/auth/logout`** with **`Content-Type: application/json`** (body may be `{}`). Do not rely on a GET URL for logout (CSRF-safe pattern). 1. **Isolated Workspace:** The agent's operations are restricted to `workspace/sessions/{isolationID}/workspace`.
- **Brute-force**: **`POST /api/auth/login`** is **rate-limited per client IP per minute** (HTTP 429 when exceeded). 2. **Isolated Memory:** Long-term memory (`MEMORY.md`) is stored and read from the isolated session path.
- **Session lifetime**: The HttpOnly session cookie lasts about **7 days** by default; sign in again with the token after it expires. 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 ### Skill Sources
@ -541,9 +559,8 @@ This design also enables **multi-agent support** with flexible provider selectio
- **Different agents, different providers**: Each agent can use its own LLM provider - **Different agents, different providers**: Each agent can use its own LLM provider
- **Model fallbacks**: Configure primary and fallback models for resilience - **Model fallbacks**: Configure primary and fallback models for resilience
- **Load balancing**: Distribute requests across multiple endpoints or keys - **Load balancing**: Distribute requests across multiple endpoints
- **Centralized configuration**: Manage all providers in one place - **Centralized configuration**: Manage all providers in one place
- **Model enable/disable**: Use the `enabled` field to temporarily disable a model without removing its configuration
#### 🔒 Security Configuration (Recommended) #### 🔒 Security Configuration (Recommended)
@ -623,7 +640,6 @@ For complete documentation, see [`security_configuration.md`](security_configura
| **通义千问 (Qwen)** | `qwen/` | `https://dashscope.aliyuncs.com/compatible-mode/v1` | OpenAI | [Get Key](https://dashscope.console.aliyun.com) | | **通义千问 (Qwen)** | `qwen/` | `https://dashscope.aliyuncs.com/compatible-mode/v1` | OpenAI | [Get Key](https://dashscope.console.aliyun.com) |
| **NVIDIA** | `nvidia/` | `https://integrate.api.nvidia.com/v1` | OpenAI | [Get Key](https://build.nvidia.com) | | **NVIDIA** | `nvidia/` | `https://integrate.api.nvidia.com/v1` | OpenAI | [Get Key](https://build.nvidia.com) |
| **Ollama** | `ollama/` | `http://localhost:11434/v1` | OpenAI | Local (no key needed) | | **Ollama** | `ollama/` | `http://localhost:11434/v1` | OpenAI | Local (no key needed) |
| **LM Studio** | `lmstudio/` | `http://localhost:1234/v1` | OpenAI | Optional (local default: no key) |
| **OpenRouter** | `openrouter/` | `https://openrouter.ai/api/v1` | OpenAI | [Get Key](https://openrouter.ai/keys) | | **OpenRouter** | `openrouter/` | `https://openrouter.ai/api/v1` | OpenAI | [Get Key](https://openrouter.ai/keys) |
| **LiteLLM Proxy** | `litellm/` | `http://localhost:4000/v1` | OpenAI | Your LiteLLM proxy key | | **LiteLLM Proxy** | `litellm/` | `http://localhost:4000/v1` | OpenAI | Your LiteLLM proxy key |
| **VLLM** | `vllm/` | `http://localhost:8000/v1` | OpenAI | Local | | **VLLM** | `vllm/` | `http://localhost:8000/v1` | OpenAI | Local |
@ -645,22 +661,22 @@ For complete documentation, see [`security_configuration.md`](security_configura
{ {
"model_name": "ark-code-latest", "model_name": "ark-code-latest",
"model": "volcengine/ark-code-latest", "model": "volcengine/ark-code-latest",
"api_keys": ["sk-your-api-key"] "api_key": "sk-your-api-key"
}, },
{ {
"model_name": "gpt-5.4", "model_name": "gpt-5.4",
"model": "openai/gpt-5.4", "model": "openai/gpt-5.4",
"api_keys": ["sk-your-openai-key"] "api_key": "sk-your-openai-key"
}, },
{ {
"model_name": "claude-sonnet-4.6", "model_name": "claude-sonnet-4.6",
"model": "anthropic/claude-sonnet-4.6", "model": "anthropic/claude-sonnet-4.6",
"api_keys": ["sk-ant-your-key"] "api_key": "sk-ant-your-key"
}, },
{ {
"model_name": "glm-4.7", "model_name": "glm-4.7",
"model": "zhipu/glm-4.7", "model": "zhipu/glm-4.7",
"api_keys": ["your-zhipu-key"] "api_key": "your-zhipu-key"
} }
], ],
"agents": { "agents": {
@ -671,9 +687,7 @@ For complete documentation, see [`security_configuration.md`](security_configura
} }
``` ```
> **Security Note**: You can remove `api_keys` fields from your config and store them in `.security.yml` instead. See [Security Configuration](#-security-configuration-recommended) above for details. > **Security Note**: You can remove `api_key` fields from your config and store them in `.security.yml` instead. See [Security Configuration](#-security-configuration-recommended) above for details.
>
> **Note**: The `enabled` field can be set to `false` to disable a model entry without removing it. When omitted, it defaults to `true` during migration for models that have API keys.
#### Vendor-Specific Examples #### Vendor-Specific Examples
@ -750,7 +764,7 @@ For direct Anthropic API access or custom endpoints that only support Anthropic'
{ {
"model_name": "claude-opus-4-6", "model_name": "claude-opus-4-6",
"model": "anthropic-messages/claude-opus-4-6", "model": "anthropic-messages/claude-opus-4-6",
"api_keys": ["sk-ant-your-key"], "api_key": "sk-ant-your-key",
"api_base": "https://api.anthropic.com" "api_base": "https://api.anthropic.com"
} }
``` ```
@ -771,21 +785,6 @@ For direct Anthropic API access or custom endpoints that only support Anthropic'
</details> </details>
<details>
<summary><b>LM Studio (local)</b></summary>
```json
{
"model_name": "lmstudio-local",
"model": "lmstudio/openai/gpt-oss-20b"
}
```
`api_base` defaults to `http://localhost:1234/v1`. API key is optional unless your LM Studio server enables authentication.<br/>
PicoClaw sends OpenAI-compatible requests to LM Studio, and strips the `lmstudio/` prefix before sending requests, so `lmstudio/openai/gpt-oss-20b` sends `openai/gpt-oss-20b` to the LM Studio server.
</details>
<details> <details>
<summary><b>Custom Proxy / LiteLLM</b></summary> <summary><b>Custom Proxy / LiteLLM</b></summary>
@ -840,13 +839,13 @@ model_list:
"model_name": "gpt-5.4", "model_name": "gpt-5.4",
"model": "openai/gpt-5.4", "model": "openai/gpt-5.4",
"api_base": "https://api1.example.com/v1", "api_base": "https://api1.example.com/v1",
"api_keys": ["sk-key1"] "api_key": "sk-key1"
}, },
{ {
"model_name": "gpt-5.4", "model_name": "gpt-5.4",
"model": "openai/gpt-5.4", "model": "openai/gpt-5.4",
"api_base": "https://api2.example.com/v1", "api_base": "https://api2.example.com/v1",
"api_keys": ["sk-key2"] "api_key": "sk-key2"
} }
] ]
} }
@ -854,7 +853,7 @@ model_list:
#### Migration from Legacy `providers` Config #### Migration from Legacy `providers` Config
The old `providers` configuration is **deprecated** and has been removed in V2. Existing V0/V1 configs are auto-migrated. See [docs/migration/model-list-migration.md](../migration/model-list-migration.md) for the full guide. The old `providers` configuration is **deprecated** but still supported for backward compatibility. See [docs/migration/model-list-migration.md](../migration/model-list-migration.md) for the full guide.
### Provider Architecture ### Provider Architecture
@ -864,7 +863,7 @@ PicoClaw routes providers by protocol family:
- **Anthropic**: Claude-native API behavior. - **Anthropic**: Claude-native API behavior.
- **Codex/OAuth**: OpenAI OAuth/token authentication route. - **Codex/OAuth**: OpenAI OAuth/token authentication route.
This keeps the runtime lightweight while making new OpenAI-compatible backends mostly a config operation (`api_base` + `api_keys`). This keeps the runtime lightweight while making new OpenAI-compatible backends mostly a config operation (`api_base` + `api_key`).
<details> <details>
<summary><b>Zhipu (legacy providers format)</b></summary> <summary><b>Zhipu (legacy providers format)</b></summary>

View file

@ -21,6 +21,7 @@ import (
type ContextBuilder struct { type ContextBuilder struct {
workspace string workspace string
baseWorkspace string
skillsLoader *skills.SkillsLoader skillsLoader *skills.SkillsLoader
memory *MemoryStore memory *MemoryStore
toolDiscoveryBM25 bool toolDiscoveryBM25 bool
@ -61,7 +62,11 @@ func getGlobalConfigDir() string {
return config.GetHome() return config.GetHome()
} }
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 // builtin skills: skills directory in current project
// Use the skills/ directory under the current working directory // Use the skills/ directory under the current working directory
builtinSkillsDir := strings.TrimSpace(os.Getenv(config.EnvBuiltinSkills)) builtinSkillsDir := strings.TrimSpace(os.Getenv(config.EnvBuiltinSkills))
@ -73,7 +78,8 @@ func NewContextBuilder(workspace string) *ContextBuilder {
return &ContextBuilder{ return &ContextBuilder{
workspace: workspace, workspace: workspace,
skillsLoader: skills.NewSkillsLoader(workspace, globalSkillsDir, builtinSkillsDir, nil, false), baseWorkspace: baseWorkspace,
skillsLoader: skills.NewSkillsLoader(workspace, baseWorkspace, globalSkillsDir, builtinSkillsDir, nil, false),
memory: NewMemoryStore(workspace), memory: NewMemoryStore(workspace),
} }
} }
@ -462,7 +468,13 @@ func (cb *ContextBuilder) LoadBootstrapFiles() string {
if agentDefinition.Source != AgentDefinitionSourceAgent { if agentDefinition.Source != AgentDefinitionSourceAgent {
filePath := filepath.Join(cb.workspace, "IDENTITY.md") 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) fmt.Fprintf(&sb, "## %s\n\n%s\n\n", "IDENTITY.md", data)
} }
} }

View file

@ -41,7 +41,7 @@ func TestSingleSystemMessage(t *testing.T) {
}) })
defer os.RemoveAll(tmpDir) defer os.RemoveAll(tmpDir)
cb := NewContextBuilder(tmpDir) cb := NewContextBuilder(tmpDir, tmpDir)
tests := []struct { tests := []struct {
name string name string
@ -132,7 +132,7 @@ func TestBuildMessages_CurrentSenderDynamicContext(t *testing.T) {
}) })
defer os.RemoveAll(tmpDir) defer os.RemoveAll(tmpDir)
cb := NewContextBuilder(tmpDir) cb := NewContextBuilder(tmpDir, tmpDir)
tests := []struct { tests := []struct {
name string name string
@ -221,7 +221,7 @@ func TestMtimeAutoInvalidation(t *testing.T) {
tmpDir := setupWorkspace(t, map[string]string{tt.file: tt.contentV1}) tmpDir := setupWorkspace(t, map[string]string{tt.file: tt.contentV1})
defer os.RemoveAll(tmpDir) defer os.RemoveAll(tmpDir)
cb := NewContextBuilder(tmpDir) cb := NewContextBuilder(tmpDir, tmpDir)
sp1 := cb.BuildSystemPromptWithCache() sp1 := cb.BuildSystemPromptWithCache()
@ -257,7 +257,7 @@ func TestMtimeAutoInvalidation(t *testing.T) {
tmpDir := setupWorkspace(t, nil) tmpDir := setupWorkspace(t, nil)
defer os.RemoveAll(tmpDir) defer os.RemoveAll(tmpDir)
cb := NewContextBuilder(tmpDir) cb := NewContextBuilder(tmpDir, tmpDir)
_ = cb.BuildSystemPromptWithCache() // populate cache _ = cb.BuildSystemPromptWithCache() // populate cache
// Touch skills directory (simulate new skill installed) // Touch skills directory (simulate new skill installed)
@ -284,7 +284,7 @@ func TestExplicitInvalidateCache(t *testing.T) {
}) })
defer os.RemoveAll(tmpDir) defer os.RemoveAll(tmpDir)
cb := NewContextBuilder(tmpDir) cb := NewContextBuilder(tmpDir, tmpDir)
sp1 := cb.BuildSystemPromptWithCache() sp1 := cb.BuildSystemPromptWithCache()
cb.InvalidateCache() cb.InvalidateCache()
@ -312,7 +312,7 @@ func TestCacheStability(t *testing.T) {
}) })
defer os.RemoveAll(tmpDir) defer os.RemoveAll(tmpDir)
cb := NewContextBuilder(tmpDir) cb := NewContextBuilder(tmpDir, tmpDir)
results := make([]string, 5) results := make([]string, 5)
for i := range results { for i := range results {
@ -361,7 +361,7 @@ func TestNewFileCreationInvalidatesCache(t *testing.T) {
tmpDir := setupWorkspace(t, nil) tmpDir := setupWorkspace(t, nil)
defer os.RemoveAll(tmpDir) defer os.RemoveAll(tmpDir)
cb := NewContextBuilder(tmpDir) cb := NewContextBuilder(tmpDir, tmpDir)
// Populate cache — file does not exist yet // Populate cache — file does not exist yet
sp1 := cb.BuildSystemPromptWithCache() sp1 := cb.BuildSystemPromptWithCache()
@ -406,7 +406,7 @@ Original content.`
}) })
defer os.RemoveAll(tmpDir) defer os.RemoveAll(tmpDir)
cb := NewContextBuilder(tmpDir) cb := NewContextBuilder(tmpDir, tmpDir)
// Populate cache // Populate cache
sp1 := cb.BuildSystemPromptWithCache() sp1 := cb.BuildSystemPromptWithCache()
@ -467,7 +467,7 @@ description: global-v1
t.Fatal(err) t.Fatal(err)
} }
cb := NewContextBuilder(tmpDir) cb := NewContextBuilder(tmpDir, tmpDir)
sp1 := cb.BuildSystemPromptWithCache() sp1 := cb.BuildSystemPromptWithCache()
if !strings.Contains(sp1, "global-v1") { if !strings.Contains(sp1, "global-v1") {
t.Fatal("expected initial prompt to contain global skill description") t.Fatal("expected initial prompt to contain global skill description")
@ -527,7 +527,7 @@ description: builtin-v1
t.Fatal(err) t.Fatal(err)
} }
cb := NewContextBuilder(tmpDir) cb := NewContextBuilder(tmpDir, tmpDir)
sp1 := cb.BuildSystemPromptWithCache() sp1 := cb.BuildSystemPromptWithCache()
if !strings.Contains(sp1, "builtin-v1") { if !strings.Contains(sp1, "builtin-v1") {
t.Fatal("expected initial prompt to contain builtin skill description") t.Fatal("expected initial prompt to contain builtin skill description")
@ -574,7 +574,7 @@ description: delete-me-v1
}) })
defer os.RemoveAll(tmpDir) defer os.RemoveAll(tmpDir)
cb := NewContextBuilder(tmpDir) cb := NewContextBuilder(tmpDir, tmpDir)
sp1 := cb.BuildSystemPromptWithCache() sp1 := cb.BuildSystemPromptWithCache()
if !strings.Contains(sp1, "delete-me-v1") { if !strings.Contains(sp1, "delete-me-v1") {
t.Fatal("expected initial prompt to contain skill description") t.Fatal("expected initial prompt to contain skill description")
@ -614,7 +614,7 @@ func TestConcurrentBuildSystemPromptWithCache(t *testing.T) {
}) })
defer os.RemoveAll(tmpDir) defer os.RemoveAll(tmpDir)
cb := NewContextBuilder(tmpDir) cb := NewContextBuilder(tmpDir, tmpDir)
const goroutines = 20 const goroutines = 20
const iterations = 50 const iterations = 50
@ -677,7 +677,7 @@ func TestEmptyWorkspaceBaselineDetectsNewFiles(t *testing.T) {
tmpDir := setupWorkspace(t, nil) tmpDir := setupWorkspace(t, nil)
defer os.RemoveAll(tmpDir) defer os.RemoveAll(tmpDir)
cb := NewContextBuilder(tmpDir) cb := NewContextBuilder(tmpDir, tmpDir)
// Build cache — all tracked files are absent, maxMtime falls back to epoch. // Build cache — all tracked files are absent, maxMtime falls back to epoch.
sp1 := cb.BuildSystemPromptWithCache() sp1 := cb.BuildSystemPromptWithCache()
@ -750,7 +750,7 @@ func BenchmarkBuildMessagesWithCache(b *testing.B) {
os.WriteFile(filepath.Join(tmpDir, name), []byte(strings.Repeat("Content.\n", 10)), 0o644) os.WriteFile(filepath.Join(tmpDir, name), []byte(strings.Repeat("Content.\n", 10)), 0o644)
} }
cb := NewContextBuilder(tmpDir) cb := NewContextBuilder(tmpDir, tmpDir)
history := []providers.Message{ history := []providers.Message{
{Role: "user", Content: "previous message"}, {Role: "user", Content: "previous message"},
{Role: "assistant", Content: "previous response"}, {Role: "assistant", Content: "previous response"},

View file

@ -73,7 +73,25 @@ type AgentContextDefinition struct {
// structured files are absent, it falls back to the legacy AGENTS.md layout so // structured files are absent, it falls back to the legacy AGENTS.md layout so
// the current runtime can transition incrementally. // the current runtime can transition incrementally.
func (cb *ContextBuilder) LoadAgentDefinition() AgentContextDefinition { 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 { func loadAgentDefinition(workspace string) AgentContextDefinition {

View file

@ -34,7 +34,7 @@ Act directly and use tools first.
}) })
defer cleanupWorkspace(t, tmpDir) defer cleanupWorkspace(t, tmpDir)
cb := NewContextBuilder(tmpDir) cb := NewContextBuilder(tmpDir, tmpDir)
definition := cb.LoadAgentDefinition() definition := cb.LoadAgentDefinition()
if definition.Source != AgentDefinitionSourceAgent { if definition.Source != AgentDefinitionSourceAgent {
@ -86,7 +86,7 @@ func TestLoadAgentDefinitionFallsBackToLegacyAgentsMarkdown(t *testing.T) {
}) })
defer cleanupWorkspace(t, tmpDir) defer cleanupWorkspace(t, tmpDir)
cb := NewContextBuilder(tmpDir) cb := NewContextBuilder(tmpDir, tmpDir)
definition := cb.LoadAgentDefinition() definition := cb.LoadAgentDefinition()
if definition.Source != AgentDefinitionSourceAgents { if definition.Source != AgentDefinitionSourceAgents {
@ -113,7 +113,7 @@ func TestLoadAgentDefinitionLoadsWorkspaceUserMarkdown(t *testing.T) {
}) })
defer cleanupWorkspace(t, tmpDir) defer cleanupWorkspace(t, tmpDir)
cb := NewContextBuilder(tmpDir) cb := NewContextBuilder(tmpDir, tmpDir)
definition := cb.LoadAgentDefinition() definition := cb.LoadAgentDefinition()
if definition.User == nil { if definition.User == nil {
@ -142,7 +142,7 @@ Keep going.
}) })
defer cleanupWorkspace(t, tmpDir) defer cleanupWorkspace(t, tmpDir)
cb := NewContextBuilder(tmpDir) cb := NewContextBuilder(tmpDir, tmpDir)
definition := cb.LoadAgentDefinition() definition := cb.LoadAgentDefinition()
if definition.Agent == nil { if definition.Agent == nil {
@ -178,7 +178,7 @@ Follow the body prompt.
}) })
defer cleanupWorkspace(t, tmpDir) defer cleanupWorkspace(t, tmpDir)
cb := NewContextBuilder(tmpDir) cb := NewContextBuilder(tmpDir, tmpDir)
bootstrap := cb.LoadBootstrapFiles() bootstrap := cb.LoadBootstrapFiles()
if !strings.Contains(bootstrap, "Follow the body prompt") { if !strings.Contains(bootstrap, "Follow the body prompt") {
@ -209,7 +209,7 @@ func TestLoadBootstrapFilesIncludesWorkspaceUserMarkdown(t *testing.T) {
}) })
defer cleanupWorkspace(t, tmpDir) defer cleanupWorkspace(t, tmpDir)
cb := NewContextBuilder(tmpDir) cb := NewContextBuilder(tmpDir, tmpDir)
bootstrap := cb.LoadBootstrapFiles() bootstrap := cb.LoadBootstrapFiles()
if !strings.Contains(bootstrap, "Shared profile") { if !strings.Contains(bootstrap, "Shared profile") {
@ -228,7 +228,7 @@ func TestStructuredAgentIgnoresIdentityChanges(t *testing.T) {
}) })
defer cleanupWorkspace(t, tmpDir) defer cleanupWorkspace(t, tmpDir)
cb := NewContextBuilder(tmpDir) cb := NewContextBuilder(tmpDir, tmpDir)
promptV1 := cb.BuildSystemPromptWithCache() promptV1 := cb.BuildSystemPromptWithCache()
if strings.Contains(promptV1, "Legacy identity") { if strings.Contains(promptV1, "Legacy identity") {
@ -265,7 +265,7 @@ func TestStructuredAgentUserChangesInvalidateCache(t *testing.T) {
}) })
defer cleanupWorkspace(t, tmpDir) defer cleanupWorkspace(t, tmpDir)
cb := NewContextBuilder(tmpDir) cb := NewContextBuilder(tmpDir, tmpDir)
promptV1 := cb.BuildSystemPromptWithCache() promptV1 := cb.BuildSystemPromptWithCache()
if !strings.Contains(promptV1, "Initial workspace preferences") { if !strings.Contains(promptV1, "Initial workspace preferences") {

View file

@ -275,7 +275,7 @@ func TestAgentLoop_EmitsSteeringAndSkippedToolEvents(t *testing.T) {
resultCh := make(chan string, 1) resultCh := make(chan string, 1)
go func() { 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 resultCh <- resp
}() }()

View file

@ -59,8 +59,9 @@ func NewAgentInstance(
defaults *config.AgentDefaults, defaults *config.AgentDefaults,
cfg *config.Config, cfg *config.Config,
provider providers.LLMProvider, provider providers.LLMProvider,
isolationID string,
) *AgentInstance { ) *AgentInstance {
workspace := resolveAgentWorkspace(agentCfg, defaults) workspace := resolveAgentWorkspace(agentCfg, defaults, isolationID)
os.MkdirAll(workspace, 0o755) os.MkdirAll(workspace, 0o755)
model := resolveAgentModel(agentCfg, defaults) model := resolveAgentModel(agentCfg, defaults)
@ -107,11 +108,15 @@ func NewAgentInstance(
toolsRegistry.Register(tools.NewAppendFileTool(workspace, restrict, allowWritePaths)) 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) sessions := initSessionStore(sessionsDir)
mcpDiscoveryActive := cfg.Tools.MCP.Enabled && cfg.Tools.MCP.Discovery.Enabled mcpDiscoveryActive := cfg.Tools.MCP.Enabled && cfg.Tools.MCP.Discovery.Enabled
contextBuilder := NewContextBuilder(workspace). baseWorkspace := mainWorkspace
contextBuilder := NewContextBuilder(workspace, baseWorkspace).
WithToolDiscovery( WithToolDiscovery(
mcpDiscoveryActive && cfg.Tools.MCP.Discovery.UseBM25, mcpDiscoveryActive && cfg.Tools.MCP.Discovery.UseBM25,
mcpDiscoveryActive && cfg.Tools.MCP.Discovery.UseRegex, mcpDiscoveryActive && cfg.Tools.MCP.Discovery.UseRegex,
@ -234,17 +239,27 @@ func NewAgentInstance(
} }
// resolveAgentWorkspace determines the workspace directory for an agent. // 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) != "" { 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" {
// Use the configured default workspace (respects PICOCLAW_HOME) base = expandHome(defaults.Workspace)
if agentCfg == nil || agentCfg.Default || agentCfg.ID == "" || routing.NormalizeAgentID(agentCfg.ID) == "main" { } else {
return expandHome(defaults.Workspace)
}
// For named agents without explicit workspace, use default workspace with agent ID suffix // For named agents without explicit workspace, use default workspace with agent ID suffix
id := routing.NormalizeAgentID(agentCfg.ID) id := routing.NormalizeAgentID(agentCfg.ID)
return filepath.Join(expandHome(defaults.Workspace), "..", "workspace-"+id) base = filepath.Join(expandHome(defaults.Workspace), "..", "workspace-"+id)
}
if isolationID != "" && isolationID != "direct" {
return filepath.Join(base, "sessions", isolationID, "workspace")
}
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. // resolveAgentModel resolves the primary model for an agent.

View file

@ -33,7 +33,7 @@ func TestNewAgentInstance_UsesDefaultsTemperatureAndMaxTokens(t *testing.T) {
cfg.Agents.Defaults.Temperature = &configuredTemp cfg.Agents.Defaults.Temperature = &configuredTemp
provider := &mockProvider{} provider := &mockProvider{}
agent := NewAgentInstance(nil, &cfg.Agents.Defaults, cfg, provider) agent := NewAgentInstance(nil, &cfg.Agents.Defaults, cfg, provider, "")
if agent.MaxTokens != 1234 { if agent.MaxTokens != 1234 {
t.Fatalf("MaxTokens = %d, want %d", 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 cfg.Agents.Defaults.Temperature = &configuredTemp
provider := &mockProvider{} provider := &mockProvider{}
agent := NewAgentInstance(nil, &cfg.Agents.Defaults, cfg, provider) agent := NewAgentInstance(nil, &cfg.Agents.Defaults, cfg, provider, "")
if agent.Temperature != 0.0 { if agent.Temperature != 0.0 {
t.Fatalf("Temperature = %f, want %f", 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{} provider := &mockProvider{}
agent := NewAgentInstance(nil, &cfg.Agents.Defaults, cfg, provider) agent := NewAgentInstance(nil, &cfg.Agents.Defaults, cfg, provider, "")
if agent.Temperature != 0.7 { if agent.Temperature != 0.7 {
t.Fatalf("Temperature = %f, want %f", 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{} provider := &mockProvider{}
agent := NewAgentInstance(nil, &cfg.Agents.Defaults, cfg, provider) agent := NewAgentInstance(nil, &cfg.Agents.Defaults, cfg, provider, "")
if len(agent.Candidates) != 1 { if len(agent.Candidates) != 1 {
t.Fatalf("len(Candidates) = %d, want 1", len(agent.Candidates)) t.Fatalf("len(Candidates) = %d, want 1", len(agent.Candidates))
@ -257,7 +257,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") readTool, ok := agent.Tools.Get("read_file")
if !ok { if !ok {
@ -361,7 +361,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 { if agent == nil {
t.Fatal("expected agent instance, got nil") t.Fatal("expected agent instance, got nil")
} }
@ -374,3 +374,31 @@ func TestNewAgentInstance_InvalidExecConfigDoesNotExit(t *testing.T) {
t.Fatal("read_file tool should still be registered") 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")
}
}

View 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"
}

View file

@ -59,11 +59,20 @@ type AgentLoop struct {
steering *steeringQueue steering *steeringQueue
pendingSkills sync.Map pendingSkills sync.Map
mu sync.RWMutex mu sync.RWMutex
manualTools []tools.Tool
// Concurrent turn management (from HEAD) // Concurrent turn management (from HEAD)
activeTurnStates sync.Map // key: sessionKey (string), value: *turnState activeTurnStates sync.Map // key: sessionKey (string), value: *turnState
subTurnCounter atomic.Int64 // Counter for generating unique SubTurn IDs 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) // Turn tracking (from Incoming)
turnSeq atomic.Uint64 turnSeq atomic.Uint64
activeRequests sync.WaitGroup activeRequests sync.WaitGroup
@ -103,7 +112,7 @@ const (
defaultResponse = "The model returned an empty response. This may indicate a provider error or token limit." 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." 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." handledToolResponseSummary = "Requested output delivered via tool attachment."
sessionKeyAgentPrefix = "agent:" sessionKeyAgentPrefix = "agent::"
metadataKeyAccountID = "account_id" metadataKeyAccountID = "account_id"
metadataKeyGuildID = "guild_id" metadataKeyGuildID = "guild_id"
metadataKeyTeamID = "team_id" metadataKeyTeamID = "team_id"
@ -183,6 +192,13 @@ func registerSharedTools(
continue 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") { if cfg.Tools.IsToolEnabled("web") {
searchTool, err := tools.NewWebSearchTool(tools.WebSearchToolOptions{ searchTool, err := tools.NewWebSearchTool(tools.WebSearchToolOptions{
BraveAPIKeys: cfg.Tools.Web.Brave.APIKeys.Values(), BraveAPIKeys: cfg.Tools.Web.Brave.APIKeys.Values(),
@ -730,7 +746,7 @@ func (al *AgentLoop) buildContinuationTarget(msg bus.InboundMessage) (*continuat
} }
return &continuationTarget{ return &continuationTarget{
SessionKey: resolveScopeKey(route, msg.SessionKey), SessionKey: resolveScopeKey(route, msg.SessionKey, msg.ChatID, route.AgentID),
Channel: msg.Channel, Channel: msg.Channel,
ChatID: msg.ChatID, ChatID: msg.ChatID,
}, nil }, nil
@ -985,6 +1001,21 @@ func (al *AgentLoop) RegisterTool(tool tools.Tool) {
agent.Tools.Register(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) { func (al *AgentLoop) SetChannelManager(cm *channels.Manager) {
@ -1387,11 +1418,63 @@ func (al *AgentLoop) processMessage(ctx context.Context, msg bus.InboundMessage)
return al.processSystemMessage(ctx, msg) return al.processSystemMessage(ctx, msg)
} }
route, agent, routeErr := al.resolveMessageRoute(msg) route, baseAgent, routeErr := al.resolveMessageRoute(msg)
if routeErr != nil { if routeErr != nil {
return "", routeErr 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. // 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 tool, ok := agent.Tools.Get("message"); ok {
if resetter, ok := tool.(interface{ ResetSentInRound() }); ok { if resetter, ok := tool.(interface{ ResetSentInRound() }); ok {
@ -1400,7 +1483,8 @@ func (al *AgentLoop) processMessage(ctx context.Context, msg bus.InboundMessage)
} }
// Resolve session key from route, while preserving explicit agent-scoped keys. // 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 sessionKey := scopeKey
logger.InfoCF("agent", "Routed message", logger.InfoCF("agent", "Routed message",
@ -1468,10 +1552,19 @@ func (al *AgentLoop) resolveMessageRoute(msg bus.InboundMessage) (routing.Resolv
return route, agent, nil 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) { if msgSessionKey != "" && strings.HasPrefix(msgSessionKey, sessionKeyAgentPrefix) {
return msgSessionKey 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 return route.SessionKey
} }
@ -1485,7 +1578,7 @@ func (al *AgentLoop) resolveSteeringTarget(msg bus.InboundMessage) (string, stri
return "", "", false 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 { func (al *AgentLoop) requeueInboundMessage(msg bus.InboundMessage) error {

View file

@ -64,7 +64,7 @@ func (al *AgentLoop) EnsureMCPInitialized(ctx context.Context) error {
return nil 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) logger.WarnCF("agent", "MCP is enabled but no servers are configured, skipping MCP initialization", nil)
return nil return nil
} }

View file

@ -670,7 +670,7 @@ func TestProcessMessage_MediaToolHandledSkipsFollowUpLLMAndFinalText(t *testing.
if err != nil { if err != nil {
t.Fatalf("resolveMessageRoute() error = %v", err) t.Fatalf("resolveMessageRoute() error = %v", err)
} }
sessionKey := resolveScopeKey(route, "") sessionKey := resolveScopeKey(route, "", "chat1", route.AgentID)
history := defaultAgent.Sessions.GetHistory(sessionKey) history := defaultAgent.Sessions.GetHistory(sessionKey)
if len(history) == 0 { if len(history) == 0 {
t.Fatal("expected session history to be saved") t.Fatal("expected session history to be saved")
@ -1399,11 +1399,8 @@ func TestProcessMessage_UsesRouteSessionKey(t *testing.T) {
}, },
} }
route := al.registry.ResolveRoute(routing.RouteInput{ // With chatID isolation, session key is derived from chatID
Channel: msg.Channel, sessionKey := fmt.Sprintf("agent:::main:%s", msg.ChatID)
Peer: extractPeer(msg),
})
sessionKey := route.SessionKey
defaultAgent := al.registry.GetDefaultAgent() defaultAgent := al.registry.GetDefaultAgent()
if defaultAgent == nil { if defaultAgent == nil {
@ -2087,7 +2084,7 @@ func TestAgentLoop_ToolLimitUsesDedicatedFallback(t *testing.T) {
al := NewAgentLoop(cfg, msgBus, provider) al := NewAgentLoop(cfg, msgBus, provider)
al.RegisterTool(&toolLimitTestTool{}) 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 { if err != nil {
t.Fatalf("ProcessDirectWithChannel failed: %v", err) t.Fatalf("ProcessDirectWithChannel failed: %v", err)
} }

View file

@ -33,14 +33,15 @@ func NewAgentRegistry(
ID: "main", ID: "main",
Default: true, Default: true,
} }
instance := NewAgentInstance(implicitAgent, &cfg.Agents.Defaults, cfg, provider) instance := NewAgentInstance(implicitAgent, &cfg.Agents.Defaults, cfg, provider, "")
registry.agents["main"] = instance registry.agents["main"] = instance
logger.InfoCF("agent", "Created implicit main agent (no agents.list configured)", nil) logger.InfoCF("agent", "Created implicit main agent (no agents.list configured)", nil)
} else { } else {
for i := range agentConfigs { for i := range agentConfigs {
ac := &agentConfigs[i] ac := &agentConfigs[i]
id := routing.NormalizeAgentID(ac.ID) 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 registry.agents[id] = instance
logger.InfoCF("agent", "Registered agent", logger.InfoCF("agent", "Registered agent",
map[string]any{ map[string]any{

View file

@ -298,7 +298,7 @@ func TestAgentLoop_Continue_NoMessages(t *testing.T) {
t.Fatal("expected provider to be initialized") 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 { if err != nil {
t.Fatalf("unexpected error: %v", err) 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"}) 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 { if err != nil {
t.Fatalf("unexpected error: %v", err) t.Fatalf("unexpected error: %v", err)
} }
@ -367,7 +367,7 @@ func TestDrainBusToSteering_RequeuesDifferentScopeMessage(t *testing.T) {
activeMsg := bus.InboundMessage{ activeMsg := bus.InboundMessage{
Channel: "telegram", Channel: "telegram",
SenderID: "user1", SenderID: "user1",
ChatID: "chat1", ChatID: "direct",
Content: "active turn", Content: "active turn",
Peer: bus.Peer{ Peer: bus.Peer{
Kind: "direct", Kind: "direct",
@ -701,7 +701,7 @@ func TestAgentLoop_Steering_SkipsRemainingTools(t *testing.T) {
"do something", "do something",
"test-session", "test-session",
"test", "test",
"chat1", "direct",
) )
resultCh <- result{resp, err} resultCh <- result{resp, err}
}() }()
@ -783,7 +783,7 @@ func TestAgentLoop_Steering_InitialPoll(t *testing.T) {
"initial message", "initial message",
"test-session", "test-session",
"test", "test",
"chat1", "direct",
) )
if err != nil { if err != nil {
t.Fatalf("unexpected error: %v", err) t.Fatalf("unexpected error: %v", err)
@ -843,7 +843,7 @@ func TestAgentLoop_Run_AutoContinuesLateSteeringMessage(t *testing.T) {
first := bus.InboundMessage{ first := bus.InboundMessage{
Channel: "test", Channel: "test",
SenderID: "user1", SenderID: "user1",
ChatID: "chat1", ChatID: "direct",
Content: "first message", Content: "first message",
Peer: bus.Peer{ Peer: bus.Peer{
Kind: "direct", Kind: "direct",
@ -853,7 +853,7 @@ func TestAgentLoop_Run_AutoContinuesLateSteeringMessage(t *testing.T) {
late := bus.InboundMessage{ late := bus.InboundMessage{
Channel: "test", Channel: "test",
SenderID: "user1", SenderID: "user1",
ChatID: "chat1", ChatID: "direct",
Content: "late append", Content: "late append",
Peer: bus.Peer{ Peer: bus.Peer{
Kind: "direct", Kind: "direct",
@ -970,7 +970,7 @@ func TestAgentLoop_Steering_DirectResponseContinuesWithQueuedMessage(t *testing.
"initial request", "initial request",
sessionKey, sessionKey,
"test", "test",
"chat1", "direct",
) )
resultCh <- struct { resultCh <- struct {
resp string resp string
@ -1073,7 +1073,7 @@ func TestAgentLoop_Continue_PreservesSteeringMedia(t *testing.T) {
t.Fatalf("Steer failed: %v", err) 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 { if err != nil {
t.Fatalf("Continue failed: %v", err) t.Fatalf("Continue failed: %v", err)
} }
@ -1184,7 +1184,7 @@ func TestAgentLoop_InterruptGraceful_UsesTerminalNoToolCall(t *testing.T) {
"do something", "do something",
sessionKey, sessionKey,
"test", "test",
"chat1", "direct",
) )
resultCh <- result{resp: resp, err: err} resultCh <- result{resp: resp, err: err}
}() }()
@ -1202,7 +1202,7 @@ func TestAgentLoop_InterruptGraceful_UsesTerminalNoToolCall(t *testing.T) {
if active.SessionKey != sessionKey { if active.SessionKey != sessionKey {
t.Fatalf("expected active session %q, got %q", sessionKey, active.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) t.Fatalf("unexpected active turn target: %#v", active)
} }
@ -1349,7 +1349,7 @@ func TestAgentLoop_InterruptHard_RestoresSession(t *testing.T) {
"do work", "do work",
sessionKey, sessionKey,
"test", "test",
"chat1", "direct",
) )
resultCh <- result{resp: resp, err: err} resultCh <- result{resp: resp, err: err}
}() }()
@ -1518,7 +1518,7 @@ func TestAgentLoop_Steering_SkippedToolsHaveErrorResults(t *testing.T) {
resultCh := make(chan string, 1) resultCh := make(chan string, 1)
go func() { go func() {
resp, _ := al.ProcessDirectWithChannel( resp, _ := al.ProcessDirectWithChannel(
context.Background(), "go", "test-session", "test", "chat1", context.Background(), "go", "test-session", "test", "direct",
) )
resultCh <- resp resultCh <- resp
}() }()

View file

@ -208,11 +208,14 @@ func Run(debug bool, homePath, configPath string, allowEmptyStartup bool) error
// Setup synchronous /chat endpoint handler // Setup synchronous /chat endpoint handler
if cfg.Gateway.ChatEnabled { 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 == "" { if sessionID == "" {
sessionID = "http-chat" sessionID = "http-chat"
} }
return agentLoop.ProcessDirectWithChannel(ctx, message, sessionID, "http", "chat") if chatID == "" {
chatID = "chat"
}
return agentLoop.ProcessDirectWithChannel(ctx, message, sessionID, "http", chatID)
}) })
} }

View file

@ -17,6 +17,7 @@ import (
type ChatRequest struct { type ChatRequest struct {
Message string `json:"message"` Message string `json:"message"`
SessionID string `json:"session_id,omitempty"` 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. // ChatResponse is the JSON response from /chat.
@ -43,7 +44,7 @@ type Server struct {
startTime time.Time startTime time.Time
reloadFunc func() error reloadFunc func() error
authToken string // optional bearer token for protected endpoints authToken string // optional bearer token for protected endpoints
chatFunc func(ctx context.Context, message, sessionID string) (string, error) chatFunc func(ctx context.Context, message, sessionID, chatID string) (string, error)
apiKey string apiKey string
chatResults map[string]*chatStatus chatResults map[string]*chatStatus
chatResultsMu sync.RWMutex chatResultsMu sync.RWMutex
@ -157,7 +158,7 @@ func (s *Server) SetReloadFunc(fn func() error) {
// fn receives the user message and an optional session ID and must return the // 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 // 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. // 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() s.mu.Lock()
defer s.mu.Unlock() defer s.mu.Unlock()
s.chatFunc = fn s.chatFunc = fn
@ -335,6 +336,56 @@ func (s *Server) handlePostChat(w http.ResponseWriter, r *http.Request) {
} }
sessionID := req.SessionID 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 == "" { if sessionID == "" {
sessionID = fmt.Sprintf("chat-%d", time.Now().UnixNano()) sessionID = fmt.Sprintf("chat-%d", time.Now().UnixNano())
} }
@ -352,7 +403,7 @@ func (s *Server) handlePostChat(w http.ResponseWriter, r *http.Request) {
// which will be cancelled when this request finishes. // which will be cancelled when this request finishes.
ctx := context.Background() ctx := context.Background()
logger.Debugf("Starting async chat for session %s", sessionID) 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() s.chatResultsMu.Lock()
defer s.chatResultsMu.Unlock() defer s.chatResultsMu.Unlock()

View file

@ -61,6 +61,7 @@ func (info SkillInfo) validate() error {
type SkillsLoader struct { type SkillsLoader struct {
workspace string workspace string
workspaceSkills string // workspace skills (project-level) workspaceSkills string // workspace skills (project-level)
baseWorkspaceSkills string // fallback workspace skills (if isolated)
globalSkills string // global skills (~/.picoclaw/skills) globalSkills string // global skills (~/.picoclaw/skills)
builtinSkills string // builtin skills builtinSkills string // builtin skills
whitelist []string whitelist []string
@ -70,7 +71,7 @@ type SkillsLoader struct {
// SkillRoots returns all unique skill root directories used by this loader. // SkillRoots returns all unique skill root directories used by this loader.
// The order follows resolution priority: workspace > global > builtin. // The order follows resolution priority: workspace > global > builtin.
func (sl *SkillsLoader) SkillRoots() []string { 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)) seen := make(map[string]struct{}, len(roots))
out := make([]string, 0, len(roots)) out := make([]string, 0, len(roots))
@ -92,6 +93,7 @@ func (sl *SkillsLoader) SkillRoots() []string {
func NewSkillsLoader( func NewSkillsLoader(
workspace string, workspace string,
baseWorkspace string,
globalSkills string, globalSkills string,
builtinSkills string, builtinSkills string,
whitelist []string, whitelist []string,
@ -100,6 +102,7 @@ func NewSkillsLoader(
return &SkillsLoader{ return &SkillsLoader{
workspace: workspace, workspace: workspace,
workspaceSkills: filepath.Join(workspace, "skills"), workspaceSkills: filepath.Join(workspace, "skills"),
baseWorkspaceSkills: filepath.Join(baseWorkspace, "skills"),
globalSkills: globalSkills, // ~/.picoclaw/skills globalSkills: globalSkills, // ~/.picoclaw/skills
builtinSkills: builtinSkills, builtinSkills: builtinSkills,
whitelist: whitelist, whitelist: whitelist,
@ -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.workspaceSkills, "workspace")
addSkills(sl.baseWorkspaceSkills, "shared")
addSkills(sl.globalSkills, "global") addSkills(sl.globalSkills, "global")
addSkills(sl.builtinSkills, "builtin") 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) // 2. then load from global skills (~/.picoclaw/skills)
if sl.globalSkills != "" { if sl.globalSkills != "" {
skillFile := filepath.Join(sl.globalSkills, name, "SKILL.md") skillFile := filepath.Join(sl.globalSkills, name, "SKILL.md")

View file

@ -155,7 +155,7 @@ func TestListSkillsWorkspaceOverridesGlobal(t *testing.T) {
createSkillDir(t, filepath.Join(ws, "skills"), "my-skill", "my-skill", "workspace version") createSkillDir(t, filepath.Join(ws, "skills"), "my-skill", "my-skill", "workspace version")
createSkillDir(t, global, "my-skill", "my-skill", "global 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() skills := sl.ListSkills()
assert.Len(t, skills, 1) 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, global, "my-skill", "my-skill", "global version")
createSkillDir(t, builtin, "my-skill", "my-skill", "builtin 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() skills := sl.ListSkills()
assert.Len(t, skills, 1) 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, filepath.Join(ws, "skills"), "dir-a", "shared-name", "workspace version")
createSkillDir(t, global, "dir-b", "shared-name", "global 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() skills := sl.ListSkills()
assert.Len(t, skills, 1) 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, global, "skill-b", "skill-b", "desc b")
createSkillDir(t, builtin, "skill-c", "skill-c", "desc c") 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() skills := sl.ListSkills()
assert.Len(t, skills, 3) assert.Len(t, skills, 3)
@ -230,7 +230,7 @@ func TestListSkillsInvalidSkillSkipped(t *testing.T) {
// Valid skill // Valid skill
createSkillDir(t, global, "good-skill", "good-skill", "desc") createSkillDir(t, global, "good-skill", "good-skill", "desc")
sl := NewSkillsLoader(ws, global, "", nil, false) sl := NewSkillsLoader(ws, ws, global, "", nil, false)
skills := sl.ListSkills() skills := sl.ListSkills()
assert.Len(t, skills, 1) assert.Len(t, skills, 1)
@ -243,7 +243,7 @@ func TestListSkillsEmptyAndNonexistentDirs(t *testing.T) {
emptyDir := filepath.Join(tmp, "empty") emptyDir := filepath.Join(tmp, "empty")
require.NoError(t, os.MkdirAll(emptyDir, 0o755)) 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() skills := sl.ListSkills()
assert.Empty(t, skills) assert.Empty(t, skills)
@ -259,7 +259,7 @@ func TestListSkillsDirWithoutSkillMD(t *testing.T) {
// Valid skill alongside // Valid skill alongside
createSkillDir(t, global, "real-skill", "real-skill", "desc") createSkillDir(t, global, "real-skill", "real-skill", "desc")
sl := NewSkillsLoader(ws, global, "", nil, false) sl := NewSkillsLoader(ws, ws, global, "", nil, false)
skills := sl.ListSkills() skills := sl.ListSkills()
assert.Len(t, skills, 1) assert.Len(t, skills, 1)
@ -333,7 +333,7 @@ func TestSkillRootsTrimsWhitespaceAndDedups(t *testing.T) {
global := filepath.Join(tmp, "global") global := filepath.Join(tmp, "global")
builtin := filepath.Join(tmp, "builtin") 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() roots := sl.SkillRoots()
assert.Equal(t, []string{ assert.Equal(t, []string{
@ -429,14 +429,14 @@ func TestListSkillsWithWhitelist(t *testing.T) {
createSkillDir(t, builtin, "skill-c", "skill-c", "desc c") createSkillDir(t, builtin, "skill-c", "skill-c", "desc c")
t.Run("allow-one", func(t *testing.T) { 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() skills := sl.ListSkills()
assert.Len(t, skills, 1) assert.Len(t, skills, 1)
assert.Equal(t, "skill-a", skills[0].Name) assert.Equal(t, "skill-a", skills[0].Name)
}) })
t.Run("allow-two", func(t *testing.T) { 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() skills := sl.ListSkills()
assert.Len(t, skills, 2) assert.Len(t, skills, 2)
names := []string{skills[0].Name, skills[1].Name} 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) { 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() skills := sl.ListSkills()
assert.Empty(t, skills) assert.Empty(t, skills)
}) })
t.Run("empty-whitelist-allows-all", func(t *testing.T) { 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() skills := sl.ListSkills()
assert.Len(t, skills, 3) assert.Len(t, skills, 3)
}) })
t.Run("nil-whitelist-allows-all", func(t *testing.T) { 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() skills := sl.ListSkills()
assert.Len(t, skills, 3) assert.Len(t, skills, 3)
}) })

View file

@ -504,6 +504,7 @@ func (h *Handler) handleDeleteSkill(w http.ResponseWriter, r *http.Request) {
func newSkillsLoader(workspace string) *skills.SkillsLoader { func newSkillsLoader(workspace string) *skills.SkillsLoader {
return skills.NewSkillsLoader( return skills.NewSkillsLoader(
workspace,
workspace, workspace,
filepath.Join(globalConfigDir(), "skills"), filepath.Join(globalConfigDir(), "skills"),
builtinSkillsDir(), builtinSkillsDir(),