diff --git a/.golangci.yaml b/.golangci.yaml index d45d69e67..dd3cbae19 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -66,7 +66,6 @@ linters: - testifylint - thelper - unparam - - unused - usestdlibvars - usetesting - wastedassign @@ -152,6 +151,9 @@ linters: - gocognit - gocyclo path: _test\.go$ + - linters: + - nolintlint + path: 'pkg/tools/(i2c\.go|spi\.go)$' issues: max-issues-per-linter: 0 diff --git a/Dockerfile b/Dockerfile index 0360cfda6..480244127 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,7 +1,7 @@ # ============================================================ # Stage 1: Build the picoclaw binary # ============================================================ -FROM golang:1.26.0-alpine AS builder +FROM golang:1.25-alpine AS builder RUN apk add --no-cache git make diff --git a/Makefile b/Makefile index 103fb88f2..38cc1da17 100644 --- a/Makefile +++ b/Makefile @@ -14,7 +14,7 @@ GO_VERSION=$(shell $(GO) version | awk '{print $$3}') LDFLAGS=-ldflags "-X main.version=$(VERSION) -X main.gitCommit=$(GIT_COMMIT) -X main.buildTime=$(BUILD_TIME) -X main.goVersion=$(GO_VERSION) -s -w" # Go variables -GO?=go +GO?=CGO_ENABLED=0 go GOFLAGS?=-v -tags stdjson # Golangci-lint @@ -24,6 +24,7 @@ GOLANGCI_LINT?=golangci-lint INSTALL_PREFIX?=$(HOME)/.local INSTALL_BIN_DIR=$(INSTALL_PREFIX)/bin INSTALL_MAN_DIR=$(INSTALL_PREFIX)/share/man/man1 +INSTALL_TMP_SUFFIX=.new # Workspace and Skills PICOCLAW_HOME?=$(HOME)/.picoclaw @@ -105,8 +106,10 @@ build-riscv64: generate install: build @echo "Installing $(BINARY_NAME)..." @mkdir -p $(INSTALL_BIN_DIR) - @cp $(BUILD_DIR)/$(BINARY_NAME) $(INSTALL_BIN_DIR)/$(BINARY_NAME) - @chmod +x $(INSTALL_BIN_DIR)/$(BINARY_NAME) + # Copy binary with temporary suffix to ensure atomic update + @cp $(BUILD_DIR)/$(BINARY_NAME) $(INSTALL_BIN_DIR)/$(BINARY_NAME)$(INSTALL_TMP_SUFFIX) + @chmod +x $(INSTALL_BIN_DIR)/$(BINARY_NAME)$(INSTALL_TMP_SUFFIX) + @mv -f $(INSTALL_BIN_DIR)/$(BINARY_NAME)$(INSTALL_TMP_SUFFIX) $(INSTALL_BIN_DIR)/$(BINARY_NAME) @echo "Installed binary to $(INSTALL_BIN_DIR)/$(BINARY_NAME)" @echo "Installation complete!" @@ -147,6 +150,10 @@ fmt: lint: @$(GOLANGCI_LINT) run +## fix: Fix linting issues +fix: + @$(GOLANGCI_LINT) run --fix + ## deps: Download dependencies deps: @$(GO) mod download @@ -172,7 +179,7 @@ help: @echo " make [target]" @echo "" @echo "Targets:" - @grep -E '^## ' $(MAKEFILE_LIST) | sed 's/## / /' + @grep -E '^## ' $(MAKEFILE_LIST) | sort | awk -F': ' '{printf " %-16s %s\n", substr($$1, 4), $$2}' @echo "" @echo "Examples:" @echo " make build # Build for current platform" diff --git a/README.fr.md b/README.fr.md index a762870ff..f59807739 100644 --- a/README.fr.md +++ b/README.fr.md @@ -171,6 +171,10 @@ vim config/config.json # Configurez DISCORD_BOT_TOKEN, clés API, etc. # 3. Compiler & Démarrer docker compose --profile gateway up -d +> [!TIP] +> **Utilisateurs Docker** : Par défaut, le Gateway écoute sur `127.0.0.1`, ce qui n'est pas accessible depuis l'hôte. Si vous avez besoin d'accéder aux endpoints de santé ou d'exposer des ports, définissez `PICOCLAW_GATEWAY_HOST=0.0.0.0` dans votre environnement ou mettez à jour `config.json`. + + # 4. Voir les logs docker compose logs -f picoclaw-gateway @@ -222,7 +226,7 @@ picoclaw onboard ], "agents": { "defaults": { - "model": "gpt4" + "model_name": "gpt4" } }, "channels": { diff --git a/README.ja.md b/README.ja.md index 3506c77c2..5a7bb8542 100644 --- a/README.ja.md +++ b/README.ja.md @@ -133,6 +133,10 @@ vim config/config.json # DISCORD_BOT_TOKEN, プロバイダーの API キ # 3. ビルドと起動 docker compose --profile gateway up -d +> [!TIP] +> **Docker ユーザー**: デフォルトでは、Gateway は `127.0.0.1` でリッスンしており、ホストからアクセスできません。ヘルスチェックエンドポイントにアクセスしたり、ポートを公開したりする必要がある場合は、環境変数で `PICOCLAW_GATEWAY_HOST=0.0.0.0` を設定するか、`config.json` を更新してください。 + + # 4. ログ確認 docker compose logs -f picoclaw-gateway @@ -184,7 +188,7 @@ picoclaw onboard ], "agents": { "defaults": { - "model": "gpt4" + "model_name": "gpt4" } }, "channels": { diff --git a/README.md b/README.md index 955255f2e..2b770f215 100644 --- a/README.md +++ b/README.md @@ -12,6 +12,9 @@
Website Twitter +
+ + Discord

[中文](README.zh.md) | [日本語](README.ja.md) | [Português](README.pt-br.md) | [Tiếng Việt](README.vi.md) | [Français](README.fr.md) | **English** @@ -171,6 +174,10 @@ vim config/config.json # Set DISCORD_BOT_TOKEN, API keys, etc. # 3. Build & Start docker compose --profile gateway up -d +> [!TIP] +> **Docker Users**: By default, the Gateway listens on `127.0.0.1` which is not accessible from the host. If you need to access the health endpoints or expose ports, set `PICOCLAW_GATEWAY_HOST=0.0.0.0` in your environment or update `config.json`. + + # 4. Check logs docker compose logs -f picoclaw-gateway @@ -215,7 +222,7 @@ picoclaw onboard "agents": { "defaults": { "workspace": "~/.picoclaw/workspace", - "model": "gpt4", + "model_name": "gpt4", "max_tokens": 8192, "temperature": 0.7, "max_tool_iterations": 20 diff --git a/README.pt-br.md b/README.pt-br.md index 900ee7932..0115b7f89 100644 --- a/README.pt-br.md +++ b/README.pt-br.md @@ -172,6 +172,10 @@ vim config/config.json # Configure DISCORD_BOT_TOKEN, API keys, etc. # 3. Build & Iniciar docker compose --profile gateway up -d +> [!TIP] +> **Usuários Docker**: Por padrão, o Gateway ouve em `127.0.0.1`, o que não é acessível a partir do host. Se você precisar acessar os endpoints de integridade ou expor portas, defina `PICOCLAW_GATEWAY_HOST=0.0.0.0` em seu ambiente ou atualize o `config.json`. + + # 4. Ver logs docker compose logs -f picoclaw-gateway @@ -223,7 +227,7 @@ picoclaw onboard ], "agents": { "defaults": { - "model": "gpt4" + "model_name": "gpt4" } }, "tools": { diff --git a/README.vi.md b/README.vi.md index 29ff12bb0..015bc264e 100644 --- a/README.vi.md +++ b/README.vi.md @@ -152,6 +152,10 @@ vim config/config.json # Thiết lập DISCORD_BOT_TOKEN, API keys, v.v. # 3. Build & Khởi động docker compose --profile gateway up -d +> [!TIP] +> **Người dùng Docker**: Theo mặc định, Gateway lắng nghe trên `127.0.0.1`, không thể truy cập từ máy chủ. Nếu bạn cần truy cập các endpoint kiểm tra sức khỏe hoặc mở cổng, hãy đặt `PICOCLAW_GATEWAY_HOST=0.0.0.0` trong môi trường của bạn hoặc cập nhật `config.json`. + + # 4. Xem logs docker compose logs -f picoclaw-gateway @@ -203,7 +207,7 @@ picoclaw onboard ], "agents": { "defaults": { - "model": "gpt4" + "model_name": "gpt4" } }, "channels": { diff --git a/README.zh.md b/README.zh.md index 17a736fec..4f4bde46a 100644 --- a/README.zh.md +++ b/README.zh.md @@ -173,6 +173,9 @@ vim config/config.json # 设置 DISCORD_BOT_TOKEN, API keys 等 # 3. 构建并启动 docker compose --profile gateway up -d +> [!TIP] +**Docker 用户**: 默认情况下, Gateway监听 `127.0.0.1`，这使得这个端口未暴露到容器外。如果你需要通过端口映射访问健康检查接口, 请在环境变量中设置 `PICOCLAW_GATEWAY_HOST=0.0.0.0` 或修改 `config.json`。 + # 4. 查看日志 docker compose logs -f picoclaw-gateway @@ -221,7 +224,7 @@ picoclaw onboard "agents": { "defaults": { "workspace": "~/.picoclaw/workspace", - "model": "gpt4", + "model_name": "gpt4", "max_tokens": 8192, "temperature": 0.7, "max_tool_iterations": 20 diff --git a/assets/wechat.png b/assets/wechat.png index a34217c33..e30c34e4e 100644 Binary files a/assets/wechat.png and b/assets/wechat.png differ diff --git a/cmd/picoclaw/cmd_agent.go b/cmd/picoclaw/cmd_agent.go index 8658c9d32..98ea51103 100644 --- a/cmd/picoclaw/cmd_agent.go +++ b/cmd/picoclaw/cmd_agent.go @@ -56,7 +56,7 @@ func agentCmd() { } if modelOverride != "" { - cfg.Agents.Defaults.Model = modelOverride + cfg.Agents.Defaults.ModelName = modelOverride } provider, modelID, err := providers.CreateProvider(cfg) @@ -66,7 +66,7 @@ func agentCmd() { } // Use the resolved model ID from provider creation if modelID != "" { - cfg.Agents.Defaults.Model = modelID + cfg.Agents.Defaults.ModelName = modelID } msgBus := bus.NewMessageBus() diff --git a/cmd/picoclaw/cmd_auth.go b/cmd/picoclaw/cmd_auth.go index 729c56177..55eb3cec3 100644 --- a/cmd/picoclaw/cmd_auth.go +++ b/cmd/picoclaw/cmd_auth.go @@ -144,7 +144,7 @@ func authLoginOpenAI(useDeviceCode bool) { } // Update default model to use OpenAI - appCfg.Agents.Defaults.Model = "gpt-5.2" + appCfg.Agents.Defaults.ModelName = "gpt-5.2" if err := config.SaveConfig(getConfigPath(), appCfg); err != nil { fmt.Printf("Warning: could not update config: %v\n", err) @@ -218,7 +218,7 @@ func authLoginGoogleAntigravity() { } // Update default model - appCfg.Agents.Defaults.Model = "gemini-flash" + appCfg.Agents.Defaults.ModelName = "gemini-flash" if err := config.SaveConfig(getConfigPath(), appCfg); err != nil { fmt.Printf("Warning: could not update config: %v\n", err) @@ -292,7 +292,7 @@ func authLoginPasteToken(provider string) { }) } // Update default model - appCfg.Agents.Defaults.Model = "claude-sonnet-4.6" + appCfg.Agents.Defaults.ModelName = "claude-sonnet-4.6" case "openai": appCfg.Providers.OpenAI.AuthMethod = "token" // Update ModelList @@ -312,7 +312,7 @@ func authLoginPasteToken(provider string) { }) } // Update default model - appCfg.Agents.Defaults.Model = "gpt-5.2" + appCfg.Agents.Defaults.ModelName = "gpt-5.2" } if err := config.SaveConfig(getConfigPath(), appCfg); err != nil { fmt.Printf("Warning: could not update config: %v\n", err) @@ -320,7 +320,7 @@ func authLoginPasteToken(provider string) { } fmt.Printf("Token saved for %s!\n", provider) - fmt.Printf("Default model set to: %s\n", appCfg.Agents.Defaults.Model) + fmt.Printf("Default model set to: %s\n", appCfg.Agents.Defaults.GetModelName()) } func authLogoutCmd() { diff --git a/cmd/picoclaw/cmd_gateway.go b/cmd/picoclaw/cmd_gateway.go index 28ef76ad3..cf7f3563a 100644 --- a/cmd/picoclaw/cmd_gateway.go +++ b/cmd/picoclaw/cmd_gateway.go @@ -52,7 +52,7 @@ func gatewayCmd() { } // Use the resolved model ID from provider creation if modelID != "" { - cfg.Agents.Defaults.Model = modelID + cfg.Agents.Defaults.ModelName = modelID } msgBus := bus.NewMessageBus() diff --git a/cmd/picoclaw/cmd_status.go b/cmd/picoclaw/cmd_status.go index 067f32332..f923b0403 100644 --- a/cmd/picoclaw/cmd_status.go +++ b/cmd/picoclaw/cmd_status.go @@ -41,7 +41,7 @@ func statusCmd() { } if _, err := os.Stat(configPath); err == nil { - fmt.Printf("Model: %s\n", cfg.Agents.Defaults.Model) + fmt.Printf("Model: %s\n", cfg.Agents.Defaults.GetModelName()) hasOpenRouter := cfg.Providers.OpenRouter.APIKey != "" hasAnthropic := cfg.Providers.Anthropic.APIKey != "" diff --git a/cmd/picoclaw/main.go b/cmd/picoclaw/main.go index 1e4b393f8..25ad701ca 100644 --- a/cmd/picoclaw/main.go +++ b/cmd/picoclaw/main.go @@ -131,7 +131,7 @@ func main() { workspace := cfg.WorkspacePath() installer := skills.NewSkillInstaller(workspace) - // 获取全局配置目录和内置 skills 目录 + // get global config directory and builtin skills directory globalDir := filepath.Dir(getConfigPath()) globalSkillsDir := filepath.Join(globalDir, "skills") builtinSkillsDir := filepath.Join(globalDir, "picoclaw", "skills") diff --git a/config/config.example.json b/config/config.example.json index b2af3d86c..57f26f8e1 100644 --- a/config/config.example.json +++ b/config/config.example.json @@ -3,7 +3,7 @@ "defaults": { "workspace": "~/.picoclaw/workspace", "restrict_to_workspace": true, - "model": "gpt4", + "model_name": "gpt4", "max_tokens": 8192, "temperature": 0.7, "max_tool_iterations": 20 @@ -203,6 +203,10 @@ "volcengine": { "api_key": "", "api_base": "" + }, + "mistral": { + "api_key": "", + "api_base": "https://api.mistral.ai/v1" } }, "tools": { @@ -250,7 +254,7 @@ "monitor_usb": true }, "gateway": { - "host": "0.0.0.0", + "host": "127.0.0.1", "port": 18790 } } diff --git a/pkg/agent/context.go b/pkg/agent/context.go index a9db5afdd..ba07e33d3 100644 --- a/pkg/agent/context.go +++ b/pkg/agent/context.go @@ -288,25 +288,6 @@ func (cb *ContextBuilder) AddAssistantMessage( return messages } -func (cb *ContextBuilder) loadSkills() string { - allSkills := cb.skillsLoader.ListSkills() - if len(allSkills) == 0 { - return "" - } - - var skillNames []string - for _, s := range allSkills { - skillNames = append(skillNames, s.Name) - } - - content := cb.skillsLoader.LoadSkillsForContext(skillNames) - if content == "" { - return "" - } - - return "# Skill Definitions\n\n" + content -} - // GetSkillsInfo returns information about loaded skills. func (cb *ContextBuilder) GetSkillsInfo() map[string]any { allSkills := cb.skillsLoader.ListSkills() diff --git a/pkg/agent/instance.go b/pkg/agent/instance.go index dfbef9fbc..c6a54c7d2 100644 --- a/pkg/agent/instance.go +++ b/pkg/agent/instance.go @@ -133,7 +133,7 @@ func resolveAgentModel(agentCfg *config.AgentConfig, defaults *config.AgentDefau if agentCfg != nil && agentCfg.Model != nil && strings.TrimSpace(agentCfg.Model.Primary) != "" { return strings.TrimSpace(agentCfg.Model.Primary) } - return defaults.Model + return defaults.GetModelName() } // resolveAgentFallbacks resolves the fallback models for an agent. diff --git a/pkg/agent/loop.go b/pkg/agent/loop.go index bf229ad74..9a2bb1198 100644 --- a/pkg/agent/loop.go +++ b/pkg/agent/loop.go @@ -626,8 +626,9 @@ func (al *AgentLoop) runLLMIteration( // Build assistant message with tool calls assistantMsg := providers.Message{ - Role: "assistant", - Content: response.Content, + Role: "assistant", + Content: response.Content, + ReasoningContent: response.ReasoningContent, } for _, tc := range normalizedToolCalls { argumentsJSON, _ := json.Marshal(tc.Arguments) diff --git a/pkg/channels/qq.go b/pkg/channels/qq.go index e66cac533..b10776db6 100644 --- a/pkg/channels/qq.go +++ b/pkg/channels/qq.go @@ -47,31 +47,31 @@ func (c *QQChannel) Start(ctx context.Context) error { logger.InfoC("qq", "Starting QQ bot (WebSocket mode)") - // 创建 token source + // create token source credentials := &token.QQBotCredentials{ AppID: c.config.AppID, AppSecret: c.config.AppSecret, } c.tokenSource = token.NewQQBotTokenSource(credentials) - // 创建子 context + // create child context c.ctx, c.cancel = context.WithCancel(ctx) - // 启动自动刷新 token 协程 + // start auto-refresh token goroutine if err := token.StartRefreshAccessToken(c.ctx, c.tokenSource); err != nil { return fmt.Errorf("failed to start token refresh: %w", err) } - // 初始化 OpenAPI 客户端 + // initialize OpenAPI client c.api = botgo.NewOpenAPI(c.config.AppID, c.tokenSource).WithTimeout(5 * time.Second) - // 注册事件处理器 + // register event handlers intent := event.RegisterHandlers( c.handleC2CMessage(), c.handleGroupATMessage(), ) - // 获取 WebSocket 接入点 + // get WebSocket endpoint wsInfo, err := c.api.WS(c.ctx, nil, "") if err != nil { return fmt.Errorf("failed to get websocket info: %w", err) @@ -81,10 +81,10 @@ func (c *QQChannel) Start(ctx context.Context) error { "shards": wsInfo.Shards, }) - // 创建并保存 sessionManager + // create and save sessionManager c.sessionManager = botgo.NewSessionManager() - // 在 goroutine 中启动 WebSocket 连接，避免阻塞 + // start WebSocket connection in goroutine to avoid blocking go func() { if err := c.sessionManager.Start(wsInfo, c.tokenSource, &intent); err != nil { logger.ErrorCF("qq", "WebSocket session error", map[string]any{ @@ -116,12 +116,12 @@ func (c *QQChannel) Send(ctx context.Context, msg bus.OutboundMessage) error { return fmt.Errorf("QQ bot not running") } - // 构造消息 + // construct message msgToCreate := &dto.MessageToCreate{ Content: msg.Content, } - // C2C 消息发送 + // send C2C message _, err := c.api.PostC2CMessage(ctx, msg.ChatID, msgToCreate) if err != nil { logger.ErrorCF("qq", "Failed to send C2C message", map[string]any{ @@ -133,15 +133,15 @@ func (c *QQChannel) Send(ctx context.Context, msg bus.OutboundMessage) error { return nil } -// handleC2CMessage 处理 QQ 私聊消息 +// handleC2CMessage handles QQ private messages func (c *QQChannel) handleC2CMessage() event.C2CMessageEventHandler { return func(event *dto.WSPayload, data *dto.WSC2CMessageData) error { - // 去重检查 + // deduplication check if c.isDuplicate(data.ID) { return nil } - // 提取用户信息 + // extract user info var senderID string if data.Author != nil && data.Author.ID != "" { senderID = data.Author.ID @@ -150,7 +150,7 @@ func (c *QQChannel) handleC2CMessage() event.C2CMessageEventHandler { return nil } - // 提取消息内容 + // extract message content content := data.Content if content == "" { logger.DebugC("qq", "Received empty message, ignoring") @@ -162,7 +162,7 @@ func (c *QQChannel) handleC2CMessage() event.C2CMessageEventHandler { "length": len(content), }) - // 转发到消息总线 + // forward to message bus metadata := map[string]string{ "message_id": data.ID, "peer_kind": "direct", @@ -175,15 +175,15 @@ func (c *QQChannel) handleC2CMessage() event.C2CMessageEventHandler { } } -// handleGroupATMessage 处理群@消息 +// handleGroupATMessage handles group @messages func (c *QQChannel) handleGroupATMessage() event.GroupATMessageEventHandler { return func(event *dto.WSPayload, data *dto.WSGroupATMessageData) error { - // 去重检查 + // deduplication check if c.isDuplicate(data.ID) { return nil } - // 提取用户信息 + // extract user info var senderID string if data.Author != nil && data.Author.ID != "" { senderID = data.Author.ID @@ -192,7 +192,7 @@ func (c *QQChannel) handleGroupATMessage() event.GroupATMessageEventHandler { return nil } - // 提取消息内容（去掉 @ 机器人部分） + // extract message content (remove @bot part) content := data.Content if content == "" { logger.DebugC("qq", "Received empty group message, ignoring") @@ -205,7 +205,7 @@ func (c *QQChannel) handleGroupATMessage() event.GroupATMessageEventHandler { "length": len(content), }) - // 转发到消息总线（使用 GroupID 作为 ChatID） + // forward to message bus (use GroupID as ChatID) metadata := map[string]string{ "message_id": data.ID, "group_id": data.GroupID, @@ -219,7 +219,7 @@ func (c *QQChannel) handleGroupATMessage() event.GroupATMessageEventHandler { } } -// isDuplicate 检查消息是否重复 +// isDuplicate checks if message is duplicate func (c *QQChannel) isDuplicate(messageID string) bool { c.mu.Lock() defer c.mu.Unlock() @@ -230,9 +230,9 @@ func (c *QQChannel) isDuplicate(messageID string) bool { c.processedIDs[messageID] = true - // 简单清理：限制 map 大小 + // simple cleanup: limit map size if len(c.processedIDs) > 10000 { - // 清空一半 + // clear half count := 0 for id := range c.processedIDs { if count >= 5000 { diff --git a/pkg/channels/slack.go b/pkg/channels/slack.go index f7359cd6d..f087aa8da 100644 --- a/pkg/channels/slack.go +++ b/pkg/channels/slack.go @@ -200,7 +200,7 @@ func (c *SlackChannel) handleMessageEvent(ev *slackevents.MessageEvent) { return } - // 检查白名单，避免为被拒绝的用户下载附件 + // check allowlist to avoid downloading attachments for rejected users if !c.IsAllowed(ev.User) { logger.DebugCF("slack", "Message rejected by allowlist", map[string]any{ "user_id": ev.User, @@ -232,9 +232,9 @@ func (c *SlackChannel) handleMessageEvent(ev *slackevents.MessageEvent) { content = c.stripBotMention(content) var mediaPaths []string - localFiles := []string{} // 跟踪需要清理的本地文件 + localFiles := []string{} // track local files that need cleanup - // 确保临时文件在函数返回时被清理 + // ensure temp files are cleaned up when function returns defer func() { for _, file := range localFiles { if err := os.Remove(file); err != nil { diff --git a/pkg/channels/telegram.go b/pkg/channels/telegram.go index a0a1c8d0a..5cd51e8bc 100644 --- a/pkg/channels/telegram.go +++ b/pkg/channels/telegram.go @@ -208,7 +208,7 @@ func (c *TelegramChannel) handleMessage(ctx context.Context, message *telego.Mes senderID = fmt.Sprintf("%d|%s", user.ID, user.Username) } - // 检查白名单，避免为被拒绝的用户下载附件 + // check allowlist to avoid downloading attachments for rejected users if !c.IsAllowed(senderID) { logger.DebugCF("telegram", "Message rejected by allowlist", map[string]any{ "user_id": senderID, @@ -221,9 +221,9 @@ func (c *TelegramChannel) handleMessage(ctx context.Context, message *telego.Mes content := "" mediaPaths := []string{} - localFiles := []string{} // 跟踪需要清理的本地文件 + localFiles := []string{} // track local files that need cleanup - // 确保临时文件在函数返回时被清理 + // ensure temp files are cleaned up when function returns defer func() { for _, file := range localFiles { if err := os.Remove(file); err != nil { diff --git a/pkg/channels/telegram_commands.go b/pkg/channels/telegram_commands.go index a084b641b..f28434f46 100644 --- a/pkg/channels/telegram_commands.go +++ b/pkg/channels/telegram_commands.go @@ -81,7 +81,7 @@ func (c *cmd) Show(ctx context.Context, message telego.Message) error { switch args { case "model": response = fmt.Sprintf("Current Model: %s (Provider: %s)", - c.config.Agents.Defaults.Model, + c.config.Agents.Defaults.GetModelName(), c.config.Agents.Defaults.Provider) case "channel": response = "Current Channel: telegram" @@ -120,7 +120,7 @@ func (c *cmd) List(ctx context.Context, message telego.Message) error { provider = "configured default" } response = fmt.Sprintf("Configured Model: %s\nProvider: %s\n\nTo change models, update config.yaml", - c.config.Agents.Defaults.Model, provider) + c.config.Agents.Defaults.GetModelName(), provider) case "channels": var enabled []string diff --git a/pkg/channels/wecom_app.go b/pkg/channels/wecom_app.go index 715c48707..302603445 100644 --- a/pkg/channels/wecom_app.go +++ b/pkg/channels/wecom_app.go @@ -571,61 +571,6 @@ func (c *WeComAppChannel) sendTextMessage(ctx context.Context, accessToken, user return nil } -// sendMarkdownMessage sends a markdown message to a user -func (c *WeComAppChannel) sendMarkdownMessage(ctx context.Context, accessToken, userID, content string) error { - apiURL := fmt.Sprintf("%s/cgi-bin/message/send?access_token=%s", wecomAPIBase, accessToken) - - msg := WeComMarkdownMessage{ - ToUser: userID, - MsgType: "markdown", - AgentID: c.config.AgentID, - } - msg.Markdown.Content = content - - jsonData, err := json.Marshal(msg) - if err != nil { - return fmt.Errorf("failed to marshal message: %w", err) - } - - // Use configurable timeout (default 5 seconds) - timeout := c.config.ReplyTimeout - if timeout <= 0 { - timeout = 5 - } - - reqCtx, cancel := context.WithTimeout(ctx, time.Duration(timeout)*time.Second) - defer cancel() - - req, err := http.NewRequestWithContext(reqCtx, http.MethodPost, apiURL, bytes.NewBuffer(jsonData)) - if err != nil { - return fmt.Errorf("failed to create request: %w", err) - } - req.Header.Set("Content-Type", "application/json") - - client := &http.Client{Timeout: time.Duration(timeout) * time.Second} - resp, err := client.Do(req) - if err != nil { - return fmt.Errorf("failed to send message: %w", err) - } - defer resp.Body.Close() - - body, err := io.ReadAll(resp.Body) - if err != nil { - return fmt.Errorf("failed to read response: %w", err) - } - - var sendResp WeComSendMessageResponse - if err := json.Unmarshal(body, &sendResp); err != nil { - return fmt.Errorf("failed to parse response: %w", err) - } - - if sendResp.ErrCode != 0 { - return fmt.Errorf("API error: %s (code: %d)", sendResp.ErrMsg, sendResp.ErrCode) - } - - return nil -} - // handleHealth handles health check requests func (c *WeComAppChannel) handleHealth(w http.ResponseWriter, r *http.Request) { status := map[string]any{ diff --git a/pkg/config/config.go b/pkg/config/config.go index f7053cd71..818472182 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -170,7 +170,8 @@ type AgentDefaults struct { Workspace string `json:"workspace" env:"PICOCLAW_AGENTS_DEFAULTS_WORKSPACE"` RestrictToWorkspace bool `json:"restrict_to_workspace" env:"PICOCLAW_AGENTS_DEFAULTS_RESTRICT_TO_WORKSPACE"` Provider string `json:"provider" env:"PICOCLAW_AGENTS_DEFAULTS_PROVIDER"` - Model string `json:"model" env:"PICOCLAW_AGENTS_DEFAULTS_MODEL"` + ModelName string `json:"model_name,omitempty" env:"PICOCLAW_AGENTS_DEFAULTS_MODEL_NAME"` + Model string `json:"model,omitempty" env:"PICOCLAW_AGENTS_DEFAULTS_MODEL"` // Deprecated: use model_name instead ModelFallbacks []string `json:"model_fallbacks,omitempty"` ImageModel string `json:"image_model,omitempty" env:"PICOCLAW_AGENTS_DEFAULTS_IMAGE_MODEL"` ImageModelFallbacks []string `json:"image_model_fallbacks,omitempty"` @@ -179,6 +180,15 @@ type AgentDefaults struct { MaxToolIterations int `json:"max_tool_iterations" env:"PICOCLAW_AGENTS_DEFAULTS_MAX_TOOL_ITERATIONS"` } +// GetModelName returns the effective model name for the agent defaults. +// It prefers the new "model_name" field but falls back to "model" for backward compatibility. +func (d *AgentDefaults) GetModelName() string { + if d.ModelName != "" { + return d.ModelName + } + return d.Model +} + type ChannelsConfig struct { WhatsApp WhatsAppConfig `json:"whatsapp"` Telegram TelegramConfig `json:"telegram"` @@ -333,6 +343,7 @@ type ProvidersConfig struct { GitHubCopilot ProviderConfig `json:"github_copilot"` Antigravity ProviderConfig `json:"antigravity"` Qwen ProviderConfig `json:"qwen"` + Mistral ProviderConfig `json:"mistral"` } // IsEmpty checks if all provider configs are empty (no API keys or API bases set) @@ -354,7 +365,8 @@ func (p ProvidersConfig) IsEmpty() bool { p.VolcEngine.APIKey == "" && p.VolcEngine.APIBase == "" && p.GitHubCopilot.APIKey == "" && p.GitHubCopilot.APIBase == "" && p.Antigravity.APIKey == "" && p.Antigravity.APIBase == "" && - p.Qwen.APIKey == "" && p.Qwen.APIBase == "" + p.Qwen.APIKey == "" && p.Qwen.APIBase == "" && + p.Mistral.APIKey == "" && p.Mistral.APIBase == "" } // MarshalJSON implements custom JSON marshaling for ProvidersConfig @@ -506,6 +518,20 @@ func LoadConfig(path string) (*Config, error) { return nil, err } + // Pre-scan the JSON to check how many model_list entries the user provided. + // Go's JSON decoder reuses existing slice backing-array elements rather than + // zero-initializing them, so fields absent from the user's JSON (e.g. api_base) + // would silently inherit values from the DefaultConfig template at the same + // index position. We only reset cfg.ModelList when the user actually provides + // entries; when count is 0 we keep DefaultConfig's built-in list as fallback. + var tmp Config + if err := json.Unmarshal(data, &tmp); err != nil { + return nil, err + } + if len(tmp.ModelList) > 0 { + cfg.ModelList = nil + } + if err := json.Unmarshal(data, cfg); err != nil { return nil, err } @@ -653,7 +679,8 @@ func (c *Config) HasProvidersConfig() bool { v.VolcEngine.APIKey != "" || v.VolcEngine.APIBase != "" || v.GitHubCopilot.APIKey != "" || v.GitHubCopilot.APIBase != "" || v.Antigravity.APIKey != "" || v.Antigravity.APIBase != "" || - v.Qwen.APIKey != "" || v.Qwen.APIBase != "" + v.Qwen.APIKey != "" || v.Qwen.APIBase != "" || + v.Mistral.APIKey != "" || v.Mistral.APIBase != "" } // ValidateModelList validates all ModelConfig entries in the model_list. diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index 0898217d6..f88c0269c 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -246,7 +246,7 @@ func TestDefaultConfig_Temperature(t *testing.T) { func TestDefaultConfig_Gateway(t *testing.T) { cfg := DefaultConfig() - if cfg.Gateway.Host != "0.0.0.0" { + if cfg.Gateway.Host != "127.0.0.1" { t.Error("Gateway host should have default value") } if cfg.Gateway.Port == 0 { @@ -343,7 +343,7 @@ func TestConfig_Complete(t *testing.T) { if cfg.Agents.Defaults.MaxToolIterations == 0 { t.Error("MaxToolIterations should not be zero") } - if cfg.Gateway.Host != "0.0.0.0" { + if cfg.Gateway.Host != "127.0.0.1" { t.Error("Gateway host should have default value") } if cfg.Gateway.Port == 0 { diff --git a/pkg/config/defaults.go b/pkg/config/defaults.go index 65bb0f1de..fb64ed400 100644 --- a/pkg/config/defaults.go +++ b/pkg/config/defaults.go @@ -262,6 +262,14 @@ func DefaultConfig() *Config { APIKey: "ollama", }, + // Mistral AI - https://console.mistral.ai/api-keys + { + ModelName: "mistral-small", + Model: "mistral/mistral-small-latest", + APIBase: "https://api.mistral.ai/v1", + APIKey: "", + }, + // VLLM (local) - http://localhost:8000 { ModelName: "local-model", @@ -271,7 +279,7 @@ func DefaultConfig() *Config { }, }, Gateway: GatewayConfig{ - Host: "0.0.0.0", + Host: "127.0.0.1", Port: 18790, }, Tools: ToolsConfig{ diff --git a/pkg/config/migration.go b/pkg/config/migration.go index 689e2312f..70e1de438 100644 --- a/pkg/config/migration.go +++ b/pkg/config/migration.go @@ -41,7 +41,7 @@ func ConvertProvidersToModelList(cfg *Config) []ModelConfig { // Get user's configured provider and model userProvider := strings.ToLower(cfg.Agents.Defaults.Provider) - userModel := cfg.Agents.Defaults.Model + userModel := cfg.Agents.Defaults.GetModelName() p := cfg.Providers @@ -324,6 +324,22 @@ func ConvertProvidersToModelList(cfg *Config) []ModelConfig { }, true }, }, + { + providerNames: []string{"mistral"}, + protocol: "mistral", + buildConfig: func(p ProvidersConfig) (ModelConfig, bool) { + if p.Mistral.APIKey == "" && p.Mistral.APIBase == "" { + return ModelConfig{}, false + } + return ModelConfig{ + ModelName: "mistral", + Model: "mistral/mistral-small-latest", + APIKey: p.Mistral.APIKey, + APIBase: p.Mistral.APIBase, + Proxy: p.Mistral.Proxy, + }, true + }, + }, } // Process each provider migration diff --git a/pkg/config/migration_test.go b/pkg/config/migration_test.go index 1e8139e68..42165cb71 100644 --- a/pkg/config/migration_test.go +++ b/pkg/config/migration_test.go @@ -131,14 +131,15 @@ func TestConvertProvidersToModelList_AllProviders(t *testing.T) { GitHubCopilot: ProviderConfig{ConnectMode: "grpc"}, Antigravity: ProviderConfig{AuthMethod: "oauth"}, Qwen: ProviderConfig{APIKey: "key17"}, + Mistral: ProviderConfig{APIKey: "key18"}, }, } result := ConvertProvidersToModelList(cfg) - // All 17 providers should be converted - if len(result) != 17 { - t.Errorf("len(result) = %d, want 17", len(result)) + // All 18 providers should be converted + if len(result) != 18 { + t.Errorf("len(result) = %d, want 18", len(result)) } } diff --git a/pkg/config/model_config_test.go b/pkg/config/model_config_test.go index 3c411dc0f..99eea2782 100644 --- a/pkg/config/model_config_test.go +++ b/pkg/config/model_config_test.go @@ -6,6 +6,7 @@ package config import ( + "encoding/json" "strings" "sync" "testing" @@ -114,6 +115,137 @@ func TestGetModelConfig_Concurrent(t *testing.T) { } } +func TestAgentDefaults_GetModelName_BackwardCompat(t *testing.T) { + tests := []struct { + name string + defaults AgentDefaults + wantName string + }{ + { + name: "new model_name field only", + defaults: AgentDefaults{ModelName: "new-model"}, + wantName: "new-model", + }, + { + name: "old model field only", + defaults: AgentDefaults{Model: "legacy-model"}, + wantName: "legacy-model", + }, + { + name: "both fields - model_name takes precedence", + defaults: AgentDefaults{ModelName: "new-model", Model: "old-model"}, + wantName: "new-model", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := tt.defaults.GetModelName(); got != tt.wantName { + t.Errorf("GetModelName() = %q, want %q", got, tt.wantName) + } + }) + } +} + +func TestAgentDefaults_JSON_BackwardCompat(t *testing.T) { + tests := []struct { + name string + json string + wantName string + }{ + { + name: "new model_name field", + json: `{"model_name": "gpt4"}`, + wantName: "gpt4", + }, + { + name: "old model field", + json: `{"model": "gpt4"}`, + wantName: "gpt4", + }, + { + name: "both fields - model_name wins", + json: `{"model_name": "new", "model": "old"}`, + wantName: "new", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var defaults AgentDefaults + if err := json.Unmarshal([]byte(tt.json), &defaults); err != nil { + t.Fatalf("Unmarshal error: %v", err) + } + if got := defaults.GetModelName(); got != tt.wantName { + t.Errorf("GetModelName() = %q, want %q", got, tt.wantName) + } + }) + } +} + +func TestFullConfig_JSON_BackwardCompat(t *testing.T) { + // Test complete config with both old and new formats + oldFormat := `{ + "agents": { + "defaults": { + "workspace": "~/.picoclaw/workspace", + "model": "gpt4", + "max_tokens": 4096 + } + }, + "model_list": [ + { + "model_name": "gpt4", + "model": "openai/gpt-4o", + "api_key": "test-key" + } + ] + }` + + newFormat := `{ + "agents": { + "defaults": { + "workspace": "~/.picoclaw/workspace", + "model_name": "gpt4", + "max_tokens": 4096 + } + }, + "model_list": [ + { + "model_name": "gpt4", + "model": "openai/gpt-4o", + "api_key": "test-key" + } + ] + }` + + for name, jsonStr := range map[string]string{ + "old format (model)": oldFormat, + "new format (model_name)": newFormat, + } { + t.Run(name, func(t *testing.T) { + cfg := &Config{} + if err := json.Unmarshal([]byte(jsonStr), cfg); err != nil { + t.Fatalf("Unmarshal error: %v", err) + } + + // Check that GetModelName returns correct value + if got := cfg.Agents.Defaults.GetModelName(); got != "gpt4" { + t.Errorf("GetModelName() = %q, want %q", got, "gpt4") + } + + // Check that GetModelConfig works + modelCfg, err := cfg.GetModelConfig("gpt4") + if err != nil { + t.Fatalf("GetModelConfig error: %v", err) + } + if modelCfg.Model != "openai/gpt-4o" { + t.Errorf("Model = %q, want %q", modelCfg.Model, "openai/gpt-4o") + } + }) + } +} + func TestModelConfig_Validate(t *testing.T) { tests := []struct { name string diff --git a/pkg/devices/sources/usb_linux.go b/pkg/devices/sources/usb_linux.go index be0193cfb..2bb38941f 100644 --- a/pkg/devices/sources/usb_linux.go +++ b/pkg/devices/sources/usb_linux.go @@ -35,9 +35,8 @@ var usbClassToCapability = map[string]string{ } type USBMonitor struct { - cmd *exec.Cmd - cancel context.CancelFunc - mu sync.Mutex + cmd *exec.Cmd + mu sync.Mutex } func NewUSBMonitor() *USBMonitor { diff --git a/pkg/logger/logger.go b/pkg/logger/logger.go index 54de66bf9..c14fbd464 100644 --- a/pkg/logger/logger.go +++ b/pkg/logger/logger.go @@ -119,13 +119,15 @@ func logMessage(level LogLevel, component string, message string, fields map[str if logger.file != nil { jsonData, err := json.Marshal(entry) if err == nil { - logger.file.WriteString(string(jsonData) + "\n") + logger.file.Write(append(jsonData, '\n')) } } var fieldStr string if len(fields) > 0 { fieldStr = " " + formatFields(fields) + } else { + fieldStr = "" } logLine := fmt.Sprintf("[%s] [%s]%s %s%s", diff --git a/pkg/migrate/config.go b/pkg/migrate/config.go index 2237a1429..869b39827 100644 --- a/pkg/migrate/config.go +++ b/pkg/migrate/config.go @@ -22,6 +22,7 @@ var supportedProviders = map[string]bool{ "qwen": true, "deepseek": true, "github_copilot": true, + "mistral": true, } var supportedChannels = map[string]bool{ @@ -72,7 +73,10 @@ func ConvertConfig(data map[string]any) (*config.Config, []string, error) { if agents, ok := getMap(data, "agents"); ok { if defaults, ok := getMap(agents, "defaults"); ok { - if v, ok := getString(defaults, "model"); ok { + // Prefer model_name, fallback to model for backward compatibility + if v, ok := getString(defaults, "model_name"); ok { + cfg.Agents.Defaults.ModelName = v + } else if v, ok := getString(defaults, "model"); ok { cfg.Agents.Defaults.Model = v } if v, ok := getFloat(defaults, "max_tokens"); ok { diff --git a/pkg/providers/antigravity_provider.go b/pkg/providers/antigravity_provider.go index cff67c88c..d4ee528b7 100644 --- a/pkg/providers/antigravity_provider.go +++ b/pkg/providers/antigravity_provider.go @@ -404,64 +404,6 @@ type antigravityJSONResponse struct { } `json:"usageMetadata"` } -func (p *AntigravityProvider) parseJSONResponse(body []byte) (*LLMResponse, error) { - var resp antigravityJSONResponse - if err := json.Unmarshal(body, &resp); err != nil { - return nil, fmt.Errorf("parsing antigravity response: %w", err) - } - - if len(resp.Candidates) == 0 { - return nil, fmt.Errorf("antigravity: no candidates in response") - } - - candidate := resp.Candidates[0] - var contentParts []string - var toolCalls []ToolCall - - for _, part := range candidate.Content.Parts { - if part.Text != "" { - contentParts = append(contentParts, part.Text) - } - if part.FunctionCall != nil { - argumentsJSON, _ := json.Marshal(part.FunctionCall.Args) - toolCalls = append(toolCalls, ToolCall{ - ID: fmt.Sprintf("call_%s_%d", part.FunctionCall.Name, time.Now().UnixNano()), - Name: part.FunctionCall.Name, - Arguments: part.FunctionCall.Args, - Function: &FunctionCall{ - Name: part.FunctionCall.Name, - Arguments: string(argumentsJSON), - ThoughtSignature: extractPartThoughtSignature(part.ThoughtSignature, part.ThoughtSignatureSnake), - }, - }) - } - } - - finishReason := "stop" - if len(toolCalls) > 0 { - finishReason = "tool_calls" - } - if candidate.FinishReason == "MAX_TOKENS" { - finishReason = "length" - } - - var usage *UsageInfo - if resp.UsageMetadata.TotalTokenCount > 0 { - usage = &UsageInfo{ - PromptTokens: resp.UsageMetadata.PromptTokenCount, - CompletionTokens: resp.UsageMetadata.CandidatesTokenCount, - TotalTokens: resp.UsageMetadata.TotalTokenCount, - } - } - - return &LLMResponse{ - Content: strings.Join(contentParts, ""), - ToolCalls: toolCalls, - FinishReason: finishReason, - Usage: usage, - }, nil -} - func (p *AntigravityProvider) parseSSEResponse(body string) (*LLMResponse, error) { var contentParts []string var toolCalls []ToolCall diff --git a/pkg/providers/factory.go b/pkg/providers/factory.go index b6f1b5e21..11af14da4 100644 --- a/pkg/providers/factory.go +++ b/pkg/providers/factory.go @@ -36,7 +36,7 @@ type providerSelection struct { } func resolveProviderSelection(cfg *config.Config) (providerSelection, error) { - model := cfg.Agents.Defaults.Model + model := cfg.Agents.Defaults.GetModelName() providerName := strings.ToLower(cfg.Agents.Defaults.Provider) lowerModel := strings.ToLower(model) @@ -172,6 +172,15 @@ func resolveProviderSelection(cfg *config.Config) (providerSelection, error) { sel.model = "deepseek-chat" } } + case "mistral": + if cfg.Providers.Mistral.APIKey != "" { + sel.apiKey = cfg.Providers.Mistral.APIKey + sel.apiBase = cfg.Providers.Mistral.APIBase + sel.proxy = cfg.Providers.Mistral.Proxy + if sel.apiBase == "" { + sel.apiBase = "https://api.mistral.ai/v1" + } + } case "github_copilot", "copilot": sel.providerType = providerTypeGitHubCopilot if cfg.Providers.GitHubCopilot.APIBase != "" { @@ -275,6 +284,13 @@ func resolveProviderSelection(cfg *config.Config) (providerSelection, error) { if sel.apiBase == "" { sel.apiBase = "http://localhost:11434/v1" } + case (strings.Contains(lowerModel, "mistral") || strings.HasPrefix(model, "mistral/")) && cfg.Providers.Mistral.APIKey != "": + sel.apiKey = cfg.Providers.Mistral.APIKey + sel.apiBase = cfg.Providers.Mistral.APIBase + sel.proxy = cfg.Providers.Mistral.Proxy + if sel.apiBase == "" { + sel.apiBase = "https://api.mistral.ai/v1" + } case cfg.Providers.VLLM.APIBase != "": sel.apiKey = cfg.Providers.VLLM.APIKey sel.apiBase = cfg.Providers.VLLM.APIBase diff --git a/pkg/providers/factory_provider.go b/pkg/providers/factory_provider.go index 74fe8a36c..7d5566eef 100644 --- a/pkg/providers/factory_provider.go +++ b/pkg/providers/factory_provider.go @@ -88,7 +88,7 @@ func CreateProviderFromConfig(cfg *config.ModelConfig) (LLMProvider, string, err case "openrouter", "groq", "zhipu", "gemini", "nvidia", "ollama", "moonshot", "shengsuanyun", "deepseek", "cerebras", - "volcengine", "vllm", "qwen": + "volcengine", "vllm", "qwen", "mistral": // All other OpenAI-compatible HTTP providers if cfg.APIKey == "" && cfg.APIBase == "" { return nil, "", fmt.Errorf("api_key or api_base is required for HTTP-based protocol %q", protocol) @@ -186,6 +186,8 @@ func getDefaultAPIBase(protocol string) string { return "https://dashscope.aliyuncs.com/compatible-mode/v1" case "vllm": return "http://localhost:8000/v1" + case "mistral": + return "https://api.mistral.ai/v1" default: return "" } diff --git a/pkg/providers/fallback_test.go b/pkg/providers/fallback_test.go index e872c672e..ebba054ef 100644 --- a/pkg/providers/fallback_test.go +++ b/pkg/providers/fallback_test.go @@ -17,12 +17,6 @@ func successRun(content string) func(ctx context.Context, provider, model string } } -func failRun(err error) func(ctx context.Context, provider, model string) (*LLMResponse, error) { - return func(ctx context.Context, provider, model string) (*LLMResponse, error) { - return nil, err - } -} - func TestFallback_SingleCandidate_Success(t *testing.T) { ct := NewCooldownTracker() fc := NewFallbackChain(ct) diff --git a/pkg/providers/legacy_provider.go b/pkg/providers/legacy_provider.go index eb13cec65..23f137538 100644 --- a/pkg/providers/legacy_provider.go +++ b/pkg/providers/legacy_provider.go @@ -16,7 +16,7 @@ import ( // The old providers config is automatically converted to model_list during config loading. // Returns the provider, the model ID to use, and any error. func CreateProvider(cfg *config.Config) (LLMProvider, string, error) { - model := cfg.Agents.Defaults.Model + model := cfg.Agents.Defaults.GetModelName() // Ensure model_list is populated (should be done by LoadConfig, but handle edge cases) if len(cfg.ModelList) == 0 && cfg.HasProvidersConfig() { diff --git a/pkg/providers/openai_compat/provider.go b/pkg/providers/openai_compat/provider.go index b8528953a..d2412ae1b 100644 --- a/pkg/providers/openai_compat/provider.go +++ b/pkg/providers/openai_compat/provider.go @@ -148,8 +148,9 @@ func parseResponse(body []byte) (*LLMResponse, error) { var apiResponse struct { Choices []struct { Message struct { - Content string `json:"content"` - ToolCalls []struct { + Content string `json:"content"` + ReasoningContent string `json:"reasoning_content"` + ToolCalls []struct { ID string `json:"id"` Type string `json:"type"` Function *struct { @@ -221,10 +222,11 @@ func parseResponse(body []byte) (*LLMResponse, error) { } return &LLMResponse{ - Content: choice.Message.Content, - ToolCalls: toolCalls, - FinishReason: choice.FinishReason, - Usage: apiResponse.Usage, + Content: choice.Message.Content, + ReasoningContent: choice.Message.ReasoningContent, + ToolCalls: toolCalls, + FinishReason: choice.FinishReason, + Usage: apiResponse.Usage, }, nil } @@ -240,7 +242,7 @@ func normalizeModel(model, apiBase string) string { prefix := strings.ToLower(model[:idx]) switch prefix { - case "moonshot", "nvidia", "groq", "ollama", "deepseek", "google", "openrouter", "zhipu": + case "moonshot", "nvidia", "groq", "ollama", "deepseek", "google", "openrouter", "zhipu", "mistral": return model[idx+1:] default: return model diff --git a/pkg/providers/openai_compat/provider_test.go b/pkg/providers/openai_compat/provider_test.go index 42f9d42ab..594a48213 100644 --- a/pkg/providers/openai_compat/provider_test.go +++ b/pkg/providers/openai_compat/provider_test.go @@ -101,6 +101,50 @@ func TestProviderChat_ParsesToolCalls(t *testing.T) { } } +func TestProviderChat_ParsesReasoningContent(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + resp := map[string]any{ + "choices": []map[string]any{ + { + "message": map[string]any{ + "content": "The answer is 2", + "reasoning_content": "Let me think step by step... 1+1=2", + "tool_calls": []map[string]any{ + { + "id": "call_1", + "type": "function", + "function": map[string]any{ + "name": "calculator", + "arguments": "{\"expr\":\"1+1\"}", + }, + }, + }, + }, + "finish_reason": "tool_calls", + }, + }, + } + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode(resp) + })) + defer server.Close() + + p := NewProvider("key", server.URL, "") + out, err := p.Chat(t.Context(), []Message{{Role: "user", Content: "1+1=?"}}, nil, "kimi-k2.5", nil) + if err != nil { + t.Fatalf("Chat() error = %v", err) + } + if out.ReasoningContent != "Let me think step by step... 1+1=2" { + t.Fatalf("ReasoningContent = %q, want %q", out.ReasoningContent, "Let me think step by step... 1+1=2") + } + if out.Content != "The answer is 2" { + t.Fatalf("Content = %q, want %q", out.Content, "The answer is 2") + } + if len(out.ToolCalls) != 1 { + t.Fatalf("len(ToolCalls) = %d, want 1", len(out.ToolCalls)) + } +} + func TestProviderChat_HTTPError(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { http.Error(w, "bad request", http.StatusBadRequest) diff --git a/pkg/providers/protocoltypes/types.go b/pkg/providers/protocoltypes/types.go index 3a089ca47..1d0ea6edd 100644 --- a/pkg/providers/protocoltypes/types.go +++ b/pkg/providers/protocoltypes/types.go @@ -4,8 +4,8 @@ type ToolCall struct { ID string `json:"id"` Type string `json:"type,omitempty"` Function *FunctionCall `json:"function,omitempty"` - Name string `json:"name,omitempty"` - Arguments map[string]any `json:"arguments,omitempty"` + Name string `json:"-"` + Arguments map[string]any `json:"-"` ThoughtSignature string `json:"-"` // Internal use only ExtraContent *ExtraContent `json:"extra_content,omitempty"` } @@ -25,10 +25,11 @@ type FunctionCall struct { } type LLMResponse struct { - Content string `json:"content"` - ToolCalls []ToolCall `json:"tool_calls,omitempty"` - FinishReason string `json:"finish_reason"` - Usage *UsageInfo `json:"usage,omitempty"` + Content string `json:"content"` + ReasoningContent string `json:"reasoning_content,omitempty"` + ToolCalls []ToolCall `json:"tool_calls,omitempty"` + FinishReason string `json:"finish_reason"` + Usage *UsageInfo `json:"usage,omitempty"` } type UsageInfo struct { @@ -38,10 +39,11 @@ type UsageInfo struct { } type Message struct { - Role string `json:"role"` - Content string `json:"content"` - ToolCalls []ToolCall `json:"tool_calls,omitempty"` - ToolCallID string `json:"tool_call_id,omitempty"` + Role string `json:"role"` + Content string `json:"content"` + ReasoningContent string `json:"reasoning_content,omitempty"` + ToolCalls []ToolCall `json:"tool_calls,omitempty"` + ToolCallID string `json:"tool_call_id,omitempty"` } type ToolDefinition struct { diff --git a/pkg/skills/loader.go b/pkg/skills/loader.go index eb0d5f322..5749d8983 100644 --- a/pkg/skills/loader.go +++ b/pkg/skills/loader.go @@ -55,9 +55,9 @@ func (info SkillInfo) validate() error { type SkillsLoader struct { workspace string - workspaceSkills string // workspace skills (项目级别) - globalSkills string // 全局 skills (~/.picoclaw/skills) - builtinSkills string // 内置 skills + workspaceSkills string // workspace skills (project-level) + globalSkills string // global skills (~/.picoclaw/skills) + builtinSkills string // builtin skills } func NewSkillsLoader(workspace string, globalSkills string, builtinSkills string) *SkillsLoader { @@ -71,118 +71,56 @@ func NewSkillsLoader(workspace string, globalSkills string, builtinSkills string func (sl *SkillsLoader) ListSkills() []SkillInfo { skills := make([]SkillInfo, 0) + seen := make(map[string]bool) - if sl.workspaceSkills != "" { - if dirs, err := os.ReadDir(sl.workspaceSkills); err == nil { - for _, dir := range dirs { - if dir.IsDir() { - skillFile := filepath.Join(sl.workspaceSkills, dir.Name(), "SKILL.md") - if _, err := os.Stat(skillFile); err == nil { - info := SkillInfo{ - Name: dir.Name(), - Path: skillFile, - Source: "workspace", - } - metadata := sl.getSkillMetadata(skillFile) - if metadata != nil { - info.Description = metadata.Description - info.Name = metadata.Name - } - if err := info.validate(); err != nil { - slog.Warn("invalid skill from workspace", "name", info.Name, "error", err) - continue - } - skills = append(skills, info) - } - } + addSkills := func(dir, source string) { + if dir == "" { + return + } + dirs, err := os.ReadDir(dir) + if err != nil { + return + } + for _, d := range dirs { + if !d.IsDir() { + continue } + skillFile := filepath.Join(dir, d.Name(), "SKILL.md") + if _, err := os.Stat(skillFile); err != nil { + continue + } + info := SkillInfo{ + Name: d.Name(), + Path: skillFile, + Source: source, + } + metadata := sl.getSkillMetadata(skillFile) + if metadata != nil { + info.Description = metadata.Description + info.Name = metadata.Name + } + if err := info.validate(); err != nil { + slog.Warn("invalid skill from "+source, "name", info.Name, "error", err) + continue + } + if seen[info.Name] { + continue + } + seen[info.Name] = true + skills = append(skills, info) } } - // 全局 skills (~/.picoclaw/skills) - 被 workspace skills 覆盖 - if sl.globalSkills != "" { - if dirs, err := os.ReadDir(sl.globalSkills); err == nil { - for _, dir := range dirs { - if dir.IsDir() { - skillFile := filepath.Join(sl.globalSkills, dir.Name(), "SKILL.md") - if _, err := os.Stat(skillFile); err == nil { - // 检查是否已被 workspace skills 覆盖 - exists := false - for _, s := range skills { - if s.Name == dir.Name() && s.Source == "workspace" { - exists = true - break - } - } - if exists { - continue - } - - info := SkillInfo{ - Name: dir.Name(), - Path: skillFile, - Source: "global", - } - metadata := sl.getSkillMetadata(skillFile) - if metadata != nil { - info.Description = metadata.Description - info.Name = metadata.Name - } - if err := info.validate(); err != nil { - slog.Warn("invalid skill from global", "name", info.Name, "error", err) - continue - } - skills = append(skills, info) - } - } - } - } - } - - if sl.builtinSkills != "" { - if dirs, err := os.ReadDir(sl.builtinSkills); err == nil { - for _, dir := range dirs { - if dir.IsDir() { - skillFile := filepath.Join(sl.builtinSkills, dir.Name(), "SKILL.md") - if _, err := os.Stat(skillFile); err == nil { - // 检查是否已被 workspace 或 global skills 覆盖 - exists := false - for _, s := range skills { - if s.Name == dir.Name() && (s.Source == "workspace" || s.Source == "global") { - exists = true - break - } - } - if exists { - continue - } - - info := SkillInfo{ - Name: dir.Name(), - Path: skillFile, - Source: "builtin", - } - metadata := sl.getSkillMetadata(skillFile) - if metadata != nil { - info.Description = metadata.Description - info.Name = metadata.Name - } - if err := info.validate(); err != nil { - slog.Warn("invalid skill from builtin", "name", info.Name, "error", err) - continue - } - skills = append(skills, info) - } - } - } - } - } + // Priority: workspace > global > builtin + addSkills(sl.workspaceSkills, "workspace") + addSkills(sl.globalSkills, "global") + addSkills(sl.builtinSkills, "builtin") return skills } func (sl *SkillsLoader) LoadSkill(name string) (string, bool) { - // 1. 优先从 workspace skills 加载（项目级别） + // 1. load from workspace skills first (project-level) if sl.workspaceSkills != "" { skillFile := filepath.Join(sl.workspaceSkills, name, "SKILL.md") if content, err := os.ReadFile(skillFile); err == nil { @@ -190,7 +128,7 @@ func (sl *SkillsLoader) LoadSkill(name string) (string, bool) { } } - // 2. 其次从全局 skills 加载 (~/.picoclaw/skills) + // 2. then load from global skills (~/.picoclaw/skills) if sl.globalSkills != "" { skillFile := filepath.Join(sl.globalSkills, name, "SKILL.md") if content, err := os.ReadFile(skillFile); err == nil { @@ -198,7 +136,7 @@ func (sl *SkillsLoader) LoadSkill(name string) (string, bool) { } } - // 3. 最后从内置 skills 加载 + // 3. finally load from builtin skills if sl.builtinSkills != "" { skillFile := filepath.Join(sl.builtinSkills, name, "SKILL.md") if content, err := os.ReadFile(skillFile); err == nil { diff --git a/pkg/skills/loader_test.go b/pkg/skills/loader_test.go index aca901d33..9428bea62 100644 --- a/pkg/skills/loader_test.go +++ b/pkg/skills/loader_test.go @@ -1,9 +1,12 @@ package skills import ( + "os" + "path/filepath" "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestSkillsInfoValidate(t *testing.T) { @@ -135,6 +138,134 @@ func TestExtractFrontmatter(t *testing.T) { } } +// createSkillDir creates a skill directory with a SKILL.md file containing the given frontmatter. +func createSkillDir(t *testing.T, base, dirName, name, description string) { + t.Helper() + dir := filepath.Join(base, dirName) + require.NoError(t, os.MkdirAll(dir, 0o755)) + content := "---\nname: " + name + "\ndescription: " + description + "\n---\n\n# " + name + require.NoError(t, os.WriteFile(filepath.Join(dir, "SKILL.md"), []byte(content), 0o644)) +} + +func TestListSkillsWorkspaceOverridesGlobal(t *testing.T) { + tmp := t.TempDir() + ws := filepath.Join(tmp, "workspace") + global := filepath.Join(tmp, "global") + + 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, "") + skills := sl.ListSkills() + + assert.Len(t, skills, 1) + assert.Equal(t, "workspace", skills[0].Source) + assert.Equal(t, "workspace version", skills[0].Description) +} + +func TestListSkillsGlobalOverridesBuiltin(t *testing.T) { + tmp := t.TempDir() + ws := filepath.Join(tmp, "workspace") + global := filepath.Join(tmp, "global") + builtin := filepath.Join(tmp, "builtin") + + createSkillDir(t, global, "my-skill", "my-skill", "global version") + createSkillDir(t, builtin, "my-skill", "my-skill", "builtin version") + + sl := NewSkillsLoader(ws, global, builtin) + skills := sl.ListSkills() + + assert.Len(t, skills, 1) + assert.Equal(t, "global", skills[0].Source) + assert.Equal(t, "global version", skills[0].Description) +} + +func TestListSkillsMetadataNameDedup(t *testing.T) { + tmp := t.TempDir() + ws := filepath.Join(tmp, "workspace") + global := filepath.Join(tmp, "global") + + // Different directory names but same metadata name + 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, "") + skills := sl.ListSkills() + + assert.Len(t, skills, 1) + assert.Equal(t, "shared-name", skills[0].Name) + assert.Equal(t, "workspace", skills[0].Source) +} + +func TestListSkillsMultipleDistinctSkills(t *testing.T) { + tmp := t.TempDir() + ws := filepath.Join(tmp, "workspace") + global := filepath.Join(tmp, "global") + builtin := filepath.Join(tmp, "builtin") + + createSkillDir(t, filepath.Join(ws, "skills"), "skill-a", "skill-a", "desc a") + createSkillDir(t, global, "skill-b", "skill-b", "desc b") + createSkillDir(t, builtin, "skill-c", "skill-c", "desc c") + + sl := NewSkillsLoader(ws, global, builtin) + skills := sl.ListSkills() + + assert.Len(t, skills, 3) + names := map[string]string{} + for _, s := range skills { + names[s.Name] = s.Source + } + assert.Equal(t, "workspace", names["skill-a"]) + assert.Equal(t, "global", names["skill-b"]) + assert.Equal(t, "builtin", names["skill-c"]) +} + +func TestListSkillsInvalidSkillSkipped(t *testing.T) { + tmp := t.TempDir() + ws := filepath.Join(tmp, "workspace") + global := filepath.Join(tmp, "global") + + // Invalid name (underscore) + createSkillDir(t, filepath.Join(ws, "skills"), "bad_skill", "bad_skill", "desc") + // Valid skill + createSkillDir(t, global, "good-skill", "good-skill", "desc") + + sl := NewSkillsLoader(ws, global, "") + skills := sl.ListSkills() + + assert.Len(t, skills, 1) + assert.Equal(t, "good-skill", skills[0].Name) +} + +func TestListSkillsEmptyAndNonexistentDirs(t *testing.T) { + tmp := t.TempDir() + ws := filepath.Join(tmp, "workspace") + emptyDir := filepath.Join(tmp, "empty") + require.NoError(t, os.MkdirAll(emptyDir, 0o755)) + + sl := NewSkillsLoader(ws, emptyDir, filepath.Join(tmp, "nonexistent")) + skills := sl.ListSkills() + + assert.Empty(t, skills) +} + +func TestListSkillsDirWithoutSkillMD(t *testing.T) { + tmp := t.TempDir() + ws := filepath.Join(tmp, "workspace") + global := filepath.Join(tmp, "global") + + // Directory exists but has no SKILL.md + require.NoError(t, os.MkdirAll(filepath.Join(global, "no-skillmd"), 0o755)) + // Valid skill alongside + createSkillDir(t, global, "real-skill", "real-skill", "desc") + + sl := NewSkillsLoader(ws, global, "") + skills := sl.ListSkills() + + assert.Len(t, skills, 1) + assert.Equal(t, "real-skill", skills[0].Name) +} + func TestStripFrontmatter(t *testing.T) { sl := &SkillsLoader{} diff --git a/pkg/tools/edit.go b/pkg/tools/edit.go index c28ca6ca2..d3ab267bf 100644 --- a/pkg/tools/edit.go +++ b/pkg/tools/edit.go @@ -2,24 +2,27 @@ package tools import ( "context" + "errors" "fmt" - "os" + "io/fs" "strings" ) // EditFileTool edits a file by replacing old_text with new_text. // The old_text must exist exactly in the file. type EditFileTool struct { - allowedDir string - restrict bool + fs fileSystem } // NewEditFileTool creates a new EditFileTool with optional directory restriction. -func NewEditFileTool(allowedDir string, restrict bool) *EditFileTool { - return &EditFileTool{ - allowedDir: allowedDir, - restrict: restrict, +func NewEditFileTool(workspace string, restrict bool) *EditFileTool { + var fs fileSystem + if restrict { + fs = &sandboxFs{workspace: workspace} + } else { + fs = &hostFs{} } + return &EditFileTool{fs: fs} } func (t *EditFileTool) Name() string { @@ -67,49 +70,24 @@ func (t *EditFileTool) Execute(ctx context.Context, args map[string]any) *ToolRe return ErrorResult("new_text is required") } - resolvedPath, err := validatePath(path, t.allowedDir, t.restrict) - if err != nil { + if err := editFile(t.fs, path, oldText, newText); err != nil { return ErrorResult(err.Error()) } - - if _, err = os.Stat(resolvedPath); os.IsNotExist(err) { - return ErrorResult(fmt.Sprintf("file not found: %s", path)) - } - - content, err := os.ReadFile(resolvedPath) - if err != nil { - return ErrorResult(fmt.Sprintf("failed to read file: %v", err)) - } - - contentStr := string(content) - - if !strings.Contains(contentStr, oldText) { - return ErrorResult("old_text not found in file. Make sure it matches exactly") - } - - count := strings.Count(contentStr, oldText) - if count > 1 { - return ErrorResult( - fmt.Sprintf("old_text appears %d times. Please provide more context to make it unique", count), - ) - } - - newContent := strings.Replace(contentStr, oldText, newText, 1) - - if err := os.WriteFile(resolvedPath, []byte(newContent), 0o644); err != nil { - return ErrorResult(fmt.Sprintf("failed to write file: %v", err)) - } - return SilentResult(fmt.Sprintf("File edited: %s", path)) } type AppendFileTool struct { - workspace string - restrict bool + fs fileSystem } func NewAppendFileTool(workspace string, restrict bool) *AppendFileTool { - return &AppendFileTool{workspace: workspace, restrict: restrict} + var fs fileSystem + if restrict { + fs = &sandboxFs{workspace: workspace} + } else { + fs = &hostFs{} + } + return &AppendFileTool{fs: fs} } func (t *AppendFileTool) Name() string { @@ -148,20 +126,52 @@ func (t *AppendFileTool) Execute(ctx context.Context, args map[string]any) *Tool return ErrorResult("content is required") } - resolvedPath, err := validatePath(path, t.workspace, t.restrict) - if err != nil { + if err := appendFile(t.fs, path, content); err != nil { return ErrorResult(err.Error()) } - - f, err := os.OpenFile(resolvedPath, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0o644) - if err != nil { - return ErrorResult(fmt.Sprintf("failed to open file: %v", err)) - } - defer f.Close() - - if _, err := f.WriteString(content); err != nil { - return ErrorResult(fmt.Sprintf("failed to append to file: %v", err)) - } - return SilentResult(fmt.Sprintf("Appended to %s", path)) } + +// editFile reads the file via sysFs, performs the replacement, and writes back. +// It uses a fileSystem interface, allowing the same logic for both restricted and unrestricted modes. +func editFile(sysFs fileSystem, path, oldText, newText string) error { + content, err := sysFs.ReadFile(path) + if err != nil { + return err + } + + newContent, err := replaceEditContent(content, oldText, newText) + if err != nil { + return err + } + + return sysFs.WriteFile(path, newContent) +} + +// appendFile reads the existing content (if any) via sysFs, appends new content, and writes back. +func appendFile(sysFs fileSystem, path, appendContent string) error { + content, err := sysFs.ReadFile(path) + if err != nil && !errors.Is(err, fs.ErrNotExist) { + return err + } + + newContent := append(content, []byte(appendContent)...) + return sysFs.WriteFile(path, newContent) +} + +// replaceEditContent handles the core logic of finding and replacing a single occurrence of oldText. +func replaceEditContent(content []byte, oldText, newText string) ([]byte, error) { + contentStr := string(content) + + if !strings.Contains(contentStr, oldText) { + return nil, fmt.Errorf("old_text not found in file. Make sure it matches exactly") + } + + count := strings.Count(contentStr, oldText) + if count > 1 { + return nil, fmt.Errorf("old_text appears %d times. Please provide more context to make it unique", count) + } + + newContent := strings.Replace(contentStr, oldText, newText, 1) + return []byte(newContent), nil +} diff --git a/pkg/tools/edit_test.go b/pkg/tools/edit_test.go index 6780dd9f6..83a7e778c 100644 --- a/pkg/tools/edit_test.go +++ b/pkg/tools/edit_test.go @@ -6,6 +6,8 @@ import ( "path/filepath" "strings" "testing" + + "github.com/stretchr/testify/assert" ) // TestEditTool_EditFile_Success verifies successful file editing @@ -151,14 +153,18 @@ func TestEditTool_EditFile_OutsideAllowedDir(t *testing.T) { result := tool.Execute(ctx, args) // Should return error result - if !result.IsError { - t.Errorf("Expected error when path is outside allowed directory") - } + assert.True(t, result.IsError, "Expected error when path is outside allowed directory") // Should mention outside allowed directory - if !strings.Contains(result.ForLLM, "outside") && !strings.Contains(result.ForUser, "outside") { - t.Errorf("Expected 'outside allowed' message, got ForLLM: %s", result.ForLLM) - } + // Note: ErrorResult only sets ForLLM by default, so ForUser might be empty. + // We check ForLLM as it's the primary error channel. + assert.True( + t, + strings.Contains(result.ForLLM, "outside") || strings.Contains(result.ForLLM, "access denied") || + strings.Contains(result.ForLLM, "escapes"), + "Expected 'outside allowed' or 'access denied' message, got ForLLM: %s", + result.ForLLM, + ) } // TestEditTool_EditFile_MissingPath verifies error handling for missing path @@ -287,3 +293,145 @@ func TestEditTool_AppendFile_MissingContent(t *testing.T) { t.Errorf("Expected error when content is missing") } } + +// TestReplaceEditContent verifies the helper function replaceEditContent +func TestReplaceEditContent(t *testing.T) { + tests := []struct { + name string + content []byte + oldText string + newText string + expected []byte + expectError bool + }{ + { + name: "successful replacement", + content: []byte("hello world"), + oldText: "world", + newText: "universe", + expected: []byte("hello universe"), + expectError: false, + }, + { + name: "old text not found", + content: []byte("hello world"), + oldText: "golang", + newText: "rust", + expected: nil, + expectError: true, + }, + { + name: "multiple matches found", + content: []byte("test text test"), + oldText: "test", + newText: "done", + expected: nil, + expectError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result, err := replaceEditContent(tt.content, tt.oldText, tt.newText) + if tt.expectError { + assert.Error(t, err) + } else { + assert.NoError(t, err) + assert.Equal(t, tt.expected, result) + } + }) + } +} + +// TestAppendFileTool_AppendToNonExistent_Restricted verifies that AppendFileTool in restricted mode +// can append to a file that does not yet exist — it should silently create the file. +// This exercises the errors.Is(err, fs.ErrNotExist) path in appendFileWithRW + rootRW. +func TestAppendFileTool_AppendToNonExistent_Restricted(t *testing.T) { + workspace := t.TempDir() + tool := NewAppendFileTool(workspace, true) + ctx := context.Background() + + args := map[string]any{ + "path": "brand_new_file.txt", + "content": "first content", + } + + result := tool.Execute(ctx, args) + assert.False( + t, + result.IsError, + "Expected success when appending to non-existent file in restricted mode, got: %s", + result.ForLLM, + ) + + // Verify the file was created with correct content + data, err := os.ReadFile(filepath.Join(workspace, "brand_new_file.txt")) + assert.NoError(t, err) + assert.Equal(t, "first content", string(data)) +} + +// TestAppendFileTool_Restricted_Success verifies that AppendFileTool in restricted mode +// correctly appends to an existing file within the sandbox. +func TestAppendFileTool_Restricted_Success(t *testing.T) { + workspace := t.TempDir() + testFile := "existing.txt" + err := os.WriteFile(filepath.Join(workspace, testFile), []byte("initial"), 0o644) + assert.NoError(t, err) + + tool := NewAppendFileTool(workspace, true) + ctx := context.Background() + args := map[string]any{ + "path": testFile, + "content": " appended", + } + + result := tool.Execute(ctx, args) + assert.False(t, result.IsError, "Expected success, got: %s", result.ForLLM) + assert.True(t, result.Silent) + + data, err := os.ReadFile(filepath.Join(workspace, testFile)) + assert.NoError(t, err) + assert.Equal(t, "initial appended", string(data)) +} + +// TestEditFileTool_Restricted_InPlaceEdit verifies that EditFileTool in restricted mode +// correctly edits a file using the single-open editFileInRoot path. +func TestEditFileTool_Restricted_InPlaceEdit(t *testing.T) { + workspace := t.TempDir() + testFile := "edit_target.txt" + err := os.WriteFile(filepath.Join(workspace, testFile), []byte("Hello World"), 0o644) + assert.NoError(t, err) + + tool := NewEditFileTool(workspace, true) + ctx := context.Background() + args := map[string]any{ + "path": testFile, + "old_text": "World", + "new_text": "Go", + } + + result := tool.Execute(ctx, args) + assert.False(t, result.IsError, "Expected success, got: %s", result.ForLLM) + assert.True(t, result.Silent) + + data, err := os.ReadFile(filepath.Join(workspace, testFile)) + assert.NoError(t, err) + assert.Equal(t, "Hello Go", string(data)) +} + +// TestEditFileTool_Restricted_FileNotFound verifies that editFileInRoot returns a proper +// error message when the target file does not exist. +func TestEditFileTool_Restricted_FileNotFound(t *testing.T) { + workspace := t.TempDir() + tool := NewEditFileTool(workspace, true) + ctx := context.Background() + args := map[string]any{ + "path": "no_such_file.txt", + "old_text": "old", + "new_text": "new", + } + + result := tool.Execute(ctx, args) + assert.True(t, result.IsError) + assert.Contains(t, result.ForLLM, "not found") +} diff --git a/pkg/tools/filesystem.go b/pkg/tools/filesystem.go index 1bf50906e..37db8b4ae 100644 --- a/pkg/tools/filesystem.go +++ b/pkg/tools/filesystem.go @@ -3,15 +3,17 @@ package tools import ( "context" "fmt" + "io/fs" "os" "path/filepath" "strings" + "time" ) // validatePath ensures the given path is within the workspace if restrict is true. func validatePath(path, workspace string, restrict bool) (string, error) { if workspace == "" { - return path, nil + return path, fmt.Errorf("workspace is not defined") } absWorkspace, err := filepath.Abs(workspace) @@ -76,16 +78,21 @@ func resolveExistingAncestor(path string) (string, error) { func isWithinWorkspace(candidate, workspace string) bool { rel, err := filepath.Rel(filepath.Clean(workspace), filepath.Clean(candidate)) - return err == nil && rel != ".." && !strings.HasPrefix(rel, ".."+string(os.PathSeparator)) + return err == nil && filepath.IsLocal(rel) } type ReadFileTool struct { - workspace string - restrict bool + fs fileSystem } func NewReadFileTool(workspace string, restrict bool) *ReadFileTool { - return &ReadFileTool{workspace: workspace, restrict: restrict} + var fs fileSystem + if restrict { + fs = &sandboxFs{workspace: workspace} + } else { + fs = &hostFs{} + } + return &ReadFileTool{fs: fs} } func (t *ReadFileTool) Name() string { @@ -115,26 +122,25 @@ func (t *ReadFileTool) Execute(ctx context.Context, args map[string]any) *ToolRe return ErrorResult("path is required") } - resolvedPath, err := validatePath(path, t.workspace, t.restrict) + content, err := t.fs.ReadFile(path) if err != nil { return ErrorResult(err.Error()) } - - content, err := os.ReadFile(resolvedPath) - if err != nil { - return ErrorResult(fmt.Sprintf("failed to read file: %v", err)) - } - return NewToolResult(string(content)) } type WriteFileTool struct { - workspace string - restrict bool + fs fileSystem } func NewWriteFileTool(workspace string, restrict bool) *WriteFileTool { - return &WriteFileTool{workspace: workspace, restrict: restrict} + var fs fileSystem + if restrict { + fs = &sandboxFs{workspace: workspace} + } else { + fs = &hostFs{} + } + return &WriteFileTool{fs: fs} } func (t *WriteFileTool) Name() string { @@ -173,30 +179,25 @@ func (t *WriteFileTool) Execute(ctx context.Context, args map[string]any) *ToolR return ErrorResult("content is required") } - resolvedPath, err := validatePath(path, t.workspace, t.restrict) - if err != nil { + if err := t.fs.WriteFile(path, []byte(content)); err != nil { return ErrorResult(err.Error()) } - dir := filepath.Dir(resolvedPath) - if err := os.MkdirAll(dir, 0o755); err != nil { - return ErrorResult(fmt.Sprintf("failed to create directory: %v", err)) - } - - if err := os.WriteFile(resolvedPath, []byte(content), 0o644); err != nil { - return ErrorResult(fmt.Sprintf("failed to write file: %v", err)) - } - return SilentResult(fmt.Sprintf("File written: %s", path)) } type ListDirTool struct { - workspace string - restrict bool + fs fileSystem } func NewListDirTool(workspace string, restrict bool) *ListDirTool { - return &ListDirTool{workspace: workspace, restrict: restrict} + var fs fileSystem + if restrict { + fs = &sandboxFs{workspace: workspace} + } else { + fs = &hostFs{} + } + return &ListDirTool{fs: fs} } func (t *ListDirTool) Name() string { @@ -226,24 +227,179 @@ func (t *ListDirTool) Execute(ctx context.Context, args map[string]any) *ToolRes path = "." } - resolvedPath, err := validatePath(path, t.workspace, t.restrict) - if err != nil { - return ErrorResult(err.Error()) - } - - entries, err := os.ReadDir(resolvedPath) + entries, err := t.fs.ReadDir(path) if err != nil { return ErrorResult(fmt.Sprintf("failed to read directory: %v", err)) } + return formatDirEntries(entries) +} - result := "" +func formatDirEntries(entries []os.DirEntry) *ToolResult { + var result strings.Builder for _, entry := range entries { if entry.IsDir() { - result += "DIR: " + entry.Name() + "\n" + result.WriteString("DIR: " + entry.Name() + "\n") } else { - result += "FILE: " + entry.Name() + "\n" + result.WriteString("FILE: " + entry.Name() + "\n") + } + } + return NewToolResult(result.String()) +} + +// fileSystem abstracts reading, writing, and listing files, allowing both +// unrestricted (host filesystem) and sandbox (os.Root) implementations to share the same polymorphic interface. +type fileSystem interface { + ReadFile(path string) ([]byte, error) + WriteFile(path string, data []byte) error + ReadDir(path string) ([]os.DirEntry, error) +} + +// hostFs is an unrestricted fileReadWriter that operates directly on the host filesystem. +type hostFs struct{} + +func (h *hostFs) ReadFile(path string) ([]byte, error) { + content, err := os.ReadFile(path) + if err != nil { + if os.IsNotExist(err) { + return nil, fmt.Errorf("failed to read file: file not found: %w", err) + } + if os.IsPermission(err) { + return nil, fmt.Errorf("failed to read file: access denied: %w", err) + } + return nil, fmt.Errorf("failed to read file: %w", err) + } + return content, nil +} + +func (h *hostFs) ReadDir(path string) ([]os.DirEntry, error) { + return os.ReadDir(path) +} + +func (h *hostFs) WriteFile(path string, data []byte) error { + dir := filepath.Dir(path) + if err := os.MkdirAll(dir, 0o755); err != nil { + return fmt.Errorf("failed to create parent directories: %w", err) + } + + // We use a "write-then-rename" pattern here to ensure an atomic write. + // This prevents the target file from being left in a truncated or partial state + // if the operation is interrupted, as the rename operation is atomic on Linux. + tmpPath := fmt.Sprintf("%s.%d.tmp", path, time.Now().UnixNano()) + if err := os.WriteFile(tmpPath, data, 0o644); err != nil { + os.Remove(tmpPath) // Ensure cleanup of partial/empty temp file + return fmt.Errorf("failed to write temp file: %w", err) + } + + if err := os.Rename(tmpPath, path); err != nil { + os.Remove(tmpPath) + return fmt.Errorf("failed to replace original file: %w", err) + } + return nil +} + +// sandboxFs is a sandboxed fileSystem that operates within a strictly defined workspace using os.Root. +type sandboxFs struct { + workspace string +} + +func (r *sandboxFs) execute(path string, fn func(root *os.Root, relPath string) error) error { + if r.workspace == "" { + return fmt.Errorf("workspace is not defined") + } + + root, err := os.OpenRoot(r.workspace) + if err != nil { + return fmt.Errorf("failed to open workspace: %w", err) + } + defer root.Close() + + relPath, err := getSafeRelPath(r.workspace, path) + if err != nil { + return err + } + + return fn(root, relPath) +} + +func (r *sandboxFs) ReadFile(path string) ([]byte, error) { + var content []byte + err := r.execute(path, func(root *os.Root, relPath string) error { + fileContent, err := root.ReadFile(relPath) + if err != nil { + if os.IsNotExist(err) { + return fmt.Errorf("failed to read file: file not found: %w", err) + } + // os.Root returns "escapes from parent" for paths outside the root + if os.IsPermission(err) || strings.Contains(err.Error(), "escapes from parent") || + strings.Contains(err.Error(), "permission denied") { + return fmt.Errorf("failed to read file: access denied: %w", err) + } + return fmt.Errorf("failed to read file: %w", err) + } + content = fileContent + return nil + }) + return content, err +} + +func (r *sandboxFs) WriteFile(path string, data []byte) error { + return r.execute(path, func(root *os.Root, relPath string) error { + dir := filepath.Dir(relPath) + if dir != "." && dir != "/" { + if err := root.MkdirAll(dir, 0o755); err != nil { + return fmt.Errorf("failed to create parent directories: %w", err) + } + } + + // We use a "write-then-rename" pattern here to ensure an atomic write. + // This prevents the target file from being left in a truncated or partial state + // if the operation is interrupted, as the rename operation is atomic on Linux. + tmpRelPath := fmt.Sprintf("%s.%d.tmp", relPath, time.Now().UnixNano()) + + if err := root.WriteFile(tmpRelPath, data, 0o644); err != nil { + root.Remove(tmpRelPath) // Ensure cleanup of partial/empty temp file + return fmt.Errorf("failed to write to temp file: %w", err) + } + + if err := root.Rename(tmpRelPath, relPath); err != nil { + root.Remove(tmpRelPath) + return fmt.Errorf("failed to rename temp file over target: %w", err) + } + return nil + }) +} + +func (r *sandboxFs) ReadDir(path string) ([]os.DirEntry, error) { + var entries []os.DirEntry + err := r.execute(path, func(root *os.Root, relPath string) error { + dirEntries, err := fs.ReadDir(root.FS(), relPath) + if err != nil { + return err + } + entries = dirEntries + return nil + }) + return entries, err +} + +// Helper to get a safe relative path for os.Root usage +func getSafeRelPath(workspace, path string) (string, error) { + if workspace == "" { + return "", fmt.Errorf("workspace is not defined") + } + + rel := filepath.Clean(path) + if filepath.IsAbs(rel) { + var err error + rel, err = filepath.Rel(workspace, rel) + if err != nil { + return "", fmt.Errorf("failed to calculate relative path: %w", err) } } - return NewToolResult(result) + if !filepath.IsLocal(rel) { + return "", fmt.Errorf("path escapes workspace: %s", path) + } + + return rel, nil } diff --git a/pkg/tools/filesystem_test.go b/pkg/tools/filesystem_test.go index 5daa3dcea..6f896e22d 100644 --- a/pkg/tools/filesystem_test.go +++ b/pkg/tools/filesystem_test.go @@ -2,10 +2,13 @@ package tools import ( "context" + "io" "os" "path/filepath" "strings" "testing" + + "github.com/stretchr/testify/assert" ) // TestFilesystemTool_ReadFile_Success verifies successful file reading @@ -14,7 +17,7 @@ func TestFilesystemTool_ReadFile_Success(t *testing.T) { testFile := filepath.Join(tmpDir, "test.txt") os.WriteFile(testFile, []byte("test content"), 0o644) - tool := &ReadFileTool{} + tool := NewReadFileTool("", false) ctx := context.Background() args := map[string]any{ "path": testFile, @@ -41,7 +44,7 @@ func TestFilesystemTool_ReadFile_Success(t *testing.T) { // TestFilesystemTool_ReadFile_NotFound verifies error handling for missing file func TestFilesystemTool_ReadFile_NotFound(t *testing.T) { - tool := &ReadFileTool{} + tool := NewReadFileTool("", false) ctx := context.Background() args := map[string]any{ "path": "/nonexistent_file_12345.txt", @@ -84,7 +87,7 @@ func TestFilesystemTool_WriteFile_Success(t *testing.T) { tmpDir := t.TempDir() testFile := filepath.Join(tmpDir, "newfile.txt") - tool := &WriteFileTool{} + tool := NewWriteFileTool("", false) ctx := context.Background() args := map[string]any{ "path": testFile, @@ -123,7 +126,7 @@ func TestFilesystemTool_WriteFile_CreateDir(t *testing.T) { tmpDir := t.TempDir() testFile := filepath.Join(tmpDir, "subdir", "newfile.txt") - tool := &WriteFileTool{} + tool := NewWriteFileTool("", false) ctx := context.Background() args := map[string]any{ "path": testFile, @@ -149,7 +152,7 @@ func TestFilesystemTool_WriteFile_CreateDir(t *testing.T) { // TestFilesystemTool_WriteFile_MissingPath verifies error handling for missing path func TestFilesystemTool_WriteFile_MissingPath(t *testing.T) { - tool := &WriteFileTool{} + tool := NewWriteFileTool("", false) ctx := context.Background() args := map[string]any{ "content": "test", @@ -165,7 +168,7 @@ func TestFilesystemTool_WriteFile_MissingPath(t *testing.T) { // TestFilesystemTool_WriteFile_MissingContent verifies error handling for missing content func TestFilesystemTool_WriteFile_MissingContent(t *testing.T) { - tool := &WriteFileTool{} + tool := NewWriteFileTool("", false) ctx := context.Background() args := map[string]any{ "path": "/tmp/test.txt", @@ -192,7 +195,7 @@ func TestFilesystemTool_ListDir_Success(t *testing.T) { os.WriteFile(filepath.Join(tmpDir, "file2.txt"), []byte("content"), 0o644) os.Mkdir(filepath.Join(tmpDir, "subdir"), 0o755) - tool := &ListDirTool{} + tool := NewListDirTool("", false) ctx := context.Background() args := map[string]any{ "path": tmpDir, @@ -216,7 +219,7 @@ func TestFilesystemTool_ListDir_Success(t *testing.T) { // TestFilesystemTool_ListDir_NotFound verifies error handling for non-existent directory func TestFilesystemTool_ListDir_NotFound(t *testing.T) { - tool := &ListDirTool{} + tool := NewListDirTool("", false) ctx := context.Background() args := map[string]any{ "path": "/nonexistent_directory_12345", @@ -237,7 +240,7 @@ func TestFilesystemTool_ListDir_NotFound(t *testing.T) { // TestFilesystemTool_ListDir_DefaultPath verifies default to current directory func TestFilesystemTool_ListDir_DefaultPath(t *testing.T) { - tool := &ListDirTool{} + tool := NewListDirTool("", false) ctx := context.Background() args := map[string]any{} @@ -275,7 +278,211 @@ func TestFilesystemTool_ReadFile_RejectsSymlinkEscape(t *testing.T) { if !result.IsError { t.Fatalf("expected symlink escape to be blocked") } - if !strings.Contains(result.ForLLM, "symlink resolves outside workspace") { + // os.Root might return different errors depending on platform/implementation + // but it definitely should error. + // Our wrapper returns "access denied or file not found" + if !strings.Contains(result.ForLLM, "access denied") && !strings.Contains(result.ForLLM, "file not found") && + !strings.Contains(result.ForLLM, "no such file") { t.Fatalf("expected symlink escape error, got: %s", result.ForLLM) } } + +func TestFilesystemTool_EmptyWorkspace_AccessDenied(t *testing.T) { + tool := NewReadFileTool("", true) // restrict=true but workspace="" + + // Try to read a sensitive file (simulated by a temp file outside workspace) + tmpDir := t.TempDir() + secretFile := filepath.Join(tmpDir, "shadow") + os.WriteFile(secretFile, []byte("secret data"), 0o600) + + result := tool.Execute(context.Background(), map[string]any{ + "path": secretFile, + }) + + // We EXPECT IsError=true (access blocked due to empty workspace) + assert.True(t, result.IsError, "Security Regression: Empty workspace allowed access! content: %s", result.ForLLM) + + // Verify it failed for the right reason + assert.Contains(t, result.ForLLM, "workspace is not defined", "Expected 'workspace is not defined' error") +} + +// TestRootMkdirAll verifies that root.MkdirAll (used by atomicWriteFileInRoot) handles all cases: +// single dir, deeply nested dirs, already-existing dirs, and a file blocking a directory path. +func TestRootMkdirAll(t *testing.T) { + workspace := t.TempDir() + root, err := os.OpenRoot(workspace) + if err != nil { + t.Fatalf("failed to open root: %v", err) + } + defer root.Close() + + // Case 1: Single directory + err = root.MkdirAll("dir1", 0o755) + assert.NoError(t, err) + _, err = os.Stat(filepath.Join(workspace, "dir1")) + assert.NoError(t, err) + + // Case 2: Deeply nested directory + err = root.MkdirAll("a/b/c/d", 0o755) + assert.NoError(t, err) + _, err = os.Stat(filepath.Join(workspace, "a/b/c/d")) + assert.NoError(t, err) + + // Case 3: Already exists — must be idempotent + err = root.MkdirAll("a/b/c/d", 0o755) + assert.NoError(t, err) + + // Case 4: A regular file blocks directory creation — must error + err = os.WriteFile(filepath.Join(workspace, "file_exists"), []byte("data"), 0o644) + assert.NoError(t, err) + err = root.MkdirAll("file_exists", 0o755) + assert.Error(t, err, "expected error when a file exists at the directory path") +} + +func TestFilesystemTool_WriteFile_Restricted_CreateDir(t *testing.T) { + workspace := t.TempDir() + tool := NewWriteFileTool(workspace, true) + ctx := context.Background() + + testFile := "deep/nested/path/to/file.txt" + content := "deep content" + args := map[string]any{ + "path": testFile, + "content": content, + } + + result := tool.Execute(ctx, args) + assert.False(t, result.IsError, "Expected success, got: %s", result.ForLLM) + + // Verify file content + actualPath := filepath.Join(workspace, testFile) + data, err := os.ReadFile(actualPath) + assert.NoError(t, err) + assert.Equal(t, content, string(data)) +} + +// TestHostRW_Read_PermissionDenied verifies that hostRW.Read surfaces access denied errors. +func TestHostRW_Read_PermissionDenied(t *testing.T) { + if os.Getuid() == 0 { + t.Skip("skipping permission test: running as root") + } + tmpDir := t.TempDir() + protected := filepath.Join(tmpDir, "protected.txt") + err := os.WriteFile(protected, []byte("secret"), 0o000) + assert.NoError(t, err) + defer os.Chmod(protected, 0o644) // ensure cleanup + + _, err = (&hostFs{}).ReadFile(protected) + assert.Error(t, err) + assert.Contains(t, err.Error(), "access denied") +} + +// TestHostRW_Read_Directory verifies that hostRW.Read returns an error when given a directory path. +func TestHostRW_Read_Directory(t *testing.T) { + tmpDir := t.TempDir() + + _, err := (&hostFs{}).ReadFile(tmpDir) + assert.Error(t, err, "expected error when reading a directory as a file") +} + +// TestRootRW_Read_Directory verifies that rootRW.Read returns an error when given a directory. +func TestRootRW_Read_Directory(t *testing.T) { + workspace := t.TempDir() + root, err := os.OpenRoot(workspace) + assert.NoError(t, err) + defer root.Close() + + // Create a subdirectory + err = root.Mkdir("subdir", 0o755) + assert.NoError(t, err) + + _, err = (&sandboxFs{workspace: workspace}).ReadFile("subdir") + assert.Error(t, err, "expected error when reading a directory as a file") +} + +// TestHostRW_Write_ParentDirMissing verifies that hostRW.Write creates parent dirs automatically. +func TestHostRW_Write_ParentDirMissing(t *testing.T) { + tmpDir := t.TempDir() + target := filepath.Join(tmpDir, "a", "b", "c", "file.txt") + + err := (&hostFs{}).WriteFile(target, []byte("hello")) + assert.NoError(t, err) + + data, err := os.ReadFile(target) + assert.NoError(t, err) + assert.Equal(t, "hello", string(data)) +} + +// TestRootRW_Write_ParentDirMissing verifies that rootRW.Write creates +// nested parent directories automatically within the sandbox. +func TestRootRW_Write_ParentDirMissing(t *testing.T) { + workspace := t.TempDir() + + relPath := "x/y/z/file.txt" + err := (&sandboxFs{workspace: workspace}).WriteFile(relPath, []byte("nested")) + assert.NoError(t, err) + + data, err := os.ReadFile(filepath.Join(workspace, relPath)) + assert.NoError(t, err) + assert.Equal(t, "nested", string(data)) +} + +// TestHostRW_Write verifies the hostRW.Write helper function +func TestHostRW_Write(t *testing.T) { + tmpDir := t.TempDir() + testFile := filepath.Join(tmpDir, "atomic_test.txt") + testData := []byte("atomic test content") + + err := (&hostFs{}).WriteFile(testFile, testData) + assert.NoError(t, err) + + content, err := os.ReadFile(testFile) + assert.NoError(t, err) + assert.Equal(t, testData, content) + + // Verify it overwrites correctly + newData := []byte("new atomic content") + err = (&hostFs{}).WriteFile(testFile, newData) + assert.NoError(t, err) + + content, err = os.ReadFile(testFile) + assert.NoError(t, err) + assert.Equal(t, newData, content) +} + +// TestRootRW_Write verifies the rootRW.Write helper function +func TestRootRW_Write(t *testing.T) { + tmpDir := t.TempDir() + + relPath := "atomic_root_test.txt" + testData := []byte("atomic root test content") + + erw := &sandboxFs{workspace: tmpDir} + err := erw.WriteFile(relPath, testData) + assert.NoError(t, err) + + root, err := os.OpenRoot(tmpDir) + assert.NoError(t, err) + defer root.Close() + + f, err := root.Open(relPath) + assert.NoError(t, err) + defer f.Close() + + content, err := io.ReadAll(f) + assert.NoError(t, err) + assert.Equal(t, testData, content) + + // Verify it overwrites correctly + newData := []byte("new root atomic content") + err = erw.WriteFile(relPath, newData) + assert.NoError(t, err) + + f2, err := root.Open(relPath) + assert.NoError(t, err) + defer f2.Close() + + content, err = io.ReadAll(f2) + assert.NoError(t, err) + assert.Equal(t, newData, content) +} diff --git a/pkg/tools/i2c.go b/pkg/tools/i2c.go index 0387a26d3..779b1d5a7 100644 --- a/pkg/tools/i2c.go +++ b/pkg/tools/i2c.go @@ -117,13 +117,19 @@ func (t *I2CTool) detect() *ToolResult { return SilentResult(fmt.Sprintf("Found %d I2C bus(es):\n%s", len(buses), string(result))) } +// Helper functions for I2C operations (used by platform-specific implementations) + // isValidBusID checks that a bus identifier is a simple number (prevents path injection) +// +//nolint:unused // Used by i2c_linux.go func isValidBusID(id string) bool { matched, _ := regexp.MatchString(`^\d+$`, id) return matched } // parseI2CAddress extracts and validates an I2C address from args +// +//nolint:unused // Used by i2c_linux.go func parseI2CAddress(args map[string]any) (int, *ToolResult) { addrFloat, ok := args["address"].(float64) if !ok { @@ -137,6 +143,8 @@ func parseI2CAddress(args map[string]any) (int, *ToolResult) { } // parseI2CBus extracts and validates an I2C bus from args +// +//nolint:unused // Used by i2c_linux.go func parseI2CBus(args map[string]any) (string, *ToolResult) { bus, ok := args["bus"].(string) if !ok || bus == "" { diff --git a/pkg/tools/shell.go b/pkg/tools/shell.go index a1ee0b6e1..6883172cd 100644 --- a/pkg/tools/shell.go +++ b/pkg/tools/shell.go @@ -81,6 +81,7 @@ func NewExecToolWithConfig(workingDir string, restrict bool, config *config.Conf execConfig := config.Tools.Exec enableDenyPatterns = execConfig.EnableDenyPatterns if enableDenyPatterns { + denyPatterns = append(denyPatterns, defaultDenyPatterns...) if len(execConfig.CustomDenyPatterns) > 0 { fmt.Printf("Using custom deny patterns: %v\n", execConfig.CustomDenyPatterns) for _, pattern := range execConfig.CustomDenyPatterns { @@ -91,8 +92,6 @@ func NewExecToolWithConfig(workingDir string, restrict bool, config *config.Conf } denyPatterns = append(denyPatterns, re) } - } else { - denyPatterns = append(denyPatterns, defaultDenyPatterns...) } } else { // If deny patterns are disabled, we won't add any patterns, allowing all commands. diff --git a/pkg/tools/spi.go b/pkg/tools/spi.go index d6a88a5b0..0ca17e84f 100644 --- a/pkg/tools/spi.go +++ b/pkg/tools/spi.go @@ -119,7 +119,11 @@ func (t *SPITool) list() *ToolResult { return SilentResult(fmt.Sprintf("Found %d SPI device(s):\n%s", len(devices), string(result))) } +// Helper function for SPI operations (used by platform-specific implementations) + // parseSPIArgs extracts and validates common SPI parameters +// +//nolint:unused // Used by spi_linux.go func parseSPIArgs(args map[string]any) (device string, speed uint32, mode uint8, bits uint8, errMsg string) { dev, ok := args["device"].(string) if !ok || dev == "" {