From 330de0c3825187b94bf10b598d5565801a516e65 Mon Sep 17 00:00:00 2001 From: wenjie Date: Wed, 8 Apr 2026 10:57:22 +0800 Subject: [PATCH 1/4] fix(agent): disable seahorse context manager on freebsd/arm (#2417) * fix(agent): disable seahorse context manager on freebsd/arm Exclude freebsd/arm from the seahorse-enabled build and route it to the unsupported stub implementation. This avoids freebsd/arm build failures caused by modernc sqlite/libc while keeping picoclaw buildable on that target. * build: bump Go version from 1.25.8 to 1.25.9 * ci: install and run govulncheck directly in PR workflow --- .github/workflows/pr.yml | 7 ++++--- go.mod | 2 +- pkg/agent/context_seahorse.go | 2 +- pkg/agent/context_seahorse_unsupported.go | 2 +- 4 files changed, 7 insertions(+), 6 deletions(-) diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 2d544d4f0..795fa5eba 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -41,10 +41,11 @@ jobs: with: go-version-file: go.mod + - name: Install govulncheck + run: go install golang.org/x/vuln/cmd/govulncheck@v1.1.4 + - name: Run Govulncheck - uses: golang/govulncheck-action@v1 - with: - go-package: ./... + run: govulncheck -C . -format text ./... test: name: Tests diff --git a/go.mod b/go.mod index a9f4bb7cb..1ff7cb306 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/sipeed/picoclaw -go 1.25.8 +go 1.25.9 require ( fyne.io/systray v1.12.0 diff --git a/pkg/agent/context_seahorse.go b/pkg/agent/context_seahorse.go index a2e09095a..327c6162a 100644 --- a/pkg/agent/context_seahorse.go +++ b/pkg/agent/context_seahorse.go @@ -1,4 +1,4 @@ -//go:build !mipsle && !netbsd +//go:build !mipsle && !netbsd && !(freebsd && arm) package agent diff --git a/pkg/agent/context_seahorse_unsupported.go b/pkg/agent/context_seahorse_unsupported.go index 882a973b9..7528f79bc 100644 --- a/pkg/agent/context_seahorse_unsupported.go +++ b/pkg/agent/context_seahorse_unsupported.go @@ -1,4 +1,4 @@ -//go:build mipsle || netbsd +//go:build mipsle || netbsd || (freebsd && arm) package agent From ee29aaa871be7336a6418cd3c5e09a80bc0af512 Mon Sep 17 00:00:00 2001 From: Harmoon Date: Wed, 8 Apr 2026 11:47:02 +0800 Subject: [PATCH 2/4] Enhance hooks with respond action and comprehensive documentation (#2215) * feat(hooks): add respond action for tool execution bypass Add a new HookActionRespond that allows hooks to return tool results directly, skipping actual tool execution. This enables plugin tool injection, caching, and mocking capabilities. - Add HookActionRespond constant and support in HookManager - Extend ToolCallHookRequest with HookResult field - Implement respond action handling in process hooks and agent loop - Add comprehensive tests for respond and deny_tool actions - Update documentation with hook actions table and examples * docs(hooks): add JSON-RPC protocol and plugin tool injection documentation Add comprehensive documentation for hook JSON-RPC protocol and plugin tool injection capabilities: - Add "Hook Actions" section to README.zh.md explaining respond action for tool execution bypass - Create hook-json-protocol.md/.zh.md detailing JSON-RPC 2.0 protocol for all hook methods - Create plugin-tool-injection.md/.zh.md with complete examples for external tool implementation - Document how hooks can inject tool definitions and return results via respond action - Include Python and Go examples for weather query plugin implementation * feat(agent): emit tool events and feedback for hook results Add ToolExecStart event emission and tool feedback for hook results to ensure consistent behavior between normal tool execution and hook bypass scenarios. This maintains parity in event tracking and user feedback when tools are executed via hooks. * style(agent): format whitespace in hook structs and constants Remove trailing whitespace and standardize spacing in JSON struct tags, constants, and test data for improved code consistency. * feat(hooks): add media support for plugin tool injection Extend the hook respond action to support media file handling: - Add `media` field for returning images and files from hooks - Add `response_handled` field to control turn completion behavior - When response_handled=true, media is automatically delivered to user - When response_handled=false, media is passed to LLM for vision requests This enables plugins to directly return generated images, downloaded files, and other media content either to users or for LLM analysis. * docs(hooks): document security implications of respond action Add security boundary documentation explaining that the respond action bypasses ApproveTool checks, allowing hooks to return results for any tool without approval. Include recommendations for secure hook implementation and code comments marking the security considerations. Changes: - Add "Security Boundaries" section to plugin-tool-injection docs - Document bypass of approval checks and associated risks - Provide security recommendations and example code - Add inline security comments in hooks.go and loop.go * refactor(agent): improve completeness of tool result cloning and hook processing Extend cloneToolResult to properly copy ArtifactTags and Messages fields, ensuring deep copies of all ToolResult data. Consolidate event emission and user message handling to match the normal tool execution flow. * fix(agent): align hook respond path with normal tool execution flow The hook respond code path was missing several critical behaviors that existed in normal tool execution: - Add logging for tool calls with arguments preview - Add is_tool_call metadata to user-facing messages - Handle attachment delivery failures by setting error state and notifying LLM - Set ResponseHandled=false when using bus for media delivery - Check for steering messages and graceful interrupts after tool execution, skipping remaining tools when appropriate - Poll for SubTurn results that arrived during tool execution This ensures consistent behavior between hook-responded tool calls and normally executed tool calls. * test(agent): add tests for hook respond media error handling Add comprehensive tests for the hook respond code path when media delivery fails. Tests cover error media channel scenarios and verify proper error state handling. Also document that AfterTool is not called when using respond action, as it provides the final answer directly (design decision). --- docs/hooks/README.md | 63 +++ docs/hooks/README.zh.md | 63 +++ docs/hooks/hook-json-protocol.md | 568 ++++++++++++++++++++++++ docs/hooks/hook-json-protocol.zh.md | 568 ++++++++++++++++++++++++ docs/hooks/plugin-tool-injection.md | 587 +++++++++++++++++++++++++ docs/hooks/plugin-tool-injection.zh.md | 587 +++++++++++++++++++++++++ pkg/agent/hook_process.go | 8 +- pkg/agent/hooks.go | 24 +- pkg/agent/hooks_test.go | 517 ++++++++++++++++++++++ pkg/agent/loop.go | 230 ++++++++++ 10 files changed, 3209 insertions(+), 6 deletions(-) create mode 100644 docs/hooks/hook-json-protocol.md create mode 100644 docs/hooks/hook-json-protocol.zh.md create mode 100644 docs/hooks/plugin-tool-injection.md create mode 100644 docs/hooks/plugin-tool-injection.zh.md diff --git a/docs/hooks/README.md b/docs/hooks/README.md index ec3bbc46a..5be0f30b5 100644 --- a/docs/hooks/README.md +++ b/docs/hooks/README.md @@ -28,6 +28,69 @@ The currently exposed synchronous hook points are: Everything else is exposed as read-only events. +## Hook Actions + +Hooks can return different actions to control the flow: + +| Action | Applicable Stages | Effect | +| --- | --- | --- | +| `continue` | All interceptors | Pass through without modification | +| `modify` | `before_llm`, `after_llm`, `before_tool`, `after_tool` | Modify request/response and continue | +| `respond` | `before_tool` | Return a tool result directly, skip actual tool execution | +| `deny_tool` | `before_tool` | Deny tool execution, return error message | +| `abort_turn` | All interceptors | Abort the current turn | +| `hard_abort` | All interceptors | Force stop the entire agent loop | + +### The `respond` Action + +The `respond` action is special: it allows a `before_tool` hook to provide the tool result directly, skipping the actual tool execution. This is useful for: + +1. **Plugin tool injection**: External hooks can implement tools without registering them in the tool registry +2. **Tool result caching**: Return cached results for repeated tool calls +3. **Tool mocking**: Return mock results for testing purposes + +When a hook returns `respond` with a `HookResult`, the agent loop: +1. Skips the actual tool execution +2. Uses the provided result as if the tool had executed +3. Continues the turn normally with the result + +Example (Go in-process hook): + +```go +func (h *MyHook) BeforeTool( + ctx context.Context, + call *agent.ToolCallHookRequest, +) (*agent.ToolCallHookRequest, agent.HookDecision, error) { + if call.Tool == "my_plugin_tool" { + next := call.Clone() + next.HookResult = &tools.ToolResult{ + ForLLM: "Plugin tool executed successfully", + Silent: false, + IsError: false, + } + return next, agent.HookDecision{Action: agent.HookActionRespond}, nil + } + return call, agent.HookDecision{Action: agent.HookActionContinue}, nil +} +``` + +Example (Python process hook): + +```python +def handle_before_tool(params: dict) -> dict: + tool = params.get("tool", "") + if tool == "my_plugin_tool": + return { + "action": "respond", + "result": { + "for_llm": "Plugin tool executed successfully", + "silent": False, + "is_error": False + } + } + return {"action": "continue"} +``` + ## Execution Order `HookManager` sorts hooks like this: diff --git a/docs/hooks/README.zh.md b/docs/hooks/README.zh.md index 46c7c9392..2170d45c8 100644 --- a/docs/hooks/README.zh.md +++ b/docs/hooks/README.zh.md @@ -28,6 +28,69 @@ 其余 lifecycle 通过事件形式只读暴露。 +## Hook Actions + +Hook 可以返回不同的 action 来控制流程： + +| Action | 适用阶段 | 效果 | +| --- | --- | --- | +| `continue` | 所有拦截型 | 放行，不做修改 | +| `modify` | `before_llm`, `after_llm`, `before_tool`, `after_tool` | 改写请求/响应后放行 | +| `respond` | `before_tool` | 直接返回工具结果，跳过实际工具执行 | +| `deny_tool` | `before_tool` | 拒绝工具执行，返回错误信息 | +| `abort_turn` | 所有拦截型 | 中止当前 turn | +| `hard_abort` | 所有拦截型 | 强制终止整个 agent loop | + +### `respond` Action + +`respond` action 是特殊的：它允许 `before_tool` hook 直接提供工具结果，跳过实际工具执行。适用于： + +1. **插件工具注入**：外部 hook 可以实现工具，无需在 ToolRegistry 注册 +2. **工具结果缓存**：对重复调用返回缓存结果 +3. **工具模拟**：测试时返回模拟结果 + +当 hook 返回 `respond` 并携带 `HookResult` 时，agent loop 会： +1. 跳过实际工具执行 +2. 使用提供的结果作为工具执行结果 +3. 正常继续 turn 流程 + +示例（Go 进程内 hook）： + +```go +func (h *MyHook) BeforeTool( + ctx context.Context, + call *agent.ToolCallHookRequest, +) (*agent.ToolCallHookRequest, agent.HookDecision, error) { + if call.Tool == "my_plugin_tool" { + next := call.Clone() + next.HookResult = &tools.ToolResult{ + ForLLM: "Plugin tool executed successfully", + Silent: false, + IsError: false, + } + return next, agent.HookDecision{Action: agent.HookActionRespond}, nil + } + return call, agent.HookDecision{Action: agent.HookActionContinue}, nil +} +``` + +示例（Python process hook）： + +```python +def handle_before_tool(params: dict) -> dict: + tool = params.get("tool", "") + if tool == "my_plugin_tool": + return { + "action": "respond", + "result": { + "for_llm": "Plugin tool executed successfully", + "silent": False, + "is_error": False + } + } + return {"action": "continue"} +``` + ## 执行顺序 HookManager 的排序规则是： diff --git a/docs/hooks/hook-json-protocol.md b/docs/hooks/hook-json-protocol.md new file mode 100644 index 000000000..58b6e323b --- /dev/null +++ b/docs/hooks/hook-json-protocol.md @@ -0,0 +1,568 @@ +# Hook JSON-RPC Protocol Details + +All hooks use `JSON-RPC 2.0` format, with one JSON message per line, transmitted via stdio. + +--- + +## Basic Protocol Structure + +### Request (PicoClaw → Hook) + +```json +{"jsonrpc":"2.0","id":1,"method":"hook.xxx","params":{...}} +``` + +### Response (Hook → PicoClaw) + +Success: +```json +{"jsonrpc":"2.0","id":1,"result":{...}} +``` + +Error: +```json +{"jsonrpc":"2.0","id":1,"error":{"code":-32000,"message":"error message"}} +``` + +--- + +## 1. `hook.hello` (Handshake) + +Handshake must be completed at startup, otherwise the hook process will be terminated. + +### Request + +```json +{ + "jsonrpc": "2.0", + "id": 1, + "method": "hook.hello", + "params": { + "name": "py_review_gate", + "version": 1, + "modes": ["observe", "tool", "approve"] + } +} +``` + +| Field | Description | +|-------|-------------| +| `name` | hook name (from configuration) | +| `version` | protocol version, currently `1` | +| `modes` | capability modes supported by the hook | + +### Response + +```json +{ + "jsonrpc": "2.0", + "id": 1, + "result": { + "ok": true, + "name": "python-review-gate" + } +} +``` + +--- + +## 2. `hook.before_llm` + +Triggered before sending request to LLM. Can be used to inject tools. + +### Request + +```json +{ + "jsonrpc": "2.0", + "id": 2, + "method": "hook.before_llm", + "params": { + "meta": { + "AgentID": "agent-1", + "TurnID": "turn-1", + "ParentTurnID": "", + "SessionKey": "session-1", + "Iteration": 0, + "TracePath": "runTurn", + "Source": "turn.llm.request" + }, + "model": "claude-sonnet", + "messages": [ + {"role": "user", "content": "hello"} + ], + "tools": [ + { + "type": "function", + "function": { + "name": "echo", + "description": "echo text", + "parameters": {"type": "object"} + } + } + ], + "options": { + "temperature": 0.7 + }, + "channel": "cli", + "chat_id": "chat-1", + "graceful_terminal": false + } +} +``` + +| Field | Description | +|-------|-------------| +| `meta` | event metadata for tracing | +| `model` | requested model name | +| `messages` | conversation history | +| `tools` | list of available tool definitions | +| `options` | LLM parameters (temperature, max_tokens, etc.) | +| `channel` | request source channel | +| `chat_id` | session ID | + +### Response (Tool Injection Example) + +```json +{ + "jsonrpc": "2.0", + "id": 2, + "result": { + "action": "modify", + "request": { + "model": "claude-sonnet", + "messages": [{"role": "user", "content": "hello"}], + "tools": [ + { + "type": "function", + "function": { + "name": "echo", + "description": "echo", + "parameters": {} + } + }, + { + "type": "function", + "function": { + "name": "my_plugin_tool", + "description": "Plugin injected tool", + "parameters": { + "type": "object", + "properties": { + "query": {"type": "string"} + } + } + } + } + ] + } + } +} +``` + +| Field | Description | +|-------|-------------| +| `action` | decision action (see table below) | +| `request` | modified request object | + +--- + +## 3. `hook.after_llm` + +Triggered after receiving LLM response. Can modify response content. + +### Request + +```json +{ + "jsonrpc": "2.0", + "id": 3, + "method": "hook.after_llm", + "params": { + "meta": { + "AgentID": "agent-1", + "TurnID": "turn-1", + "SessionKey": "session-1" + }, + "model": "claude-sonnet", + "response": { + "role": "assistant", + "content": "Hi!", + "tool_calls": [ + { + "id": "tc-1", + "type": "function", + "function": { + "name": "echo", + "arguments": "{\"text\":\"hi\"}" + } + } + ] + }, + "channel": "cli", + "chat_id": "chat-1" + } +} +``` + +### Response + +```json +{ + "jsonrpc": "2.0", + "id": 3, + "result": { + "action": "continue" + } +} +``` + +--- + +## 4. `hook.before_tool` + +Triggered before tool execution. Can modify tool name and arguments, deny execution, or return result directly. + +### Request + +```json +{ + "jsonrpc": "2.0", + "id": 4, + "method": "hook.before_tool", + "params": { + "meta": { + "AgentID": "agent-1", + "TurnID": "turn-1", + "SessionKey": "session-1" + }, + "tool": "echo_text", + "arguments": { + "text": "hello" + }, + "channel": "cli", + "chat_id": "chat-1" + } +} +``` + +| Field | Description | +|-------|-------------| +| `tool` | tool name | +| `arguments` | tool arguments | + +### Response (Modify Arguments) + +```json +{ + "jsonrpc": "2.0", + "id": 4, + "result": { + "action": "modify", + "call": { + "tool": "echo_text", + "arguments": { + "text": "modified hello" + } + } + } +} +``` + +### Response (Deny Execution) + +```json +{ + "jsonrpc": "2.0", + "id": 4, + "result": { + "action": "deny_tool", + "reason": "Invalid arguments" + } +} +``` + +### Response (Return Result Directly - respond) + +```json +{ + "jsonrpc": "2.0", + "id": 4, + "result": { + "action": "respond", + "call": { + "tool": "my_plugin_tool", + "arguments": { + "query": "hello" + } + }, + "result": { + "for_llm": "Plugin tool executed successfully", + "for_user": "", + "silent": false, + "is_error": false + } + } +} +``` + +The `respond` action allows hooks to return tool results directly, skipping actual tool execution. Use cases: +1. **Plugin tool injection**: External hooks can implement tools without registering in ToolRegistry +2. **Tool result caching**: Return cached results for repeated calls +3. **Tool mocking**: Return mock results during testing + +| Field | Description | +|-------|-------------| +| `action` | must be `respond` | +| `call` | modified call information (optional) | +| `result` | tool result to return directly | + +--- + +## 5. `hook.after_tool` + +Triggered after tool execution completes. Can modify the result returned to LLM. + +### Request + +```json +{ + "jsonrpc": "2.0", + "id": 5, + "method": "hook.after_tool", + "params": { + "meta": { + "AgentID": "agent-1", + "TurnID": "turn-1", + "SessionKey": "session-1" + }, + "tool": "echo_text", + "arguments": { + "text": "hello" + }, + "result": { + "for_llm": "echoed: hello", + "for_user": "", + "silent": false, + "is_error": false, + "async": false, + "media": [], + "artifact_tags": [], + "response_handled": false + }, + "duration": 15000000, + "channel": "cli", + "chat_id": "chat-1" + } +} +``` + +| Field | Description | +|-------|-------------| +| `result.for_llm` | content returned to LLM | +| `result.for_user` | content sent to user | +| `result.silent` | whether silent (not sent to user) | +| `result.is_error` | whether it's an error | +| `result.async` | whether executed asynchronously | +| `result.media` | list of media references | +| `result.artifact_tags` | local artifact path tags | +| `result.response_handled` | whether response has been handled | +| `duration` | execution time (nanoseconds) | + +### Response + +```json +{ + "jsonrpc": "2.0", + "id": 5, + "result": { + "action": "continue" + } +} +``` + +--- + +## 6. `hook.approve_tool` + +Approval hook for deciding whether to allow execution of sensitive tools. + +### Request + +```json +{ + "jsonrpc": "2.0", + "id": 6, + "method": "hook.approve_tool", + "params": { + "meta": { + "AgentID": "agent-1", + "TurnID": "turn-1", + "SessionKey": "session-1" + }, + "tool": "bash", + "arguments": { + "command": "rm -rf /" + }, + "channel": "cli", + "chat_id": "chat-1" + } +} +``` + +### Response (Approved) + +```json +{ + "jsonrpc": "2.0", + "id": 6, + "result": { + "approved": true + } +} +``` + +### Response (Denied) + +```json +{ + "jsonrpc": "2.0", + "id": 6, + "result": { + "approved": false, + "reason": "Dangerous command, execution denied" + } +} +``` + +--- + +## 7. `hook.event` (notification) + +Observer event, broadcast only, no response required. `id` is `0` or absent. + +```json +{ + "jsonrpc": "2.0", + "method": "hook.event", + "params": { + "Kind": "tool_exec_start", + "Meta": { + "AgentID": "agent-1", + "TurnID": "turn-1" + }, + "Payload": { + "Tool": "echo_text", + "Arguments": {"text": "hello"} + } + } +} +``` + +Common `Kind` values: +- `turn_start` / `turn_end` +- `llm_request` / `llm_response` +- `tool_exec_start` / `tool_exec_end` / `tool_exec_skipped` +- `steering_injected` +- `interrupt_received` +- `error` + +--- + +## Action Options + +| action | Applicable hooks | Effect | +|--------|-----------------|--------| +| `continue` | All interceptor types | Pass through without modification | +| `modify` | `before_llm`, `before_tool`, `after_llm`, `after_tool` | Modify request/response and pass through | +| `respond` | `before_tool` | Return tool result directly, skip actual execution. **Note: AfterTool is NOT called (design decision - respond provides final answer).** | +| `deny_tool` | `before_tool` | Deny tool execution | +| `abort_turn` | All interceptor types | Abort current turn, return error | +| `hard_abort` | All interceptor types | Force stop entire agent loop | + +--- + +## Complete Flow Example + +```json +{"jsonrpc":"2.0","id":1,"method":"hook.hello","params":{"name":"my_hook","version":1,"modes":["tool","approve"]}} +{"jsonrpc":"2.0","id":1,"result":{"ok":true,"name":"my_hook"}} +{"jsonrpc":"2.0","id":2,"method":"hook.before_llm","params":{"model":"claude-sonnet","messages":[{"role":"user","content":"hello"}],"tools":[]}} +{"jsonrpc":"2.0","id":2,"result":{"action":"continue"}} +{"jsonrpc":"2.0","id":3,"method":"hook.before_tool","params":{"tool":"bash","arguments":{"command":"ls"}}} +{"jsonrpc":"2.0","id":3,"result":{"action":"continue"}} +{"jsonrpc":"2.0","id":4,"method":"hook.approve_tool","params":{"tool":"bash","arguments":{"command":"ls"}}} +{"jsonrpc":"2.0","id":4,"result":{"approved":true}} +{"jsonrpc":"2.0","id":5,"method":"hook.after_tool","params":{"tool":"bash","arguments":{"command":"ls"},"result":{"for_llm":"file1.txt\nfile2.txt"},"duration":5000000}} +{"jsonrpc":"2.0","id":5,"result":{"action":"continue"}} +{"jsonrpc":"2.0","id":6,"method":"hook.after_llm","params":{"model":"claude-sonnet","response":{"role":"assistant","content":"Files listed"}}} +{"jsonrpc":"2.0","id":6,"result":{"action":"continue"}} +``` + +--- + +## Plugin Tool Injection via `before_llm` and `before_tool` + +Standard flow for plugin tool injection: + +1. In `before_llm`, inject tool definition to let LLM know the tool is available +2. In `before_tool`, use `respond` action to return tool execution result directly + +### `before_llm` Inject Tool Definition + +```python +def handle_before_llm(params: dict) -> dict: + tools = params.get("tools", []) + + # Add plugin tool definition + tools.append({ + "type": "function", + "function": { + "name": "my_plugin_tool", + "description": "Plugin provided tool", + "parameters": { + "type": "object", + "properties": { + "input": {"type": "string", "description": "Input content"} + }, + "required": ["input"] + } + } + }) + + return { + "action": "modify", + "request": { + "model": params["model"], + "messages": params["messages"], + "tools": tools, + "options": params.get("options", {}) + } + } +``` + +### `before_tool` Return Execution Result + +```python +def handle_before_tool(params: dict) -> dict: + tool = params.get("tool", "") + + if tool == "my_plugin_tool": + # Implement tool logic here + args = params.get("arguments", {}) + input_text = args.get("input", "") + + # Return result directly, no need to register in ToolRegistry + return { + "action": "respond", + "result": { + "for_llm": f"Plugin tool executed successfully, input: {input_text}", + "silent": False, + "is_error": False + } + } + + return {"action": "continue"} +``` + +This way, external hooks can fully implement plugin tools without registering any tool implementation inside PicoClaw. \ No newline at end of file diff --git a/docs/hooks/hook-json-protocol.zh.md b/docs/hooks/hook-json-protocol.zh.md new file mode 100644 index 000000000..675e0a429 --- /dev/null +++ b/docs/hooks/hook-json-protocol.zh.md @@ -0,0 +1,568 @@ +# Hook JSON-RPC 协议详解 + +所有 hook 使用 `JSON-RPC 2.0` 格式，每行一个 JSON 消息，通过 stdio 传输。 + +--- + +## 基础协议结构 + +### 请求（PicoClaw → Hook） + +```json +{"jsonrpc":"2.0","id":1,"method":"hook.xxx","params":{...}} +``` + +### 响应（Hook → PicoClaw） + +成功： +```json +{"jsonrpc":"2.0","id":1,"result":{...}} +``` + +错误： +```json +{"jsonrpc":"2.0","id":1,"error":{"code":-32000,"message":"错误信息"}} +``` + +--- + +## 1. `hook.hello`（握手） + +启动时必须完成握手，否则 hook 进程会被终止。 + +### 请求 + +```json +{ + "jsonrpc": "2.0", + "id": 1, + "method": "hook.hello", + "params": { + "name": "py_review_gate", + "version": 1, + "modes": ["observe", "tool", "approve"] + } +} +``` + +| 字段 | 说明 | +|------|------| +| `name` | hook 名称（来自配置） | +| `version` | 协议版本，当前为 `1` | +| `modes` | hook 支持的能力模式 | + +### 响应 + +```json +{ + "jsonrpc": "2.0", + "id": 1, + "result": { + "ok": true, + "name": "python-review-gate" + } +} +``` + +--- + +## 2. `hook.before_llm` + +在发送请求给 LLM 之前触发。可用于注入工具。 + +### 请求 + +```json +{ + "jsonrpc": "2.0", + "id": 2, + "method": "hook.before_llm", + "params": { + "meta": { + "AgentID": "agent-1", + "TurnID": "turn-1", + "ParentTurnID": "", + "SessionKey": "session-1", + "Iteration": 0, + "TracePath": "runTurn", + "Source": "turn.llm.request" + }, + "model": "claude-sonnet", + "messages": [ + {"role": "user", "content": "hello"} + ], + "tools": [ + { + "type": "function", + "function": { + "name": "echo", + "description": "echo text", + "parameters": {"type": "object"} + } + } + ], + "options": { + "temperature": 0.7 + }, + "channel": "cli", + "chat_id": "chat-1", + "graceful_terminal": false + } +} +``` + +| 字段 | 说明 | +|------|------| +| `meta` | 事件元数据，用于追踪 | +| `model` | 请求的模型名称 | +| `messages` | 对话历史 | +| `tools` | 可用工具定义列表 | +| `options` | LLM 参数（temperature、max_tokens 等） | +| `channel` | 请求来源通道 | +| `chat_id` | 会话 ID | + +### 响应（注入工具示例） + +```json +{ + "jsonrpc": "2.0", + "id": 2, + "result": { + "action": "modify", + "request": { + "model": "claude-sonnet", + "messages": [{"role": "user", "content": "hello"}], + "tools": [ + { + "type": "function", + "function": { + "name": "echo", + "description": "echo", + "parameters": {} + } + }, + { + "type": "function", + "function": { + "name": "my_plugin_tool", + "description": "插件注入的工具", + "parameters": { + "type": "object", + "properties": { + "query": {"type": "string"} + } + } + } + } + ] + } + } +} +``` + +| 字段 | 说明 | +|------|------| +| `action` | 决策动作（见下表） | +| `request` | 修改后的请求对象 | + +--- + +## 3. `hook.after_llm` + +在收到 LLM 响应后触发。可修改响应内容。 + +### 请求 + +```json +{ + "jsonrpc": "2.0", + "id": 3, + "method": "hook.after_llm", + "params": { + "meta": { + "AgentID": "agent-1", + "TurnID": "turn-1", + "SessionKey": "session-1" + }, + "model": "claude-sonnet", + "response": { + "role": "assistant", + "content": "Hi!", + "tool_calls": [ + { + "id": "tc-1", + "type": "function", + "function": { + "name": "echo", + "arguments": "{\"text\":\"hi\"}" + } + } + ] + }, + "channel": "cli", + "chat_id": "chat-1" + } +} +``` + +### 响应 + +```json +{ + "jsonrpc": "2.0", + "id": 3, + "result": { + "action": "continue" + } +} +``` + +--- + +## 4. `hook.before_tool` + +在执行工具前触发。可修改工具名称和参数，或拒绝执行，或直接返回结果。 + +### 请求 + +```json +{ + "jsonrpc": "2.0", + "id": 4, + "method": "hook.before_tool", + "params": { + "meta": { + "AgentID": "agent-1", + "TurnID": "turn-1", + "SessionKey": "session-1" + }, + "tool": "echo_text", + "arguments": { + "text": "hello" + }, + "channel": "cli", + "chat_id": "chat-1" + } +} +``` + +| 字段 | 说明 | +|------|------| +| `tool` | 工具名称 | +| `arguments` | 工具参数 | + +### 响应（改写参数） + +```json +{ + "jsonrpc": "2.0", + "id": 4, + "result": { + "action": "modify", + "call": { + "tool": "echo_text", + "arguments": { + "text": "modified hello" + } + } + } +} +``` + +### 响应（拒绝执行） + +```json +{ + "jsonrpc": "2.0", + "id": 4, + "result": { + "action": "deny_tool", + "reason": "参数不合法" + } +} +``` + +### 响应（直接返回结果 - respond） + +```json +{ + "jsonrpc": "2.0", + "id": 4, + "result": { + "action": "respond", + "call": { + "tool": "my_plugin_tool", + "arguments": { + "query": "hello" + } + }, + "result": { + "for_llm": "Plugin tool executed successfully", + "for_user": "", + "silent": false, + "is_error": false + } + } +} +``` + +`respond` action 允许 hook 直接返回工具结果，跳过实际工具执行。适用于： +1. **插件工具注入**：外部 hook 可实现工具，无需在 ToolRegistry 注册 +2. **工具结果缓存**：对重复调用返回缓存结果 +3. **工具模拟**：测试时返回模拟结果 + +| 字段 | 说明 | +|------|------| +| `action` | 必须为 `respond` | +| `call` | 修改后的调用信息（可选） | +| `result` | 直接返回的工具结果 | + +--- + +## 5. `hook.after_tool` + +在工具执行完成后触发。可修改返回给 LLM 的结果。 + +### 请求 + +```json +{ + "jsonrpc": "2.0", + "id": 5, + "method": "hook.after_tool", + "params": { + "meta": { + "AgentID": "agent-1", + "TurnID": "turn-1", + "SessionKey": "session-1" + }, + "tool": "echo_text", + "arguments": { + "text": "hello" + }, + "result": { + "for_llm": "echoed: hello", + "for_user": "", + "silent": false, + "is_error": false, + "async": false, + "media": [], + "artifact_tags": [], + "response_handled": false + }, + "duration": 15000000, + "channel": "cli", + "chat_id": "chat-1" + } +} +``` + +| 字段 | 说明 | +|------|------| +| `result.for_llm` | 返回给 LLM 的内容 | +| `result.for_user` | 发送给用户的内容 | +| `result.silent` | 是否静默（不发送给用户） | +| `result.is_error` | 是否为错误 | +| `result.async` | 是否异步执行 | +| `result.media` | 媒体引用列表 | +| `result.artifact_tags` | 本地产物路径标签 | +| `result.response_handled` | 是否已处理响应 | +| `duration` | 执行耗时（纳秒） | + +### 响应 + +```json +{ + "jsonrpc": "2.0", + "id": 5, + "result": { + "action": "continue" + } +} +``` + +--- + +## 6. `hook.approve_tool` + +审批型 hook，用于决定是否允许执行敏感工具。 + +### 请求 + +```json +{ + "jsonrpc": "2.0", + "id": 6, + "method": "hook.approve_tool", + "params": { + "meta": { + "AgentID": "agent-1", + "TurnID": "turn-1", + "SessionKey": "session-1" + }, + "tool": "bash", + "arguments": { + "command": "rm -rf /" + }, + "channel": "cli", + "chat_id": "chat-1" + } +} +``` + +### 响应（批准） + +```json +{ + "jsonrpc": "2.0", + "id": 6, + "result": { + "approved": true + } +} +``` + +### 响应（拒绝） + +```json +{ + "jsonrpc": "2.0", + "id": 6, + "result": { + "approved": false, + "reason": "危险命令，禁止执行" + } +} +``` + +--- + +## 7. `hook.event`（notification） + +观察型事件，仅广播，无需响应。`id` 为 `0` 或不存在。 + +```json +{ + "jsonrpc": "2.0", + "method": "hook.event", + "params": { + "Kind": "tool_exec_start", + "Meta": { + "AgentID": "agent-1", + "TurnID": "turn-1" + }, + "Payload": { + "Tool": "echo_text", + "Arguments": {"text": "hello"} + } + } +} +``` + +常见 `Kind` 值： +- `turn_start` / `turn_end` +- `llm_request` / `llm_response` +- `tool_exec_start` / `tool_exec_end` / `tool_exec_skipped` +- `steering_injected` +- `interrupt_received` +- `error` + +--- + +## action 可选值 + +| action | 适用 hook | 效果 | +|--------|----------|------| +| `continue` | 所有拦截型 | 放行，不做修改 | +| `modify` | `before_llm`, `before_tool`, `after_llm`, `after_tool` | 改写请求/响应后放行 | +| `respond` | `before_tool` | 直接返回工具结果，跳过实际执行 | +| `deny_tool` | `before_tool` | 拒绝执行该工具 | +| `abort_turn` | 所有拦截型 | 中止当前 turn，返回错误 | +| `hard_abort` | 所有拦截型 | 强制终止整个 agent loop | + +--- + +## 完整流程示例 + +```json +{"jsonrpc":"2.0","id":1,"method":"hook.hello","params":{"name":"my_hook","version":1,"modes":["tool","approve"]}} +{"jsonrpc":"2.0","id":1,"result":{"ok":true,"name":"my_hook"}} +{"jsonrpc":"2.0","id":2,"method":"hook.before_llm","params":{"model":"claude-sonnet","messages":[{"role":"user","content":"hello"}],"tools":[]}} +{"jsonrpc":"2.0","id":2,"result":{"action":"continue"}} +{"jsonrpc":"2.0","id":3,"method":"hook.before_tool","params":{"tool":"bash","arguments":{"command":"ls"}}} +{"jsonrpc":"2.0","id":3,"result":{"action":"continue"}} +{"jsonrpc":"2.0","id":4,"method":"hook.approve_tool","params":{"tool":"bash","arguments":{"command":"ls"}}} +{"jsonrpc":"2.0","id":4,"result":{"approved":true}} +{"jsonrpc":"2.0","id":5,"method":"hook.after_tool","params":{"tool":"bash","arguments":{"command":"ls"},"result":{"for_llm":"file1.txt\nfile2.txt"},"duration":5000000}} +{"jsonrpc":"2.0","id":5,"result":{"action":"continue"}} +{"jsonrpc":"2.0","id":6,"method":"hook.after_llm","params":{"model":"claude-sonnet","response":{"role":"assistant","content":"已列出文件"}}} +{"jsonrpc":"2.0","id":6,"result":{"action":"continue"}} +``` + +--- + +## 通过 `before_llm` 和 `before_tool` 实现插件工具注入 + +插件工具注入的标准流程： + +1. 在 `before_llm` 中注入工具定义，让 LLM 知道有这个工具可用 +2. 在 `before_tool` 中使用 `respond` action 直接返回工具执行结果 + +### `before_llm` 注入工具定义 + +```python +def handle_before_llm(params: dict) -> dict: + tools = params.get("tools", []) + + # 添加插件工具定义 + tools.append({ + "type": "function", + "function": { + "name": "my_plugin_tool", + "description": "插件提供的工具", + "parameters": { + "type": "object", + "properties": { + "input": {"type": "string", "description": "输入内容"} + }, + "required": ["input"] + } + } + }) + + return { + "action": "modify", + "request": { + "model": params["model"], + "messages": params["messages"], + "tools": tools, + "options": params.get("options", {}) + } + } +``` + +### `before_tool` 返回执行结果 + +```python +def handle_before_tool(params: dict) -> dict: + tool = params.get("tool", "") + + if tool == "my_plugin_tool": + # 在这里实现工具逻辑 + args = params.get("arguments", {}) + input_text = args.get("input", "") + + # 直接返回结果，无需在 ToolRegistry 注册 + return { + "action": "respond", + "result": { + "for_llm": f"插件工具执行成功，输入: {input_text}", + "silent": False, + "is_error": False + } + } + + return {"action": "continue"} +``` + +通过这种方式，外部 hook 可以完全实现插件工具，无需在 PicoClaw 内部注册任何工具实现。 \ No newline at end of file diff --git a/docs/hooks/plugin-tool-injection.md b/docs/hooks/plugin-tool-injection.md new file mode 100644 index 000000000..9e699867b --- /dev/null +++ b/docs/hooks/plugin-tool-injection.md @@ -0,0 +1,587 @@ +# Plugin Tool Injection Example + +This document demonstrates how to use PicoClaw's hook system to implement external plugin tool injection, allowing LLM to call tools implemented by external hook processes. + +--- + +## Core Principle + +Through the hook system's `respond` action, external hooks can: + +1. Inject tool **definitions** in `before_llm`, letting LLM know the tool is available +2. Return tool **execution results** directly in `before_tool` using `respond` action, skipping ToolRegistry + +This way, external hooks can fully implement plugin tools without registering any tools inside PicoClaw. + +--- + +## Complete Example: Weather Query Plugin + +Below is a complete Python hook example implementing a weather query plugin tool. + +### 1. Hook Script Implementation + +Save as `/tmp/weather_plugin.py`: + +```python +#!/usr/bin/env python3 +"""Weather query plugin hook example""" +from __future__ import annotations + +import json +import sys +import signal +from typing import Any + +# Simulated weather data +WEATHER_DATA = { + "Beijing": {"temp": 15, "weather": "Sunny", "humidity": 45}, + "Shanghai": {"temp": 18, "weather": "Cloudy", "humidity": 60}, + "Guangzhou": {"temp": 25, "weather": "Sunny", "humidity": 70}, + "Shenzhen": {"temp": 26, "weather": "Cloudy", "humidity": 75}, +} + + +def get_weather(city: str) -> dict: + """Get weather data (simulated)""" + data = WEATHER_DATA.get(city) + if data: + return { + "for_llm": f"{city} weather: {data['weather']}, temperature {data['temp']}°C, humidity {data['humidity']}%", + "for_user": "", + "silent": False, + "is_error": False, + } + return { + "for_llm": f"Weather data not found for city {city}", + "for_user": "", + "silent": False, + "is_error": True, + } + + +def handle_hello(params: dict) -> dict: + return {"ok": True, "name": "weather-plugin"} + + +def handle_before_llm(params: dict) -> dict: + """Inject weather query tool definition""" + tools = params.get("tools", []) + + # Add weather query tool + tools.append({ + "type": "function", + "function": { + "name": "get_weather", + "description": "Query weather information for a specified city", + "parameters": { + "type": "object", + "properties": { + "city": { + "type": "string", + "description": "City name, e.g.: Beijing, Shanghai, Guangzhou" + } + }, + "required": ["city"] + } + } + }) + + return { + "action": "modify", + "request": { + "model": params.get("model"), + "messages": params.get("messages", []), + "tools": tools, + "options": params.get("options", {}), + } + } + + +def handle_before_tool(params: dict) -> dict: + """Handle tool call, return result directly""" + tool = params.get("tool", "") + args = params.get("arguments", {}) + + if tool == "get_weather": + city = args.get("city", "") + result = get_weather(city) + + # Use respond action to return result directly, skip ToolRegistry + return { + "action": "respond", + "result": result, + } + + # Other tools continue normal flow + return {"action": "continue"} + + +def handle_request(method: str, params: dict) -> dict: + if method == "hook.hello": + return handle_hello(params) + if method == "hook.before_llm": + return handle_before_llm(params) + if method == "hook.before_tool": + return handle_before_tool(params) + if method == "hook.after_llm": + return {"action": "continue"} + if method == "hook.after_tool": + return {"action": "continue"} + if method == "hook.approve_tool": + return {"approved": True} + raise KeyError(f"method not found: {method}") + + +def send_response(message_id: int, result: Any | None = None, error: str | None = None) -> None: + payload: dict[str, Any] = { + "jsonrpc": "2.0", + "id": message_id, + } + if error is not None: + payload["error"] = {"code": -32000, "message": error} + else: + payload["result"] = result if result is not None else {} + + sys.stdout.write(json.dumps(payload, ensure_ascii=True) + "\n") + sys.stdout.flush() + + +def main() -> int: + for raw_line in sys.stdin: + line = raw_line.strip() + if not line: + continue + + try: + message = json.loads(line) + except json.JSONDecodeError: + continue + + method = message.get("method") + message_id = message.get("id", 0) + params = message.get("params") or {} + + if not message_id: + continue + + try: + result = handle_request(str(method or ""), params) + send_response(int(message_id), result=result) + except KeyError as exc: + send_response(int(message_id), error=str(exc)) + except Exception as exc: + send_response(int(message_id), error=f"unexpected error: {exc}") + + return 0 + + +if __name__ == "__main__": + signal.signal(signal.SIGINT, lambda *_: raise SystemExit(0)) + signal.signal(signal.SIGTERM, lambda *_: raise SystemExit(0)) + raise SystemExit(main()) +``` + +### 2. Configure PicoClaw + +Add hook configuration in the config file: + +```json +{ + "hooks": { + "enabled": true, + "processes": { + "weather_plugin": { + "enabled": true, + "priority": 100, + "transport": "stdio", + "command": ["python3", "/tmp/weather_plugin.py"], + "intercept": ["before_llm", "before_tool"] + } + } + } +} +``` + +### 3. Test Results + +When user asks "What's the weather in Beijing today?": + +1. PicoClaw sends `hook.before_llm`, hook injects `get_weather` tool definition +2. LLM sees tool definition, decides to call `get_weather(city="Beijing")` +3. PicoClaw sends `hook.before_tool`, hook uses `respond` action to return weather data +4. LLM receives result, replies to user "Beijing is sunny today, temperature 15°C" + +--- + +## Flow Diagram + +``` +User: "What's the weather in Beijing today?" + ↓ + PicoClaw + ↓ + hook.before_llm + ↓ (inject get_weather tool definition) + LLM request + ↓ + LLM decides to call get_weather(city="Beijing") + ↓ + hook.before_tool + ↓ (respond action returns weather data) + Return result directly to LLM + ↓ (skip ToolRegistry) + LLM replies: "Beijing is sunny today, temperature 15°C" +``` + +--- + +## Key Points + +### `before_llm` Inject Tool Definition + +Tool definition follows OpenAI function calling format: + +```json +{ + "type": "function", + "function": { + "name": "tool_name", + "description": "tool description", + "parameters": { + "type": "object", + "properties": { + "param_name": { + "type": "string", + "description": "parameter description" + } + }, + "required": ["list of required parameters"] + } + } +} +``` + +### `before_tool` Use respond Action + +`respond` action response format: + +```json +{ + "action": "respond", + "result": { + "for_llm": "Content returned to LLM", + "for_user": "Optional, content sent to user", + "silent": false, + "is_error": false, + "media": ["Optional, media reference list"], + "response_handled": false + } +} +``` + +| Field | Description | +|-------|-------------| +| `for_llm` | Required, LLM will see this content | +| `for_user` | Optional, sent directly to user | +| `silent` | When true, not sent to user | +| `is_error` | When true, indicates execution failure | +| `media` | Optional, media file references (images, files, etc.) | +| `response_handled` | When true, indicates user request is handled, turn will end | + +--- + +## Media File Handling + +The `respond` action supports returning media files (images, files, etc.). There are two processing modes: + +### 1. Automatic Delivery (`response_handled=true`) + +When `response_handled=true`, media files are automatically sent to the user and the turn ends: + +```json +{ + "action": "respond", + "result": { + "for_llm": "Image sent to user", + "for_user": "", + "media": ["media://abc123"], + "response_handled": true + } +} +``` + +Use cases: +- Image generation plugin directly returning results +- File download plugin sending files to user + +### 2. LLM Visible (`response_handled=false`) + +When `response_handled=false`, media references are passed to the LLM, which can see the content in the next request: + +```json +{ + "action": "respond", + "result": { + "for_llm": "Image loaded, path: /tmp/image.png [file:/tmp/image.png]", + "media": ["media://abc123"] + } +} +``` + +After seeing the content, the LLM can decide: +- Use `send_file` tool to send to user +- Analyze image content and reply to user +- Other processing approaches + +### Media Reference Format + +Media references use the `media://` protocol: + +``` +media:// +``` + +These references are managed by PicoClaw's MediaStore and can be: +- Sent to user via channel +- Converted to base64 in LLM vision requests + +### Alternative: Use Existing Tools + +If the plugin generates files, you can return the file path and let the LLM call `send_file` or similar tools: + +```json +{ + "action": "respond", + "result": { + "for_llm": "Image generated, saved at /tmp/generated_image.png. Use send_file tool to send to user.", + "for_user": "", + "silent": false + } +} +``` + +This approach: +- More decoupled, LLM decides when to send +- Leverages existing tool mechanisms +- Supports batch sending, delayed sending, etc. + +--- + +## Multi-Tool Injection Example + +Multiple tools can be injected simultaneously: + +```python +def handle_before_llm(params: dict) -> dict: + tools = params.get("tools", []) + + # Tool 1: Weather query + tools.append({ + "type": "function", + "function": { + "name": "get_weather", + "description": "Query city weather", + "parameters": { + "type": "object", + "properties": { + "city": {"type": "string", "description": "City name"} + }, + "required": ["city"] + } + } + }) + + # Tool 2: Calculator + tools.append({ + "type": "function", + "function": { + "name": "calculate", + "description": "Perform mathematical calculations", + "parameters": { + "type": "object", + "properties": { + "expression": {"type": "string", "description": "Mathematical expression"} + }, + "required": ["expression"] + } + } + }) + + return { + "action": "modify", + "request": { + "model": params.get("model"), + "messages": params.get("messages", []), + "tools": tools, + "options": params.get("options", {}), + } + } + + +def handle_before_tool(params: dict) -> dict: + tool = params.get("tool", "") + args = params.get("arguments", {}) + + if tool == "get_weather": + return { + "action": "respond", + "result": get_weather(args.get("city", "")), + } + + if tool == "calculate": + # Simple calculation example + try: + expr = args.get("expression", "") + result = eval(expr) # Note: needs security handling in actual use + return { + "action": "respond", + "result": { + "for_llm": f"Calculation result: {result}", + "silent": False, + "is_error": False, + }, + } + except Exception as e: + return { + "action": "respond", + "result": { + "for_llm": f"Calculation error: {e}", + "silent": False, + "is_error": True, + }, + } + + return {"action": "continue"} +``` + +--- + +## Coexistence with Built-in Tools + +Injected plugin tools coexist with PicoClaw built-in tools: + +- Built-in tools (like `bash`, `read_file`) execute normally through ToolRegistry +- Plugin tools return results through hook's `respond` action +- `handle_before_tool` only handles plugin tools, other tools return `continue` + +--- + +## Go In-Process Hook Example + +If you need to implement plugin tool injection in Go code: + +```go +package myhooks + +import ( + "context" + "github.com/sipeed/picoclaw/pkg/agent" + "github.com/sipeed/picoclaw/pkg/tools" +) + +type WeatherPluginHook struct{} + +func (h *WeatherPluginHook) BeforeLLM( + ctx context.Context, + req *agent.LLMHookRequest, +) (*agent.LLMHookRequest, agent.HookDecision, error) { + // Inject tool definition + req.Tools = append(req.Tools, agent.ToolDefinition{ + Type: "function", + Function: agent.FunctionDefinition{ + Name: "get_weather", + Description: "Query city weather", + Parameters: map[string]any{ + "type": "object", + "properties": map[string]any{ + "city": map[string]any{ + "type": "string", + "description": "City name", + }, + }, + "required": []string{"city"}, + }, + }, + }) + + return req, agent.HookDecision{Action: agent.HookActionContinue}, nil +} + +func (h *WeatherPluginHook) BeforeTool( + ctx context.Context, + call *agent.ToolCallHookRequest, +) (*agent.ToolCallHookRequest, agent.HookDecision, error) { + if call.Tool == "get_weather" { + city := call.Arguments["city"].(string) + + // Set HookResult, use respond action + next := call.Clone() + next.HookResult = &tools.ToolResult{ + ForLLM: getWeatherData(city), + Silent: false, + IsError: false, + } + + return next, agent.HookDecision{Action: agent.HookActionRespond}, nil + } + + return call, agent.HookDecision{Action: agent.HookActionContinue}, nil +} + +func getWeatherData(city string) string { + // Implement weather query logic + return fmt.Sprintf("%s weather: Sunny, temperature 20°C", city) +} +``` + +--- + +## Summary + +Through the hook system's `respond` action, external processes can: + +1. **Inject tool definitions**: Let LLM know new tools are available +2. **Provide tool implementation**: Return execution results directly, no need to register in ToolRegistry +3. **Coexist with built-in tools**: Does not affect normal operation of PicoClaw's original tools + +This provides a flexible and elegant solution for plugin development. + +--- + +## Security Boundaries + +### Bypassing Approval Checks + +**Important**: The `respond` action bypasses `ApproveTool` approval checks. + +This means: +- A `before_tool` hook can return `respond` for **any tool name**, including sensitive tools (like `bash`) +- The tool won't go through the approval process, directly returning the hook-provided result +- This is designed for plugin tools but introduces security risks + +### Security Recommendations + +1. **Review hook configuration**: Ensure only trusted hook processes are enabled +2. **Limit hook scope**: Add your own security checks in hook implementation +3. **Use `deny_tool` for rejection**: Use `deny_tool` action instead of `respond` with error for denying execution + +### Example: Hook-Internal Security Check + +```python +def handle_before_tool(params: dict) -> dict: + tool = params.get("tool", "") + args = params.get("arguments", {}) + + # Security check: only handle plugin tools + if tool in ["get_weather", "calculate"]: + return { + "action": "respond", + "result": execute_plugin_tool(tool, args), + } + + # Other tools continue normal flow (will go through approval) + return {"action": "continue"} +``` + +This ensures the hook only affects plugin tools, not system tool approval flow. \ No newline at end of file diff --git a/docs/hooks/plugin-tool-injection.zh.md b/docs/hooks/plugin-tool-injection.zh.md new file mode 100644 index 000000000..ccc7ff7f6 --- /dev/null +++ b/docs/hooks/plugin-tool-injection.zh.md @@ -0,0 +1,587 @@ +# 插件工具注入示例 + +本文档展示如何利用 PicoClaw 的 hook 系统实现外部插件工具注入，让 LLM 能调用由外部 hook 进程实现的工具。 + +--- + +## 核心原理 + +通过 hook 系统的 `respond` action，外部 hook 可以： + +1. 在 `before_llm` 中注入工具**定义**，让 LLM 知道有这个工具可用 +2. 在 `before_tool` 中使用 `respond` action 直接返回工具**执行结果**，跳过 ToolRegistry + +这样，外部 hook 可以完全实现插件工具，无需在 PicoClaw 内部注册任何工具。 + +--- + +## 完整示例：天气查询插件 + +下面是一个完整的 Python hook 示例，实现一个天气查询插件工具。 + +### 1. Hook 脚本实现 + +保存为 `/tmp/weather_plugin.py`： + +```python +#!/usr/bin/env python3 +"""天气查询插件 hook 示例""" +from __future__ import annotations + +import json +import sys +import signal +from typing import Any + +# 模拟天气数据 +WEATHER_DATA = { + "北京": {"temp": 15, "weather": "晴", "humidity": 45}, + "上海": {"temp": 18, "weather": "多云", "humidity": 60}, + "广州": {"temp": 25, "weather": "晴", "humidity": 70}, + "深圳": {"temp": 26, "weather": "多云", "humidity": 75}, +} + + +def get_weather(city: str) -> dict: + """获取天气数据（模拟）""" + data = WEATHER_DATA.get(city) + if data: + return { + "for_llm": f"{city}天气：{data['weather']}，温度{data['temp']}°C，湿度{data['humidity']}%", + "for_user": "", + "silent": False, + "is_error": False, + } + return { + "for_llm": f"未找到城市 {city} 的天气数据", + "for_user": "", + "silent": False, + "is_error": True, + } + + +def handle_hello(params: dict) -> dict: + return {"ok": True, "name": "weather-plugin"} + + +def handle_before_llm(params: dict) -> dict: + """注入天气查询工具定义""" + tools = params.get("tools", []) + + # 添加天气查询工具 + tools.append({ + "type": "function", + "function": { + "name": "get_weather", + "description": "查询指定城市的天气信息", + "parameters": { + "type": "object", + "properties": { + "city": { + "type": "string", + "description": "城市名称，如：北京、上海、广州" + } + }, + "required": ["city"] + } + } + }) + + return { + "action": "modify", + "request": { + "model": params.get("model"), + "messages": params.get("messages", []), + "tools": tools, + "options": params.get("options", {}), + } + } + + +def handle_before_tool(params: dict) -> dict: + """处理工具调用，直接返回结果""" + tool = params.get("tool", "") + args = params.get("arguments", {}) + + if tool == "get_weather": + city = args.get("city", "") + result = get_weather(city) + + # 使用 respond action 直接返回结果，跳过 ToolRegistry + return { + "action": "respond", + "result": result, + } + + # 其他工具继续正常流程 + return {"action": "continue"} + + +def handle_request(method: str, params: dict) -> dict: + if method == "hook.hello": + return handle_hello(params) + if method == "hook.before_llm": + return handle_before_llm(params) + if method == "hook.before_tool": + return handle_before_tool(params) + if method == "hook.after_llm": + return {"action": "continue"} + if method == "hook.after_tool": + return {"action": "continue"} + if method == "hook.approve_tool": + return {"approved": True} + raise KeyError(f"method not found: {method}") + + +def send_response(message_id: int, result: Any | None = None, error: str | None = None) -> None: + payload: dict[str, Any] = { + "jsonrpc": "2.0", + "id": message_id, + } + if error is not None: + payload["error"] = {"code": -32000, "message": error} + else: + payload["result"] = result if result is not None else {} + + sys.stdout.write(json.dumps(payload, ensure_ascii=True) + "\n") + sys.stdout.flush() + + +def main() -> int: + for raw_line in sys.stdin: + line = raw_line.strip() + if not line: + continue + + try: + message = json.loads(line) + except json.JSONDecodeError: + continue + + method = message.get("method") + message_id = message.get("id", 0) + params = message.get("params") or {} + + if not message_id: + continue + + try: + result = handle_request(str(method or ""), params) + send_response(int(message_id), result=result) + except KeyError as exc: + send_response(int(message_id), error=str(exc)) + except Exception as exc: + send_response(int(message_id), error=f"unexpected error: {exc}") + + return 0 + + +if __name__ == "__main__": + signal.signal(signal.SIGINT, lambda *_: raise SystemExit(0)) + signal.signal(signal.SIGTERM, lambda *_: raise SystemExit(0)) + raise SystemExit(main()) +``` + +### 2. 配置 PicoClaw + +在配置文件中添加 hook 配置： + +```json +{ + "hooks": { + "enabled": true, + "processes": { + "weather_plugin": { + "enabled": true, + "priority": 100, + "transport": "stdio", + "command": ["python3", "/tmp/weather_plugin.py"], + "intercept": ["before_llm", "before_tool"] + } + } + } +} +``` + +### 3. 测试效果 + +当用户问"北京今天天气怎么样？"时： + +1. PicoClaw 发送 `hook.before_llm`，hook 注入 `get_weather` 工具定义 +2. LLM 看到工具定义，决定调用 `get_weather(city="北京")` +3. PicoClaw 发送 `hook.before_tool`，hook 使用 `respond` action 返回天气数据 +4. LLM 收到结果，回复用户"北京今天晴天，温度15°C" + +--- + +## 流程图解 + +``` +用户: "北京今天天气怎么样？" + ↓ + PicoClaw + ↓ + hook.before_llm + ↓ (注入 get_weather 工具定义) + LLM 请求 + ↓ + LLM 决定调用 get_weather(city="北京") + ↓ + hook.before_tool + ↓ (respond action 返回天气数据) + 直接返回结果给 LLM + ↓ (跳过 ToolRegistry) + LLM 回复: "北京今天晴天，温度15°C" +``` + +--- + +## 关键点说明 + +### `before_llm` 注入工具定义 + +工具定义遵循 OpenAI function calling 格式： + +```json +{ + "type": "function", + "function": { + "name": "工具名称", + "description": "工具描述", + "parameters": { + "type": "object", + "properties": { + "参数名": { + "type": "string", + "description": "参数描述" + } + }, + "required": ["必需参数列表"] + } + } +} +``` + +### `before_tool` 使用 respond action + +`respond` action 的响应格式： + +```json +{ + "action": "respond", + "result": { + "for_llm": "返回给 LLM 的内容", + "for_user": "可选，发送给用户的内容", + "silent": false, + "is_error": false, + "media": ["可选，媒体引用列表"], + "response_handled": false + } +} +``` + +| 字段 | 说明 | +|------|------| +| `for_llm` | 必须，LLM 会看到这个内容 | +| `for_user` | 可选，直接发送给用户 | +| `silent` | 为 true 时不发送给用户 | +| `is_error` | 为 true 时表示执行失败 | +| `media` | 可选，媒体文件引用列表（如图片、文件） | +| `response_handled` | 为 true 时表示已处理用户请求，轮次将结束 | + +--- + +## 媒体文件处理 + +`respond` action 支持返回媒体文件（图片、文件等）。有两种处理方式： + +### 1. 自动发送（`response_handled=true`） + +当 `response_handled=true` 时，媒体文件会自动发送给用户，轮次结束： + +```json +{ + "action": "respond", + "result": { + "for_llm": "图片已发送给用户", + "for_user": "", + "media": ["media://abc123"], + "response_handled": true + } +} +``` + +适用场景： +- 图像生成插件直接返回结果 +- 文件下载插件发送文件给用户 + +### 2. LLM 可见（`response_handled=false`） + +当 `response_handled=false` 时，媒体引用会传递给 LLM，LLM 可以在下一轮请求中看到内容： + +```json +{ + "action": "respond", + "result": { + "for_llm": "图片已加载，路径：/tmp/image.png [file:/tmp/image.png]", + "media": ["media://abc123"] + } +} +``` + +LLM 看到内容后，可以自主决定： +- 使用 `send_file` 工具发送给用户 +- 分析图片内容并回复用户 +- 其他处理方式 + +### 媒体引用格式 + +媒体引用使用 `media://` 协议： + +``` +media:// +``` + +这些引用由 PicoClaw 的 MediaStore 管理，可以： +- 通过 channel 发送给用户 +- 在 LLM vision 请求中转换为 base64 + +### 替代方案：使用现有工具 + +如果插件生成文件，可以返回文件路径让 LLM 调用 `send_file` 等工具： + +```json +{ + "action": "respond", + "result": { + "for_llm": "图片已生成，保存在 /tmp/generated_image.png。使用 send_file 工具发送给用户。", + "for_user": "", + "silent": false + } +} +``` + +这种方式： +- 更解耦，LLM 自主决策发送时机 +- 利用现有工具机制 +- 支持批量发送、延迟发送等场景 + +--- + +## 多工具注入示例 + +可以同时注入多个工具： + +```python +def handle_before_llm(params: dict) -> dict: + tools = params.get("tools", []) + + # 工具1：天气查询 + tools.append({ + "type": "function", + "function": { + "name": "get_weather", + "description": "查询城市天气", + "parameters": { + "type": "object", + "properties": { + "city": {"type": "string", "description": "城市名称"} + }, + "required": ["city"] + } + } + }) + + # 工具2：计算器 + tools.append({ + "type": "function", + "function": { + "name": "calculate", + "description": "执行数学计算", + "parameters": { + "type": "object", + "properties": { + "expression": {"type": "string", "description": "数学表达式"} + }, + "required": ["expression"] + } + } + }) + + return { + "action": "modify", + "request": { + "model": params.get("model"), + "messages": params.get("messages", []), + "tools": tools, + "options": params.get("options", {}), + } + } + + +def handle_before_tool(params: dict) -> dict: + tool = params.get("tool", "") + args = params.get("arguments", {}) + + if tool == "get_weather": + return { + "action": "respond", + "result": get_weather(args.get("city", "")), + } + + if tool == "calculate": + # 简单计算示例 + try: + expr = args.get("expression", "") + result = eval(expr) # 注意：实际使用时需要安全处理 + return { + "action": "respond", + "result": { + "for_llm": f"计算结果: {result}", + "silent": False, + "is_error": False, + }, + } + except Exception as e: + return { + "action": "respond", + "result": { + "for_llm": f"计算错误: {e}", + "silent": False, + "is_error": True, + }, + } + + return {"action": "continue"} +``` + +--- + +## 与内置工具共存 + +注入的插件工具与 PicoClaw 内置工具共存： + +- 内置工具（如 `bash`、`read_file`）正常通过 ToolRegistry 执行 +- 插件工具通过 hook 的 `respond` action 返回结果 +- `handle_before_tool` 中只处理插件工具，其他工具返回 `continue` + +--- + +## Go 进程内 Hook 示例 + +如果需要在 Go 代码中实现插件工具注入： + +```go +package myhooks + +import ( + "context" + "github.com/sipeed/picoclaw/pkg/agent" + "github.com/sipeed/picoclaw/pkg/tools" +) + +type WeatherPluginHook struct{} + +func (h *WeatherPluginHook) BeforeLLM( + ctx context.Context, + req *agent.LLMHookRequest, +) (*agent.LLMHookRequest, agent.HookDecision, error) { + // 注入工具定义 + req.Tools = append(req.Tools, agent.ToolDefinition{ + Type: "function", + Function: agent.FunctionDefinition{ + Name: "get_weather", + Description: "查询城市天气", + Parameters: map[string]any{ + "type": "object", + "properties": map[string]any{ + "city": map[string]any{ + "type": "string", + "description": "城市名称", + }, + }, + "required": []string{"city"}, + }, + }, + }) + + return req, agent.HookDecision{Action: agent.HookActionContinue}, nil +} + +func (h *WeatherPluginHook) BeforeTool( + ctx context.Context, + call *agent.ToolCallHookRequest, +) (*agent.ToolCallHookRequest, agent.HookDecision, error) { + if call.Tool == "get_weather" { + city := call.Arguments["city"].(string) + + // 设置 HookResult，使用 respond action + next := call.Clone() + next.HookResult = &tools.ToolResult{ + ForLLM: getWeatherData(city), + Silent: false, + IsError: false, + } + + return next, agent.HookDecision{Action: agent.HookActionRespond}, nil + } + + return call, agent.HookDecision{Action: agent.HookActionContinue}, nil +} + +func getWeatherData(city string) string { + // 实现天气查询逻辑 + return fmt.Sprintf("%s天气：晴，温度20°C", city) +} +``` + +--- + +## 总结 + +通过 hook 系统的 `respond` action，外部进程可以： + +1. **注入工具定义**：让 LLM 知道有新工具可用 +2. **提供工具实现**：直接返回执行结果，无需注册到 ToolRegistry +3. **与内置工具共存**：不影响 PicoClaw 原有工具的正常运行 + +这为插件开发提供了灵活、优雅的解决方案。 + +--- + +## 安全边界说明 + +### 绕过审批检查 + +**重要**：`respond` action 会绕过 `ApproveTool` 审批检查。 + +这意味着： +- `before_tool` hook 可以为**任何工具名称**返回 `respond`，包括敏感工具（如 `bash`） +- 工具不会经过审批流程，直接返回 hook 提供的结果 +- 这是为了支持插件工具而设计，但也带来了安全风险 + +### 安全建议 + +1. **审查 hook 配置**：确保只有可信的 hook 进程被启用 +2. **限制 hook 权限**：在 hook 实现中添加自己的安全检查 +3. **优先使用 `deny_tool`**：对于拒绝执行，使用 `deny_tool` action 而非 `respond` 返回错误 + +### 示例：hook 内置安全检查 + +```python +def handle_before_tool(params: dict) -> dict: + tool = params.get("tool", "") + args = params.get("arguments", {}) + + # 安全检查：只处理插件工具 + if tool in ["get_weather", "calculate"]: + return { + "action": "respond", + "result": execute_plugin_tool(tool, args), + } + + # 其他工具继续正常流程（会经过审批） + return {"action": "continue"} +``` + +这样可以确保 hook 只影响插件工具，不影响系统工具的审批流程。 \ No newline at end of file diff --git a/pkg/agent/hook_process.go b/pkg/agent/hook_process.go index e5632913d..59dc8ad62 100644 --- a/pkg/agent/hook_process.go +++ b/pkg/agent/hook_process.go @@ -13,6 +13,7 @@ import ( "time" "github.com/sipeed/picoclaw/pkg/logger" + "github.com/sipeed/picoclaw/pkg/tools" ) const ( @@ -90,7 +91,8 @@ type processHookAfterLLMResponse struct { type processHookBeforeToolResponse struct { processHookDecisionResponse - Call *ToolCallHookRequest `json:"call,omitempty"` + Call *ToolCallHookRequest `json:"call,omitempty"` + Result *tools.ToolResult `json:"result,omitempty"` // Result returned directly by hook (for respond action) } type processHookAfterToolResponse struct { @@ -241,6 +243,10 @@ func (ph *ProcessHook) BeforeTool( if resp.Call == nil { resp.Call = call } + // If hook returned a Result, carry it in ToolCallHookRequest + if resp.Result != nil { + resp.Call.HookResult = resp.Result + } return resp.Call, HookDecision{Action: resp.Action, Reason: resp.Reason}, nil } diff --git a/pkg/agent/hooks.go b/pkg/agent/hooks.go index c1ef58ffd..c23961dc6 100644 --- a/pkg/agent/hooks.go +++ b/pkg/agent/hooks.go @@ -25,6 +25,7 @@ type HookAction string const ( HookActionContinue HookAction = "continue" HookActionModify HookAction = "modify" + HookActionRespond HookAction = "respond" // Return result directly, skip tool execution. SECURITY: This bypasses ApproveTool checks, allowing hooks to return results for any tool (including sensitive ones like bash) without approval. Use with caution. HookActionDenyTool HookAction = "deny_tool" HookActionAbortTurn HookAction = "abort_turn" HookActionHardAbort HookAction = "hard_abort" @@ -127,11 +128,12 @@ func (r *LLMHookResponse) Clone() *LLMHookResponse { } type ToolCallHookRequest struct { - Meta EventMeta `json:"meta"` - Tool string `json:"tool"` - Arguments map[string]any `json:"arguments,omitempty"` - Channel string `json:"channel,omitempty"` - ChatID string `json:"chat_id,omitempty"` + Meta EventMeta `json:"meta"` + Tool string `json:"tool"` + Arguments map[string]any `json:"arguments,omitempty"` + Channel string `json:"channel,omitempty"` + ChatID string `json:"chat_id,omitempty"` + HookResult *tools.ToolResult `json:"hook_result,omitempty"` // Result returned directly by hook (for respond action). Media is supported - see Media handling section in docs. } func (r *ToolCallHookRequest) Clone() *ToolCallHookRequest { @@ -140,6 +142,7 @@ func (r *ToolCallHookRequest) Clone() *ToolCallHookRequest { } cloned := *r cloned.Arguments = cloneStringAnyMap(r.Arguments) + cloned.HookResult = cloneToolResult(r.HookResult) return &cloned } @@ -382,6 +385,10 @@ func (hm *HookManager) BeforeTool( if next != nil { current = next } + case HookActionRespond: + // Hook returns result directly, skip tool execution + // Carry HookResult in ToolCallHookRequest and return + return next, decision case HookActionDenyTool, HookActionAbortTurn, HookActionHardAbort: return current, decision default: @@ -793,6 +800,13 @@ func cloneToolResult(result *tools.ToolResult) *tools.ToolResult { if len(result.Media) > 0 { cloned.Media = append([]string(nil), result.Media...) } + if len(result.ArtifactTags) > 0 { + cloned.ArtifactTags = append([]string(nil), result.ArtifactTags...) + } + if len(result.Messages) > 0 { + cloned.Messages = make([]providers.Message, len(result.Messages)) + copy(cloned.Messages, result.Messages) + } return &cloned } diff --git a/pkg/agent/hooks_test.go b/pkg/agent/hooks_test.go index 49e1b1784..92e9caae9 100644 --- a/pkg/agent/hooks_test.go +++ b/pkg/agent/hooks_test.go @@ -2,6 +2,7 @@ package agent import ( "context" + "errors" "os" "sync" "testing" @@ -10,6 +11,7 @@ import ( "github.com/sipeed/picoclaw/pkg/bus" "github.com/sipeed/picoclaw/pkg/config" "github.com/sipeed/picoclaw/pkg/providers" + "github.com/sipeed/picoclaw/pkg/routing" "github.com/sipeed/picoclaw/pkg/tools" ) @@ -343,3 +345,518 @@ func TestAgentLoop_Hooks_ToolApproverCanDeny(t *testing.T) { t.Fatalf("expected skipped reason %q, got %q", expected, payload.Reason) } } + +// respondHook is a test hook for testing HookActionRespond functionality +type respondHook struct { + respondTools map[string]bool // tool names to respond to +} + +func (h *respondHook) BeforeTool( + ctx context.Context, + call *ToolCallHookRequest, +) (*ToolCallHookRequest, HookDecision, error) { + if h.respondTools[call.Tool] { + next := call.Clone() + next.HookResult = &tools.ToolResult{ + ForLLM: "hook-responded: " + call.Tool, + ForUser: "", + Silent: false, + IsError: false, + } + return next, HookDecision{Action: HookActionRespond}, nil + } + return call, HookDecision{Action: HookActionContinue}, nil +} + +func (h *respondHook) AfterTool( + ctx context.Context, + result *ToolResultHookResponse, +) (*ToolResultHookResponse, HookDecision, error) { + // Should not be called since respond skips tool execution + return result, HookDecision{Action: HookActionContinue}, nil +} + +func TestAgentLoop_Hooks_ToolRespondAction(t *testing.T) { + provider := &toolHookProvider{} + al, agent, cleanup := newHookTestLoop(t, provider) + defer cleanup() + + al.RegisterTool(&echoTextTool{}) + if err := al.MountHook(NamedHook("respond-hook", &respondHook{ + respondTools: map[string]bool{"echo_text": true}, + })); err != nil { + t.Fatalf("MountHook failed: %v", err) + } + + sub := al.SubscribeEvents(16) + defer al.UnsubscribeEvents(sub.ID) + + resp, err := al.runAgentLoop(context.Background(), agent, processOptions{ + SessionKey: "session-1", + Channel: "cli", + ChatID: "direct", + UserMessage: "run tool", + DefaultResponse: defaultResponse, + EnableSummary: false, + SendResponse: false, + }) + if err != nil { + t.Fatalf("runAgentLoop failed: %v", err) + } + + // Verify response comes from hook, not tool + expected := "hook-responded: echo_text" + if resp != expected { + t.Fatalf("expected %q, got %q", expected, resp) + } + + // Verify event stream has ToolExecEnd, not actual tool execution + events := collectEventStream(sub.C) + endEvt, ok := findEvent(events, EventKindToolExecEnd) + if !ok { + t.Fatal("expected tool exec end event") + } + payload, ok := endEvt.Payload.(ToolExecEndPayload) + if !ok { + t.Fatalf("expected ToolExecEndPayload, got %T", endEvt.Payload) + } + if payload.Tool != "echo_text" { + t.Fatalf("expected tool echo_text, got %q", payload.Tool) + } + if payload.ForLLMLen != len(expected) { + t.Fatalf("expected ForLLMLen %d, got %d", len(expected), payload.ForLLMLen) + } +} + +// denyToolHook tests HookActionDenyTool functionality +type denyToolHook struct { + denyTools map[string]bool +} + +func (h *denyToolHook) BeforeTool( + ctx context.Context, + call *ToolCallHookRequest, +) (*ToolCallHookRequest, HookDecision, error) { + if h.denyTools[call.Tool] { + return call, HookDecision{Action: HookActionDenyTool, Reason: "tool denied by hook"}, nil + } + return call, HookDecision{Action: HookActionContinue}, nil +} + +func (h *denyToolHook) AfterTool( + ctx context.Context, + result *ToolResultHookResponse, +) (*ToolResultHookResponse, HookDecision, error) { + return result, HookDecision{Action: HookActionContinue}, nil +} + +func TestAgentLoop_Hooks_ToolDenyAction(t *testing.T) { + provider := &toolHookProvider{} + al, agent, cleanup := newHookTestLoop(t, provider) + defer cleanup() + + al.RegisterTool(&echoTextTool{}) + if err := al.MountHook(NamedHook("deny-hook", &denyToolHook{ + denyTools: map[string]bool{"echo_text": true}, + })); err != nil { + t.Fatalf("MountHook failed: %v", err) + } + + resp, err := al.runAgentLoop(context.Background(), agent, processOptions{ + SessionKey: "session-1", + Channel: "cli", + ChatID: "direct", + UserMessage: "run tool", + DefaultResponse: defaultResponse, + EnableSummary: false, + SendResponse: false, + }) + if err != nil { + t.Fatalf("runAgentLoop failed: %v", err) + } + + expected := "Tool execution denied by hook: tool denied by hook" + if resp != expected { + t.Fatalf("expected %q, got %q", expected, resp) + } +} + +func TestHookManager_BeforeTool_RespondAction(t *testing.T) { + hm := NewHookManager(nil) + defer hm.Close() + + hook := &respondHook{ + respondTools: map[string]bool{"test_tool": true}, + } + if err := hm.Mount(NamedHook("respond-test", hook)); err != nil { + t.Fatalf("mount hook: %v", err) + } + + req := &ToolCallHookRequest{ + Tool: "test_tool", + Arguments: map[string]any{"arg": "value"}, + } + result, decision := hm.BeforeTool(context.Background(), req) + + if decision.Action != HookActionRespond { + t.Fatalf("expected action %q, got %q", HookActionRespond, decision.Action) + } + + if result.HookResult == nil { + t.Fatal("expected HookResult to be set") + } + if result.HookResult.ForLLM != "hook-responded: test_tool" { + t.Fatalf("unexpected HookResult.ForLLM: %q", result.HookResult.ForLLM) + } +} + +type respondWithMediaHook struct { + respondTools map[string]bool + media []string + responseHandled bool + forLLM string + sendMediaErr error +} + +func (h *respondWithMediaHook) BeforeTool( + ctx context.Context, + call *ToolCallHookRequest, +) (*ToolCallHookRequest, HookDecision, error) { + if h.respondTools[call.Tool] { + next := call.Clone() + next.HookResult = &tools.ToolResult{ + ForLLM: h.forLLM, + ForUser: "media result", + Media: h.media, + ResponseHandled: h.responseHandled, + Silent: false, + IsError: false, + } + return next, HookDecision{Action: HookActionRespond}, nil + } + return call, HookDecision{Action: HookActionContinue}, nil +} + +func (h *respondWithMediaHook) AfterTool( + ctx context.Context, + result *ToolResultHookResponse, +) (*ToolResultHookResponse, HookDecision, error) { + return result, HookDecision{Action: HookActionContinue}, nil +} + +type errorMediaChannel struct { + fakeChannel + sendErr error +} + +func (f *errorMediaChannel) SendMedia(ctx context.Context, msg bus.OutboundMediaMessage) ([]string, error) { + return nil, f.sendErr +} + +func TestAgentLoop_HookRespond_MediaError(t *testing.T) { + provider := &multiToolProvider{ + toolCalls: []providers.ToolCall{ + {ID: "call-1", Name: "media_tool", Arguments: map[string]any{}}, + }, + finalContent: "done", + } + al, agent, cleanup := newHookTestLoop(t, provider) + defer cleanup() + + hook := &respondWithMediaHook{ + respondTools: map[string]bool{"media_tool": true}, + media: []string{"media://test/image.png"}, + responseHandled: true, + forLLM: "media sent successfully", + } + if err := al.MountHook(NamedHook("media-hook", hook)); err != nil { + t.Fatalf("MountHook failed: %v", err) + } + + al.channelManager = newStartedTestChannelManager(t, al.bus, al.mediaStore, "discord", &errorMediaChannel{ + sendErr: errors.New("channel unavailable"), + }) + + sub := al.SubscribeEvents(16) + defer al.UnsubscribeEvents(sub.ID) + + _, err := al.runAgentLoop(context.Background(), agent, processOptions{ + SessionKey: "session-media-err", + Channel: "discord", + ChatID: "chat1", + UserMessage: "send media", + DefaultResponse: defaultResponse, + EnableSummary: false, + SendResponse: false, + }) + if err != nil { + t.Fatalf("runAgentLoop failed: %v", err) + } + + events := collectEventStream(sub.C) + endEvt, ok := findEvent(events, EventKindToolExecEnd) + if !ok { + t.Fatal("expected ToolExecEnd event") + } + payload, ok := endEvt.Payload.(ToolExecEndPayload) + if !ok { + t.Fatalf("expected ToolExecEndPayload, got %T", endEvt.Payload) + } + + if !payload.IsError { + t.Fatal("expected IsError=true when SendMedia fails") + } + + if payload.ForLLMLen < 30 { + t.Fatalf("expected ForLLM to contain error message, got ForLLMLen=%d", payload.ForLLMLen) + } +} + +func TestAgentLoop_HookRespond_BusFallback(t *testing.T) { + provider := &multiToolProvider{ + toolCalls: []providers.ToolCall{ + {ID: "call-1", Name: "media_tool", Arguments: map[string]any{}}, + }, + finalContent: "done", + } + al, agent, cleanup := newHookTestLoop(t, provider) + defer cleanup() + + hook := &respondWithMediaHook{ + respondTools: map[string]bool{"media_tool": true}, + media: []string{"media://test/image.png"}, + responseHandled: true, + forLLM: "media queued", + } + if err := al.MountHook(NamedHook("media-hook", hook)); err != nil { + t.Fatalf("MountHook failed: %v", err) + } + + sub := al.SubscribeEvents(16) + defer al.UnsubscribeEvents(sub.ID) + + resp, err := al.runAgentLoop(context.Background(), agent, processOptions{ + SessionKey: "session-bus-fallback", + Channel: "cli", + ChatID: "chat1", + UserMessage: "send media", + DefaultResponse: defaultResponse, + EnableSummary: false, + SendResponse: false, + }) + if err != nil { + t.Fatalf("runAgentLoop failed: %v", err) + } + + events := collectEventStream(sub.C) + endEvt, ok := findEvent(events, EventKindToolExecEnd) + if !ok { + t.Fatal("expected ToolExecEnd event") + } + payload, ok := endEvt.Payload.(ToolExecEndPayload) + if !ok { + t.Fatalf("expected ToolExecEndPayload, got %T", endEvt.Payload) + } + + if payload.IsError { + t.Fatal("expected IsError=false for bus fallback (media queued, not delivered)") + } + + if resp != "done" { + t.Fatalf("expected response 'done', got %q", resp) + } +} + +type multiToolProvider struct { + mu sync.Mutex + callCount int + toolCalls []providers.ToolCall + finalContent string +} + +func (p *multiToolProvider) Chat( + ctx context.Context, + messages []providers.Message, + tools []providers.ToolDefinition, + model string, + opts map[string]any, +) (*providers.LLMResponse, error) { + p.mu.Lock() + defer p.mu.Unlock() + + p.callCount++ + if p.callCount == 1 && len(p.toolCalls) > 0 { + return &providers.LLMResponse{ + ToolCalls: p.toolCalls, + }, nil + } + + return &providers.LLMResponse{ + Content: p.finalContent, + }, nil +} + +func (p *multiToolProvider) GetDefaultModel() string { + return "multi-tool-provider" +} + +func TestAgentLoop_HookRespond_InterruptSkipsRemaining(t *testing.T) { + provider := &multiToolProvider{ + toolCalls: []providers.ToolCall{ + {ID: "call-1", Name: "tool_one", Arguments: map[string]any{}}, + {ID: "call-2", Name: "tool_two", Arguments: map[string]any{}}, + {ID: "call-3", Name: "tool_three", Arguments: map[string]any{}}, + }, + finalContent: "done", + } + al, _, cleanup := newHookTestLoop(t, provider) + defer cleanup() + + tool1ExecCh := make(chan struct{}, 1) + al.RegisterTool(&slowTool{name: "tool_two", duration: 100 * time.Millisecond, execCh: tool1ExecCh}) + al.RegisterTool(&slowTool{name: "tool_three", duration: 100 * time.Millisecond}) + + hook := &respondHook{ + respondTools: map[string]bool{"tool_one": true}, + } + if err := al.MountHook(NamedHook("respond-hook", hook)); err != nil { + t.Fatalf("MountHook failed: %v", err) + } + + sub := al.SubscribeEvents(32) + defer al.UnsubscribeEvents(sub.ID) + + sessionKey := routing.BuildAgentMainSessionKey(routing.DefaultAgentID) + + type result struct { + resp string + err error + } + resultCh := make(chan result, 1) + go func() { + resp, err := al.ProcessDirectWithChannel( + context.Background(), + "run tools", + sessionKey, + "cli", + "chat1", + ) + resultCh <- result{resp: resp, err: err} + }() + + time.Sleep(50 * time.Millisecond) + + if err := al.InterruptGraceful("stop now"); err != nil { + t.Fatalf("InterruptGraceful failed: %v", err) + } + + select { + case r := <-resultCh: + if r.err != nil { + t.Fatalf("unexpected error: %v", r.err) + } + case <-time.After(3 * time.Second): + t.Fatal("timeout waiting for result") + } + + events := collectEventStream(sub.C) + + skippedEvts := filterEvents(events, EventKindToolExecSkipped) + if len(skippedEvts) < 1 { + t.Fatal("expected at least one ToolExecSkipped event after interrupt") + } + + for _, evt := range skippedEvts { + payload, ok := evt.Payload.(ToolExecSkippedPayload) + if !ok { + t.Fatalf("expected ToolExecSkippedPayload, got %T", evt.Payload) + } + if payload.Reason != "graceful interrupt requested" { + t.Fatalf("expected skip reason 'graceful interrupt requested', got %q", payload.Reason) + } + } +} + +func TestAgentLoop_HookRespond_SteeringSkipsRemaining(t *testing.T) { + provider := &multiToolProvider{ + toolCalls: []providers.ToolCall{ + {ID: "call-1", Name: "tool_one", Arguments: map[string]any{}}, + {ID: "call-2", Name: "tool_two", Arguments: map[string]any{}}, + {ID: "call-3", Name: "tool_three", Arguments: map[string]any{}}, + }, + finalContent: "done", + } + al, _, cleanup := newHookTestLoop(t, provider) + defer cleanup() + + al.RegisterTool(&slowTool{name: "tool_two", duration: 100 * time.Millisecond}) + al.RegisterTool(&slowTool{name: "tool_three", duration: 100 * time.Millisecond}) + + hook := &respondHook{ + respondTools: map[string]bool{"tool_one": true}, + } + if err := al.MountHook(NamedHook("respond-hook", hook)); err != nil { + t.Fatalf("MountHook failed: %v", err) + } + + sub := al.SubscribeEvents(32) + defer al.UnsubscribeEvents(sub.ID) + + sessionKey := routing.BuildAgentMainSessionKey(routing.DefaultAgentID) + + type result struct { + resp string + err error + } + resultCh := make(chan result, 1) + go func() { + resp, err := al.ProcessDirectWithChannel( + context.Background(), + "run tools", + sessionKey, + "cli", + "chat1", + ) + resultCh <- result{resp: resp, err: err} + }() + + time.Sleep(50 * time.Millisecond) + + al.Steer(providers.Message{Role: "user", Content: "change direction"}) + + select { + case r := <-resultCh: + if r.err != nil { + t.Fatalf("unexpected error: %v", r.err) + } + case <-time.After(3 * time.Second): + t.Fatal("timeout waiting for result") + } + + events := collectEventStream(sub.C) + + skippedEvts := filterEvents(events, EventKindToolExecSkipped) + if len(skippedEvts) < 1 { + t.Fatal("expected at least one ToolExecSkipped event after steering") + } + + for _, evt := range skippedEvts { + payload, ok := evt.Payload.(ToolExecSkippedPayload) + if !ok { + t.Fatalf("expected ToolExecSkippedPayload, got %T", evt.Payload) + } + if payload.Reason != "queued user steering message" { + t.Fatalf("expected skip reason 'queued user steering message', got %q", payload.Reason) + } + } +} + +func filterEvents(events []Event, kind EventKind) []Event { + var result []Event + for _, evt := range events { + if evt.Kind == kind { + result = append(result, evt) + } + } + return result +} diff --git a/pkg/agent/loop.go b/pkg/agent/loop.go index 369928d78..11446d222 100644 --- a/pkg/agent/loop.go +++ b/pkg/agent/loop.go @@ -2352,6 +2352,236 @@ turnLoop: toolName = toolReq.Tool toolArgs = toolReq.Arguments } + case HookActionRespond: + // Hook returns result directly, skip tool execution. + // SECURITY: This bypasses ApproveTool, allowing hooks to respond + // for any tool name without approval. This is intentional for + // plugin tools but means a before_tool hook can override even + // sensitive tools like bash. Hook configuration should be + // carefully reviewed to prevent unauthorized tool execution. + if toolReq != nil && toolReq.HookResult != nil { + hookResult := toolReq.HookResult + + argsJSON, _ := json.Marshal(toolArgs) + argsPreview := utils.Truncate(string(argsJSON), 200) + logger.InfoCF("agent", fmt.Sprintf("Tool call (hook respond): %s(%s)", toolName, argsPreview), + map[string]any{ + "agent_id": ts.agent.ID, + "tool": toolName, + "iteration": iteration, + }) + + // Emit ToolExecStart event (same as normal tool execution) + al.emitEvent( + EventKindToolExecStart, + ts.eventMeta("runTurn", "turn.tool.start"), + ToolExecStartPayload{ + Tool: toolName, + Arguments: cloneEventArguments(toolArgs), + }, + ) + + // Send tool feedback to chat channel if enabled (same as normal tool execution) + if al.cfg.Agents.Defaults.IsToolFeedbackEnabled() && + ts.channel != "" && + !ts.opts.SuppressToolFeedback { + argsJSON, _ := json.Marshal(toolArgs) + feedbackPreview := utils.Truncate( + string(argsJSON), + al.cfg.Agents.Defaults.GetToolFeedbackMaxArgsLength(), + ) + feedbackMsg := fmt.Sprintf("\U0001f527 `%s`\n```\n%s\n```", toolName, feedbackPreview) + fbCtx, fbCancel := context.WithTimeout(turnCtx, 3*time.Second) + _ = al.bus.PublishOutbound(fbCtx, bus.OutboundMessage{ + Channel: ts.channel, + ChatID: ts.chatID, + Content: feedbackMsg, + }) + fbCancel() + } + + toolDuration := time.Duration(0) // Hook execution time unknown + + // Send ForUser content to user + // For ResponseHandled results, send regardless of SendResponse setting, + // same as normal tool execution path. + shouldSendForUser := !hookResult.Silent && hookResult.ForUser != "" && + (ts.opts.SendResponse || hookResult.ResponseHandled) + if shouldSendForUser { + al.bus.PublishOutbound(ctx, bus.OutboundMessage{ + Channel: ts.channel, + ChatID: ts.chatID, + Content: hookResult.ForUser, + Metadata: map[string]string{ + "is_tool_call": "true", + }, + }) + } + + // Handle media from hook result (same as normal tool execution) + if len(hookResult.Media) > 0 && hookResult.ResponseHandled { + parts := make([]bus.MediaPart, 0, len(hookResult.Media)) + for _, ref := range hookResult.Media { + part := bus.MediaPart{Ref: ref} + if al.mediaStore != nil { + if _, meta, err := al.mediaStore.ResolveWithMeta(ref); err == nil { + part.Filename = meta.Filename + part.ContentType = meta.ContentType + part.Type = inferMediaType(meta.Filename, meta.ContentType) + } + } + parts = append(parts, part) + } + outboundMedia := bus.OutboundMediaMessage{ + Channel: ts.channel, + ChatID: ts.chatID, + Parts: parts, + } + if al.channelManager != nil && ts.channel != "" && !constants.IsInternalChannel(ts.channel) { + if err := al.channelManager.SendMedia(ctx, outboundMedia); err != nil { + logger.WarnCF("agent", "Failed to deliver hook media", + map[string]any{ + "agent_id": ts.agent.ID, + "tool": toolName, + "channel": ts.channel, + "chat_id": ts.chatID, + "error": err.Error(), + }) + // Same as normal tool execution: notify LLM about delivery failure + hookResult.IsError = true + hookResult.ForLLM = fmt.Sprintf("failed to deliver attachment: %v", err) + } + } else if al.bus != nil { + al.bus.PublishOutboundMedia(ctx, outboundMedia) + // Same as normal tool execution: bus only queues, media not yet delivered + hookResult.ResponseHandled = false + } + } + + // Track response handling status (same as normal tool execution) + if !hookResult.ResponseHandled { + allResponsesHandled = false + } + + // Build tool message + contentForLLM := hookResult.ContentForLLM() + if al.cfg.Tools.IsFilterSensitiveDataEnabled() { + contentForLLM = al.cfg.FilterSensitiveData(contentForLLM) + } + + toolResultMsg := providers.Message{ + Role: "tool", + Content: contentForLLM, + ToolCallID: tc.ID, + } + + // Handle media for LLM vision (same as normal tool execution) + if len(hookResult.Media) > 0 && !hookResult.ResponseHandled { + hookResult.ArtifactTags = buildArtifactTags(al.mediaStore, hookResult.Media) + // Recalculate contentForLLM after adding ArtifactTags + contentForLLM = hookResult.ContentForLLM() + if al.cfg.Tools.IsFilterSensitiveDataEnabled() { + contentForLLM = al.cfg.FilterSensitiveData(contentForLLM) + } + toolResultMsg.Content = contentForLLM + toolResultMsg.Media = append(toolResultMsg.Media, hookResult.Media...) + } + + // Emit ToolExecEnd event (after filtering, same as normal tool execution) + al.emitEvent( + EventKindToolExecEnd, + ts.eventMeta("runTurn", "turn.tool.end"), + ToolExecEndPayload{ + Tool: toolName, + Duration: toolDuration, + ForLLMLen: len(contentForLLM), + ForUserLen: len(hookResult.ForUser), + IsError: hookResult.IsError, + Async: hookResult.Async, + }, + ) + + messages = append(messages, toolResultMsg) + if !ts.opts.NoHistory { + ts.agent.Sessions.AddFullMessage(ts.sessionKey, toolResultMsg) + ts.recordPersistedMessage(toolResultMsg) + ts.ingestMessage(turnCtx, al, toolResultMsg) + } + + // Same as normal tool execution: check for steering/interrupt/SubTurn after each tool + if steerMsgs := al.dequeueSteeringMessagesForScope(ts.sessionKey); len(steerMsgs) > 0 { + pendingMessages = append(pendingMessages, steerMsgs...) + } + + skipReason := "" + skipMessage := "" + if len(pendingMessages) > 0 { + skipReason = "queued user steering message" + skipMessage = "Skipped due to queued user message." + } else if gracefulPending, _ := ts.gracefulInterruptRequested(); gracefulPending { + skipReason = "graceful interrupt requested" + skipMessage = "Skipped due to graceful interrupt." + } + + if skipReason != "" { + remaining := len(normalizedToolCalls) - i - 1 + if remaining > 0 { + logger.InfoCF("agent", "Turn checkpoint: skipping remaining tools after hook respond", + map[string]any{ + "agent_id": ts.agent.ID, + "completed": i + 1, + "skipped": remaining, + "reason": skipReason, + }) + for j := i + 1; j < len(normalizedToolCalls); j++ { + skippedTC := normalizedToolCalls[j] + al.emitEvent( + EventKindToolExecSkipped, + ts.eventMeta("runTurn", "turn.tool.skipped"), + ToolExecSkippedPayload{ + Tool: skippedTC.Name, + Reason: skipReason, + }, + ) + skippedMsg := providers.Message{ + Role: "tool", + Content: skipMessage, + ToolCallID: skippedTC.ID, + } + messages = append(messages, skippedMsg) + if !ts.opts.NoHistory { + ts.agent.Sessions.AddFullMessage(ts.sessionKey, skippedMsg) + ts.recordPersistedMessage(skippedMsg) + } + } + } + break + } + + // Also poll for any SubTurn results that arrived during tool execution. + if ts.pendingResults != nil { + select { + case result, ok := <-ts.pendingResults: + if ok && result != nil && result.ForLLM != "" { + content := al.cfg.FilterSensitiveData(result.ForLLM) + msg := providers.Message{Role: "user", Content: fmt.Sprintf("[SubTurn Result] %s", content)} + messages = append(messages, msg) + ts.agent.Sessions.AddFullMessage(ts.sessionKey, msg) + } + default: + // No results available + } + } + + continue + } + // If no HookResult, fall back to continue with warning + logger.WarnCF("agent", "Hook returned respond action but no HookResult provided", + map[string]any{ + "agent_id": ts.agent.ID, + "tool": toolName, + "action": "respond", + }) case HookActionDenyTool: allResponsesHandled = false denyContent := hookDeniedToolContent("Tool execution denied by hook", decision.Reason) From 7d167646749b11b54a3d27c42fa2d5bf05e88381 Mon Sep 17 00:00:00 2001 From: wenjie Date: Wed, 8 Apr 2026 14:23:21 +0800 Subject: [PATCH 3/4] fix(gateway): validate PID ownership and clean stale pid files (#2422) * fix(gateway): validate PID ownership and clean stale pid files - include `pid` in health responses for runtime PID verification - add `RemovePidFileIfPID` to safely delete PID files only on PID match - sanitize gateway PID data via process-command checks with health fallback - ignore and remove stale/non-gateway PID files before gateway operations - refuse stop/restart actions when the attached process is not a gateway - update gateway and websocket tests to cover PID validation and safety paths * test(seahorse): use shared in-memory SQLite DB in tests to fix async compaction failures * test: remove unused sendMediaErr field from hook test mock --- pkg/agent/hooks_test.go | 1 - pkg/health/server.go | 3 + pkg/pid/pidfile.go | 24 ++++ pkg/pid/pidfile_test.go | 34 +++++ pkg/seahorse/schema_test.go | 14 +- web/backend/api/gateway.go | 172 +++++++++++++++++++++- web/backend/api/gateway_test.go | 246 ++++++++++++++++++++++++++++++-- web/backend/api/pico.go | 2 +- web/backend/api/pico_test.go | 60 ++++++-- 9 files changed, 528 insertions(+), 28 deletions(-) diff --git a/pkg/agent/hooks_test.go b/pkg/agent/hooks_test.go index 92e9caae9..9049a5c72 100644 --- a/pkg/agent/hooks_test.go +++ b/pkg/agent/hooks_test.go @@ -515,7 +515,6 @@ type respondWithMediaHook struct { media []string responseHandled bool forLLM string - sendMediaErr error } func (h *respondWithMediaHook) BeforeTool( diff --git a/pkg/health/server.go b/pkg/health/server.go index 2602cb965..a152d8ab1 100644 --- a/pkg/health/server.go +++ b/pkg/health/server.go @@ -7,6 +7,7 @@ import ( "fmt" "maps" "net/http" + "os" "sync" "time" ) @@ -31,6 +32,7 @@ type Check struct { type StatusResponse struct { Status string `json:"status"` Uptime string `json:"uptime"` + PID int `json:"pid,omitempty"` Checks map[string]Check `json:"checks,omitempty"` } @@ -170,6 +172,7 @@ func (s *Server) healthHandler(w http.ResponseWriter, r *http.Request) { resp := StatusResponse{ Status: "ok", Uptime: uptime.String(), + PID: os.Getpid(), } json.NewEncoder(w).Encode(resp) diff --git a/pkg/pid/pidfile.go b/pkg/pid/pidfile.go index 0b6d461c2..f7c1f42b2 100644 --- a/pkg/pid/pidfile.go +++ b/pkg/pid/pidfile.go @@ -151,6 +151,30 @@ func RemovePidFile(homePath string) { os.Remove(pidPath) } +// RemovePidFileIfPID deletes the PID file only when the recorded PID matches +// expectedPID. It returns true when the file is removed successfully. +func RemovePidFileIfPID(homePath string, expectedPID int) bool { + if expectedPID <= 0 { + return false + } + + pidMu.Lock() + defer pidMu.Unlock() + + pidPath := pidFilePath(homePath) + data, err := readPidFileUnlocked(pidPath) + if err != nil { + return false + } + if data.PID != expectedPID { + return false + } + if err := os.Remove(pidPath); err != nil { + return false + } + return true +} + // readPidFileUnlocked reads the PID file without acquiring the lock. // Caller must hold pidMu. func readPidFileUnlocked(pidPath string) (*PidFileData, error) { diff --git a/pkg/pid/pidfile_test.go b/pkg/pid/pidfile_test.go index e54b93f4f..2da44bbbc 100644 --- a/pkg/pid/pidfile_test.go +++ b/pkg/pid/pidfile_test.go @@ -244,6 +244,40 @@ func TestRemovePidFileNonexistent(t *testing.T) { RemovePidFile(dir) } +func TestRemovePidFileIfPID(t *testing.T) { + dir := tmpDir(t) + + other := PidFileData{PID: 99999999, Token: "deadbeef12345678deadbeef12345678"} + raw, _ := json.MarshalIndent(other, "", " ") + path := filepath.Join(dir, pidFileName) + os.WriteFile(path, raw, 0o600) + + removed := RemovePidFileIfPID(dir, 99999999) + if !removed { + t.Fatal("expected RemovePidFileIfPID to remove matching pid file") + } + if _, err := os.Stat(path); !os.IsNotExist(err) { + t.Error("PID file should be removed for matching expected PID") + } +} + +func TestRemovePidFileIfPIDMismatch(t *testing.T) { + dir := tmpDir(t) + + other := PidFileData{PID: 99999999, Token: "deadbeef12345678deadbeef12345678"} + raw, _ := json.MarshalIndent(other, "", " ") + path := filepath.Join(dir, pidFileName) + os.WriteFile(path, raw, 0o600) + + removed := RemovePidFileIfPID(dir, 88888888) + if removed { + t.Fatal("expected RemovePidFileIfPID to keep non-matching pid file") + } + if _, err := os.Stat(path); os.IsNotExist(err) { + t.Error("PID file should NOT be removed for mismatching expected PID") + } +} + // TestReadPidFileUnlockedInvalidJSON returns error for malformed content. func TestReadPidFileUnlockedInvalidJSON(t *testing.T) { dir := tmpDir(t) diff --git a/pkg/seahorse/schema_test.go b/pkg/seahorse/schema_test.go index 17879f66c..e11e6e96e 100644 --- a/pkg/seahorse/schema_test.go +++ b/pkg/seahorse/schema_test.go @@ -2,14 +2,26 @@ package seahorse import ( "database/sql" + "fmt" + "strings" + "sync/atomic" "testing" _ "modernc.org/sqlite" ) +var testDBCounter uint64 + func openTestDB(t *testing.T) *sql.DB { t.Helper() - db, err := sql.Open("sqlite", ":memory:") + + n := atomic.AddUint64(&testDBCounter, 1) + testName := strings.NewReplacer("/", "_", " ", "_").Replace(t.Name()) + // Use a shared in-memory database so concurrent goroutines/connections in tests + // observe the same schema/data. + dsn := fmt.Sprintf("file:seahorse_test_%s_%d?mode=memory&cache=shared", testName, n) + + db, err := sql.Open("sqlite", dsn) if err != nil { t.Fatalf("open test db: %v", err) } diff --git a/web/backend/api/gateway.go b/web/backend/api/gateway.go index 139f2c8c8..8994e9c60 100644 --- a/web/backend/api/gateway.go +++ b/web/backend/api/gateway.go @@ -108,6 +108,8 @@ var gatewayHealthGet = func(url string, timeout time.Duration) (*http.Response, return client.Get(url) } +var gatewayProcessMatcher = isLikelyGatewayProcess + // getGatewayHealth checks the gateway health endpoint and returns the status response. // Returns (*health.StatusResponse, statusCode, error). If error is not nil, the other values are not valid. func (h *Handler) getGatewayHealth(cfg *config.Config, timeout time.Duration) (*health.StatusResponse, int, error) { @@ -117,7 +119,7 @@ func (h *Handler) getGatewayHealth(cfg *config.Config, timeout time.Duration) (* gateway.mu.Lock() if d := gateway.pidData; d != nil && d.Port > 0 { port = d.Port - host = d.Host + host = gatewayProbeHost(d.Host) } gateway.mu.Unlock() if port == 0 { @@ -150,6 +152,150 @@ func getGatewayHealthByURL(url string, timeout time.Duration) (*health.StatusRes return &healthResponse, resp.StatusCode, nil } +// isLikelyGatewayProcess returns whether PID appears to be a picoclaw gateway +// process plus whether inspection was conclusive on this platform/environment. +func isLikelyGatewayProcess(pid int) (bool, bool) { + if pid <= 0 { + return false, true + } + + if runtime.GOOS == "windows" { + psCmd := fmt.Sprintf( + `$p=Get-CimInstance Win32_Process -Filter "ProcessId = %d"; if ($null -eq $p) { "" } else { $p.CommandLine }`, + pid, + ) + out, err := exec.Command("powershell", "-NoProfile", "-NonInteractive", "-Command", psCmd).Output() + if err == nil { + cmdline := strings.TrimSpace(string(out)) + if cmdline != "" { + return looksLikeGatewayCommandLine(cmdline), true + } + } + + // Fallback: determine only whether the process still exists. + out, err = exec.Command("tasklist", "/FI", "PID eq "+strconv.Itoa(pid), "/FO", "CSV", "/NH").Output() + if err != nil { + return false, false + } + line := strings.ToLower(strings.TrimSpace(string(out))) + if line == "" { + return false, true + } + // A CSV row means the process exists, but may have a custom executable + // name we cannot classify here. + if strings.HasPrefix(line, "\"") { + if strings.Contains(line, "\"picoclaw.exe\"") { + return true, true + } + return false, false + } + if strings.Contains(line, "no tasks are running") { + return false, true + } + return false, true + } + + out, err := exec.Command("ps", "-o", "command=", "-p", strconv.Itoa(pid)).Output() + if err != nil { + return false, false + } + cmdline := strings.ToLower(strings.TrimSpace(string(out))) + if cmdline == "" { + return false, true + } + return looksLikeGatewayCommandLine(cmdline), true +} + +// looksLikeGatewayCommandLine checks whether a process command line likely +// represents "picoclaw gateway ..." regardless of executable filename. +func looksLikeGatewayCommandLine(cmdline string) bool { + fields := strings.Fields(strings.ToLower(strings.TrimSpace(cmdline))) + if len(fields) == 0 { + return false + } + for _, f := range fields { + token := strings.Trim(f, `"'`) + if token == "gateway" || strings.HasSuffix(token, "/gateway") || strings.HasSuffix(token, `\gateway`) { + return true + } + } + return false +} + +func (h *Handler) getGatewayHealthForPidData( + pidData *ppid.PidFileData, + cfg *config.Config, + timeout time.Duration, +) (*health.StatusResponse, int, error) { + if pidData == nil { + return nil, 0, errors.New("nil pid data") + } + + port := pidData.Port + if port == 0 { + port = 18790 + if cfg != nil && cfg.Gateway.Port != 0 { + port = cfg.Gateway.Port + } + } + + host := gatewayProbeHost(strings.TrimSpace(pidData.Host)) + if host == "" { + host = gatewayProbeHost(h.effectiveGatewayBindHost(cfg)) + } + if host == "" { + host = "127.0.0.1" + } + + url := "http://" + net.JoinHostPort(host, strconv.Itoa(port)) + "/health" + return getGatewayHealthByURL(url, timeout) +} + +func (h *Handler) validateGatewayPidData( + pidData *ppid.PidFileData, + cfg *config.Config, +) (ok bool, decisive bool, reason string) { + if pidData == nil || pidData.PID <= 0 { + return false, true, "invalid pid data" + } + + if gatewayProcess, inspected := gatewayProcessMatcher(pidData.PID); inspected { + if !gatewayProcess { + return false, true, "pid process command is not picoclaw gateway" + } + return true, true, "" + } + + healthResp, statusCode, err := h.getGatewayHealthForPidData(pidData, cfg, 800*time.Millisecond) + if err != nil { + return false, false, fmt.Sprintf("health probe failed: %v", err) + } + if statusCode != http.StatusOK { + return false, false, fmt.Sprintf("health endpoint returned status %d", statusCode) + } + if healthResp.PID > 0 && healthResp.PID != pidData.PID { + return false, true, fmt.Sprintf("health pid mismatch: pidFile=%d, health=%d", pidData.PID, healthResp.PID) + } + return true, true, "" +} + +func (h *Handler) sanitizeGatewayPidData(pidData *ppid.PidFileData, cfg *config.Config) *ppid.PidFileData { + if pidData == nil { + return nil + } + + ok, decisive, reason := h.validateGatewayPidData(pidData, cfg) + if ok { + return pidData + } + + logger.Warnf("ignore pid file for PID %d: %s", pidData.PID, reason) + if decisive && ppid.RemovePidFileIfPID(globalConfigDir(), pidData.PID) { + logger.Warnf("removed stale pid file for PID %d", pidData.PID) + } + return nil +} + // registerGatewayRoutes binds gateway lifecycle endpoints to the ServeMux. func (h *Handler) registerGatewayRoutes(mux *http.ServeMux) { mux.HandleFunc("GET /api/gateway/status", h.handleGatewayStatus) @@ -164,7 +310,7 @@ func (h *Handler) registerGatewayRoutes(mux *http.ServeMux) { // starts it when possible. Intended to be called by the backend at startup. func (h *Handler) TryAutoStartGateway() { // Check PID file first to detect an already-running gateway. - pidData := ppid.ReadPidFileWithCheck(globalConfigDir()) + pidData := h.sanitizeGatewayPidData(ppid.ReadPidFileWithCheck(globalConfigDir()), nil) if pidData != nil { gateway.mu.Lock() ready, reason, err := h.gatewayStartReady() @@ -472,6 +618,11 @@ func stopGatewayLocked() (int, error) { } pid := gateway.cmd.Process.Pid + if !gateway.owned { + if isGateway, inspected := gatewayProcessMatcher(pid); inspected && !isGateway { + return pid, fmt.Errorf("refuse to stop non-gateway process (PID %d)", pid) + } + } // Send SIGTERM for graceful shutdown (SIGKILL on Windows) var sigErr error @@ -681,7 +832,7 @@ func (h *Handler) startGatewayLocked(initialStatus string, existingPid int) (int // POST /api/gateway/start func (h *Handler) handleGatewayStart(w http.ResponseWriter, r *http.Request) { // Check PID file first to detect an already-running gateway. - pidData := ppid.ReadPidFileWithCheck(globalConfigDir()) + pidData := h.sanitizeGatewayPidData(ppid.ReadPidFileWithCheck(globalConfigDir()), nil) if pidData != nil { pid := pidData.PID gateway.mu.Lock() @@ -807,9 +958,22 @@ func (h *Handler) RestartGateway() (int, error) { gateway.mu.Lock() previousCmd := gateway.cmd + previousOwned := gateway.owned setGatewayRuntimeStatusLocked("restarting") gateway.mu.Unlock() + if previousCmd != nil && previousCmd.Process != nil && !previousOwned { + if isGateway, inspected := gatewayProcessMatcher(previousCmd.Process.Pid); inspected && !isGateway { + logger.Warnf("refuse restarting non-gateway process (PID: %d)", previousCmd.Process.Pid) + gateway.mu.Lock() + if gateway.cmd == previousCmd { + setGatewayRuntimeStatusLocked("running") + } + gateway.mu.Unlock() + return 0, fmt.Errorf("refuse to restart non-gateway process (PID %d)", previousCmd.Process.Pid) + } + } + if err = stopGatewayProcessForRestart(previousCmd); err != nil { gateway.mu.Lock() if gateway.cmd == previousCmd { @@ -921,7 +1085,7 @@ func (h *Handler) gatewayStatusData() map[string]any { } // Primary detection: read PID file and check if process is alive. - pidData := ppid.ReadPidFileWithCheck(globalConfigDir()) + pidData := h.sanitizeGatewayPidData(ppid.ReadPidFileWithCheck(globalConfigDir()), cfg) if pidData != nil { gateway.mu.Lock() gateway.pidData = pidData diff --git a/web/backend/api/gateway_test.go b/web/backend/api/gateway_test.go index 1f5f13e27..d300b657c 100644 --- a/web/backend/api/gateway_test.go +++ b/web/backend/api/gateway_test.go @@ -15,8 +15,6 @@ import ( "testing" "time" - "github.com/stretchr/testify/require" - "github.com/sipeed/picoclaw/pkg/auth" "github.com/sipeed/picoclaw/pkg/config" ppid "github.com/sipeed/picoclaw/pkg/pid" @@ -40,6 +38,36 @@ func startLongRunningProcess(t *testing.T) *exec.Cmd { return cmd } +func startGatewayLikeProcess(t *testing.T) *exec.Cmd { + t.Helper() + + var cmd *exec.Cmd + if runtime.GOOS == "windows" { + t.Skip("gateway-like process commandline check is not deterministic on Windows tests") + } + cmd = exec.Command("sh", "-c", "sleep 30 # picoclaw gateway") + + if err := cmd.Start(); err != nil { + t.Fatalf("Start() error = %v", err) + } + + return cmd +} + +func writeTestPidFile(t *testing.T, data ppid.PidFileData) string { + t.Helper() + + path := filepath.Join(globalConfigDir(), ".picoclaw.pid") + raw, err := json.MarshalIndent(data, "", " ") + if err != nil { + t.Fatalf("marshal pid file: %v", err) + } + if err := os.WriteFile(path, raw, 0o600); err != nil { + t.Fatalf("write pid file: %v", err) + } + return path +} + func mockGatewayHealthResponse(statusCode, pid int) *http.Response { return &http.Response{ StatusCode: statusCode, @@ -68,12 +96,14 @@ func resetGatewayTestState(t *testing.T) { t.Helper() originalHealthGet := gatewayHealthGet + originalProcessMatcher := gatewayProcessMatcher originalRestartGracePeriod := gatewayRestartGracePeriod originalRestartForceKillWindow := gatewayRestartForceKillWindow originalRestartPollInterval := gatewayRestartPollInterval t.Setenv("PICOCLAW_HOME", t.TempDir()) t.Cleanup(func() { gatewayHealthGet = originalHealthGet + gatewayProcessMatcher = originalProcessMatcher gatewayRestartGracePeriod = originalRestartGracePeriod gatewayRestartForceKillWindow = originalRestartForceKillWindow gatewayRestartPollInterval = originalRestartPollInterval @@ -105,6 +135,105 @@ func TestGatewayStartReady_NoDefaultModel(t *testing.T) { } } +func TestLooksLikeGatewayCommandLine(t *testing.T) { + cases := []struct { + name string + cmdline string + want bool + }{ + { + name: "default picoclaw gateway", + cmdline: "/usr/local/bin/picoclaw gateway -E", + want: true, + }, + { + name: "renamed binary with gateway subcommand", + cmdline: "/opt/bin/custom-claw gateway -E -d", + want: true, + }, + { + name: "standalone gateway binary path", + cmdline: "/opt/bin/gateway -E", + want: true, + }, + { + name: "non gateway process", + cmdline: "/bin/sleep 30", + want: false, + }, + { + name: "gateway substring only", + cmdline: "/opt/bin/gatewayd --serve", + want: false, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := looksLikeGatewayCommandLine(tc.cmdline) + if got != tc.want { + t.Fatalf("looksLikeGatewayCommandLine(%q) = %v, want %v", tc.cmdline, got, tc.want) + } + }) + } +} + +func TestValidateGatewayPidDataAcceptsHealthWhenMatcherInconclusive(t *testing.T) { + resetGatewayTestState(t) + + configPath := filepath.Join(t.TempDir(), "config.json") + h := NewHandler(configPath) + + const testPID = 34567 + pidData := &ppid.PidFileData{ + PID: testPID, + Host: "127.0.0.1", + Port: 18790, + } + + gatewayProcessMatcher = func(int) (bool, bool) { return false, false } + gatewayHealthGet = func(string, time.Duration) (*http.Response, error) { + return mockGatewayHealthResponse(http.StatusOK, testPID), nil + } + + ok, decisive, reason := h.validateGatewayPidData(pidData, nil) + if !ok { + t.Fatalf("validateGatewayPidData() ok = false, want true (reason=%q)", reason) + } + if !decisive { + t.Fatalf("validateGatewayPidData() decisive = false, want true") + } +} + +func TestValidateGatewayPidDataRejectsHealthPidMismatchWhenMatcherInconclusive(t *testing.T) { + resetGatewayTestState(t) + + configPath := filepath.Join(t.TempDir(), "config.json") + h := NewHandler(configPath) + + pidData := &ppid.PidFileData{ + PID: 34567, + Host: "127.0.0.1", + Port: 18790, + } + + gatewayProcessMatcher = func(int) (bool, bool) { return false, false } + gatewayHealthGet = func(string, time.Duration) (*http.Response, error) { + return mockGatewayHealthResponse(http.StatusOK, 99999), nil + } + + ok, decisive, reason := h.validateGatewayPidData(pidData, nil) + if ok { + t.Fatalf("validateGatewayPidData() ok = true, want false") + } + if !decisive { + t.Fatalf("validateGatewayPidData() decisive = false, want true") + } + if !strings.Contains(reason, "health pid mismatch") { + t.Fatalf("validateGatewayPidData() reason = %q, want contains %q", reason, "health pid mismatch") + } +} + func TestGatewayStartReady_InvalidDefaultModel(t *testing.T) { configPath := filepath.Join(t.TempDir(), "config.json") cfg := config.DefaultConfig() @@ -533,7 +662,7 @@ func TestGatewayStatusDowngradesRunningWhenTrackedProcessExitedAndPidFileMissing } } -func TestGatewayStatusReportsRunningFromPidProbe(t *testing.T) { +func TestGatewayStatusIgnoresAndRemovesPidFileForNonGatewayProcess(t *testing.T) { resetGatewayTestState(t) configPath := filepath.Join(t.TempDir(), "config.json") @@ -549,6 +678,87 @@ func TestGatewayStatusReportsRunningFromPidProbe(t *testing.T) { _ = cmd.Wait() }) + pidPath := writeTestPidFile(t, ppid.PidFileData{ + PID: cmd.Process.Pid, + Token: "stale-token", + Host: "127.0.0.1", + Port: 18790, + }) + + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodGet, "/api/gateway/status", nil) + mux.ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want %d", rec.Code, http.StatusOK) + } + + var body map[string]any + if err := json.Unmarshal(rec.Body.Bytes(), &body); err != nil { + t.Fatalf("unmarshal response: %v", err) + } + if got := body["gateway_status"]; got != "stopped" { + t.Fatalf("gateway_status = %#v, want %q", got, "stopped") + } + if _, err := os.Stat(pidPath); !os.IsNotExist(err) { + t.Fatal("stale pid file should be removed for non-gateway process") + } +} + +func TestGatewayStopRefusesNonGatewayAttachedProcess(t *testing.T) { + resetGatewayTestState(t) + if runtime.GOOS == "windows" { + t.Skip("commandline-based process type check is best-effort on Windows") + } + + configPath := filepath.Join(t.TempDir(), "config.json") + h := NewHandler(configPath) + mux := http.NewServeMux() + h.RegisterRoutes(mux) + + cmd := startLongRunningProcess(t) + t.Cleanup(func() { + if cmd.Process != nil { + _ = cmd.Process.Kill() + } + _ = cmd.Wait() + }) + + gateway.mu.Lock() + gateway.cmd = cmd + gateway.owned = false + setGatewayRuntimeStatusLocked("running") + gateway.mu.Unlock() + + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodPost, "/api/gateway/stop", nil) + mux.ServeHTTP(rec, req) + + if rec.Code != http.StatusInternalServerError { + t.Fatalf("status = %d, want %d", rec.Code, http.StatusInternalServerError) + } + if !isCmdProcessAliveLocked(cmd) { + t.Fatal("non-gateway process should not be terminated by /api/gateway/stop") + } +} + +func TestGatewayStatusReportsRunningFromPidProbe(t *testing.T) { + resetGatewayTestState(t) + gatewayProcessMatcher = func(int) (bool, bool) { return true, true } + + configPath := filepath.Join(t.TempDir(), "config.json") + h := NewHandler(configPath) + mux := http.NewServeMux() + h.RegisterRoutes(mux) + + cmd := startGatewayLikeProcess(t) + t.Cleanup(func() { + if cmd.Process != nil { + _ = cmd.Process.Kill() + } + _ = cmd.Wait() + }) + gateway.mu.Lock() setGatewayRuntimeStatusLocked("stopped") gateway.mu.Unlock() @@ -557,8 +767,12 @@ func TestGatewayStatusReportsRunningFromPidProbe(t *testing.T) { return mockGatewayHealthResponse(http.StatusOK, cmd.Process.Pid), nil } - _, err := ppid.WritePidFile(globalConfigDir(), "localhost", 0) - require.NoError(t, err) + writeTestPidFile(t, ppid.PidFileData{ + PID: cmd.Process.Pid, + Token: "test-token", + Host: "127.0.0.1", + Port: 18790, + }) rec := httptest.NewRecorder() req := httptest.NewRequest(http.MethodGet, "/api/gateway/status", nil) @@ -583,6 +797,7 @@ func TestGatewayStatusReportsRunningFromPidProbe(t *testing.T) { func TestGatewayStatusRequiresRestartAfterDefaultModelChange(t *testing.T) { resetGatewayTestState(t) + gatewayProcessMatcher = func(int) (bool, bool) { return true, true } configPath := filepath.Join(t.TempDir(), "config.json") cfg := config.DefaultConfig() @@ -601,16 +816,23 @@ func TestGatewayStatusRequiresRestartAfterDefaultModelChange(t *testing.T) { mux := http.NewServeMux() h.RegisterRoutes(mux) - process, err := os.FindProcess(os.Getpid()) - if err != nil { - t.Fatalf("FindProcess() error = %v", err) - } - _, err = ppid.WritePidFile(globalConfigDir(), "localhost", 0) - require.NoError(t, err) + cmd := startGatewayLikeProcess(t) + t.Cleanup(func() { + if cmd.Process != nil { + _ = cmd.Process.Kill() + } + _ = cmd.Wait() + }) + writeTestPidFile(t, ppid.PidFileData{ + PID: cmd.Process.Pid, + Token: "test-token", + Host: "127.0.0.1", + Port: 18790, + }) bootSignature := computeConfigSignature(cfg) gateway.mu.Lock() - gateway.cmd = &exec.Cmd{Process: process} + gateway.cmd = cmd gateway.bootDefaultModel = cfg.ModelList[0].ModelName gateway.bootConfigSignature = bootSignature setGatewayRuntimeStatusLocked("running") diff --git a/web/backend/api/pico.go b/web/backend/api/pico.go index 95bbfd2c1..1d6b46d32 100644 --- a/web/backend/api/pico.go +++ b/web/backend/api/pico.go @@ -64,7 +64,7 @@ func (h *Handler) handleWebSocketProxy() http.HandlerFunc { gatewayAvailable := false // Prefer fresh PID file data when available. - if pidData := ppid.ReadPidFileWithCheck(globalConfigDir()); pidData != nil { + if pidData := h.sanitizeGatewayPidData(ppid.ReadPidFileWithCheck(globalConfigDir()), nil); pidData != nil { gateway.mu.Lock() gateway.pidData = pidData setGatewayRuntimeStatusLocked("running") diff --git a/web/backend/api/pico_test.go b/web/backend/api/pico_test.go index 04888fde7..af5ba205f 100644 --- a/web/backend/api/pico_test.go +++ b/web/backend/api/pico_test.go @@ -308,6 +308,10 @@ func TestHandlePicoSetup_Response(t *testing.T) { } func TestHandleWebSocketProxyReloadsGatewayTargetFromConfig(t *testing.T) { + origMatcher := gatewayProcessMatcher + gatewayProcessMatcher = func(int) (bool, bool) { return true, true } + t.Cleanup(func() { gatewayProcessMatcher = origMatcher }) + home := t.TempDir() t.Setenv("PICOCLAW_HOME", home) @@ -339,9 +343,19 @@ func TestHandleWebSocketProxyReloadsGatewayTargetFromConfig(t *testing.T) { if err := config.SaveConfig(configPath, cfg); err != nil { t.Fatalf("SaveConfig() error = %v", err) } - if _, err := ppid.WritePidFile(globalConfigDir(), cfg.Gateway.Host, cfg.Gateway.Port); err != nil { - t.Fatalf("WritePidFile() error = %v", err) - } + cmd := startGatewayLikeProcess(t) + t.Cleanup(func() { + if cmd.Process != nil { + _ = cmd.Process.Kill() + } + _ = cmd.Wait() + }) + writeTestPidFile(t, ppid.PidFileData{ + PID: cmd.Process.Pid, + Token: "test-token", + Host: cfg.Gateway.Host, + Port: cfg.Gateway.Port, + }) origPidData := gateway.pidData origPicoToken := gateway.picoToken t.Cleanup(func() { @@ -392,6 +406,10 @@ func TestHandleWebSocketProxyReloadsGatewayTargetFromConfig(t *testing.T) { } func TestHandleWebSocketProxyLoadsCachedPicoTokenWhenMissing(t *testing.T) { + origMatcher := gatewayProcessMatcher + gatewayProcessMatcher = func(int) (bool, bool) { return true, true } + t.Cleanup(func() { gatewayProcessMatcher = origMatcher }) + home := t.TempDir() t.Setenv("PICOCLAW_HOME", home) @@ -416,9 +434,19 @@ func TestHandleWebSocketProxyLoadsCachedPicoTokenWhenMissing(t *testing.T) { if err := config.SaveConfig(configPath, cfg); err != nil { t.Fatalf("SaveConfig() error = %v", err) } - if _, err := ppid.WritePidFile(globalConfigDir(), cfg.Gateway.Host, cfg.Gateway.Port); err != nil { - t.Fatalf("WritePidFile() error = %v", err) - } + cmd := startGatewayLikeProcess(t) + t.Cleanup(func() { + if cmd.Process != nil { + _ = cmd.Process.Kill() + } + _ = cmd.Wait() + }) + writeTestPidFile(t, ppid.PidFileData{ + PID: cmd.Process.Pid, + Token: "test-token", + Host: cfg.Gateway.Host, + Port: cfg.Gateway.Port, + }) t.Cleanup(func() { ppid.RemovePidFile(globalConfigDir()) }) @@ -450,6 +478,10 @@ func TestHandleWebSocketProxyLoadsCachedPicoTokenWhenMissing(t *testing.T) { } func TestHandleWebSocketProxyLoadsPidDataOnDemand(t *testing.T) { + origMatcher := gatewayProcessMatcher + gatewayProcessMatcher = func(int) (bool, bool) { return true, true } + t.Cleanup(func() { gatewayProcessMatcher = origMatcher }) + home := t.TempDir() t.Setenv("PICOCLAW_HOME", home) @@ -475,10 +507,20 @@ func TestHandleWebSocketProxyLoadsPidDataOnDemand(t *testing.T) { t.Fatalf("SaveConfig() error = %v", err) } - pidData, err := ppid.WritePidFile(globalConfigDir(), cfg.Gateway.Host, cfg.Gateway.Port) - if err != nil { - t.Fatalf("WritePidFile() error = %v", err) + cmd := startGatewayLikeProcess(t) + t.Cleanup(func() { + if cmd.Process != nil { + _ = cmd.Process.Kill() + } + _ = cmd.Wait() + }) + pidData := ppid.PidFileData{ + PID: cmd.Process.Pid, + Token: "test-token", + Host: cfg.Gateway.Host, + Port: cfg.Gateway.Port, } + writeTestPidFile(t, pidData) t.Cleanup(func() { ppid.RemovePidFile(globalConfigDir()) }) From 8b3e5026903d4a6c02b3a7f8860d6625c4117fbe Mon Sep 17 00:00:00 2001 From: ywj <138745068+yangwenjie1231@users.noreply.github.com> Date: Wed, 8 Apr 2026 14:26:17 +0800 Subject: [PATCH 4/4] fix(feishu): enrich reply context for card and file replies (#2144) * fix(feishu): enrich reply context for card and file replies * refactor(feishu): extract reply functions to feishu_reply.go - Move reply-related functions to new feishu_reply.go - Move corresponding tests to feishu_reply_test.go - Extract magic number 600 to maxReplyContextLen constant - Unify replyTargetID/replyTargetFromMessage (prefer parent_id, fallback root_id) - Add source comment for containsFeishuUpgradePlaceholder * fix(feishu): skip API fallback for non-thread messages, prepend replied media refs - resolveReplyTargetMessageID: only call fetchMessageByID fallback when ThreadId is set, avoiding unnecessary API calls for non-reply messages - prependReplyContext: prepend replied media refs before current media refs to maintain correct ordering * fix(feishu): add message cache for fetchMessageByID to avoid repeated downloads - Add messageCache (sync.Map) to FeishuChannel struct - Cache fetched messages with 30s TTL to avoid re-downloading attachments when multiple users reply to the same parent message in a thread - Cleanup expired entries on read access (no background goroutine needed) * fix(feishu): early-return for non-reply messages, add cache and fetchMessageByID comment * fix: remove duplicate test and fix gci import order * fix(feishu): remove duplicate prependReplyContext call --- pkg/channels/feishu/feishu_64.go | 40 +-- pkg/channels/feishu/feishu_reply.go | 298 +++++++++++++++++++++++ pkg/channels/feishu/feishu_reply_test.go | 229 +++++++++++++++++ 3 files changed, 549 insertions(+), 18 deletions(-) create mode 100644 pkg/channels/feishu/feishu_reply.go create mode 100644 pkg/channels/feishu/feishu_reply_test.go diff --git a/pkg/channels/feishu/feishu_64.go b/pkg/channels/feishu/feishu_64.go index b0b231d09..c12827729 100644 --- a/pkg/channels/feishu/feishu_64.go +++ b/pkg/channels/feishu/feishu_64.go @@ -14,6 +14,7 @@ import ( "strings" "sync" "sync/atomic" + "time" lark "github.com/larksuite/oapi-sdk-go/v3" larkcore "github.com/larksuite/oapi-sdk-go/v3/core" @@ -42,12 +43,18 @@ type FeishuChannel struct { wsClient *larkws.Client tokenCache *tokenCache // custom cache that supports invalidation - botOpenID atomic.Value // stores string; populated lazily for @mention detection + botOpenID atomic.Value // stores string; populated lazily for @mention detection + messageCache sync.Map // caches fetched messages (messageID -> *larkim.Message) mu sync.Mutex cancel context.CancelFunc } +type cachedMessage struct { + msg *larkim.Message + expiry time.Time +} + func NewFeishuChannel(cfg config.FeishuConfig, bus *bus.MessageBus) (*FeishuChannel, error) { base := channels.NewBaseChannel("feishu", cfg, bus, cfg.AllowFrom, channels.WithGroupTrigger(cfg.GroupTrigger), @@ -436,24 +443,8 @@ func (c *FeishuChannel) handleMessageReceive(ctx context.Context, event *larkim. // Append media tags to content (like Telegram does) content = appendMediaTags(content, messageType, mediaRefs) - if content == "" { - content = "[empty message]" - } - - metadata := map[string]string{} - if messageID != "" { - metadata["message_id"] = messageID - } - if messageType != "" { - metadata["message_type"] = messageType - } chatType := stringValue(message.ChatType) - if chatType != "" { - metadata["chat_type"] = chatType - } - if sender != nil && sender.TenantKey != nil { - metadata["tenant_key"] = *sender.TenantKey - } + metadata := buildInboundMetadata(message, sender) var peer bus.Peer if chatType == "p2p" { @@ -477,12 +468,25 @@ func (c *FeishuChannel) handleMessageReceive(ctx context.Context, event *larkim. content = cleaned } + if replyTargetID(message) != "" || stringValue(message.ThreadId) != "" { + content, mediaRefs = c.prependReplyContext(ctx, message, chatID, content, mediaRefs) + } + if content == "" { + content = "[empty message]" + } + logger.InfoCF("feishu", "Feishu message received", map[string]any{ "sender_id": senderID, "chat_id": chatID, "message_id": messageID, "preview": utils.Truncate(content, 80), }) + logger.InfoCF("feishu", "Feishu reply linkage", map[string]any{ + "message_id": messageID, + "parent_id": stringValue(message.ParentId), + "root_id": stringValue(message.RootId), + "thread_id": stringValue(message.ThreadId), + }) c.HandleMessage(ctx, peer, messageID, senderID, chatID, content, mediaRefs, metadata, senderInfo) return nil diff --git a/pkg/channels/feishu/feishu_reply.go b/pkg/channels/feishu/feishu_reply.go new file mode 100644 index 000000000..22dfe3e87 --- /dev/null +++ b/pkg/channels/feishu/feishu_reply.go @@ -0,0 +1,298 @@ +//go:build amd64 || arm64 || riscv64 || mips64 || ppc64 + +package feishu + +import ( + "context" + "fmt" + "strings" + "time" + + larkim "github.com/larksuite/oapi-sdk-go/v3/service/im/v1" + + "github.com/sipeed/picoclaw/pkg/logger" + "github.com/sipeed/picoclaw/pkg/utils" +) + +const messageCacheTTL = 30 * time.Second + +const ( + maxReplyContextLen = 600 +) + +func (c *FeishuChannel) prependReplyContext( + ctx context.Context, + message *larkim.EventMessage, + chatID string, + content string, + mediaRefs []string, +) (string, []string) { + if message == nil { + return content, mediaRefs + } + + lookupCtx, cancel := context.WithTimeout(ctx, 5*time.Second) + defer cancel() + + targetMessageID := c.resolveReplyTargetMessageID(lookupCtx, message) + if targetMessageID == "" { + logger.DebugCF("feishu", "No reply target resolved; skip reply context", map[string]any{ + "message_id": stringValue(message.MessageId), + "parent_id": stringValue(message.ParentId), + "root_id": stringValue(message.RootId), + "thread_id": stringValue(message.ThreadId), + }) + return content, mediaRefs + } + + repliedMessage, err := c.fetchMessageByID(lookupCtx, targetMessageID) + if err != nil { + logger.DebugCF("feishu", "Failed to fetch replied message context", map[string]any{ + "target_message_id": targetMessageID, + "error": err.Error(), + }) + return content, mediaRefs + } + + messageType := stringValue(repliedMessage.MsgType) + rawContent := "" + if repliedMessage.Body != nil { + rawContent = stringValue(repliedMessage.Body.Content) + } + + var repliedMediaRefs []string + if store := c.GetMediaStore(); store != nil { + repliedMediaRefs = c.downloadInboundMedia(lookupCtx, chatID, targetMessageID, messageType, rawContent, store) + if messageType == larkim.MsgTypeInteractive { + _, externalURLs := extractCardImageKeys(rawContent) + if len(externalURLs) > 0 { + repliedMediaRefs = append(repliedMediaRefs, externalURLs...) + } + } + } + + repliedContent := normalizeRepliedContent(messageType, rawContent, repliedMediaRefs) + if len(repliedMediaRefs) > 0 { + mediaRefs = append(repliedMediaRefs, mediaRefs...) + } + + return formatReplyContext(targetMessageID, repliedContent, content), mediaRefs +} + +func (c *FeishuChannel) resolveReplyTargetMessageID(ctx context.Context, message *larkim.EventMessage) string { + if targetID := replyTargetID(message); targetID != "" { + logger.DebugCF("feishu", "Resolved reply target from event payload", map[string]any{ + "message_id": stringValue(message.MessageId), + "parent_id": stringValue(message.ParentId), + "root_id": stringValue(message.RootId), + "target_id": targetID, + }) + return targetID + } + + currentMessageID := stringValue(message.MessageId) + if currentMessageID == "" { + return "" + } + + if stringValue(message.ThreadId) == "" { + logger.DebugCF("feishu", "No reply target found; message is not in a thread", map[string]any{ + "message_id": stringValue(message.MessageId), + }) + return "" + } + + msg, err := c.fetchMessageByID(ctx, currentMessageID) + if err != nil { + logger.DebugCF("feishu", "Failed to query current message detail for reply info", map[string]any{ + "message_id": currentMessageID, + "error": err.Error(), + }) + return "" + } + + targetID := replyTargetIDFromMessage(msg) + if targetID != "" { + logger.DebugCF("feishu", "Resolved reply target from message detail", map[string]any{ + "message_id": currentMessageID, + "parent_id": stringValue(msg.ParentId), + "root_id": stringValue(msg.RootId), + "target_id": targetID, + }) + } + return targetID +} + +func (c *FeishuChannel) fetchMessageByID(ctx context.Context, messageID string) (*larkim.Message, error) { + if cached, ok := c.messageCache.Load(messageID); ok { + cm := cached.(*cachedMessage) + if time.Now().Before(cm.expiry) { + return cm.msg, nil + } + c.messageCache.Delete(messageID) + } + + req := larkim.NewGetMessageReqBuilder(). + MessageId(messageID). + Build() + + resp, err := c.client.Im.V1.Message.Get(ctx, req) + if err != nil { + return nil, fmt.Errorf("feishu get message: %w", err) + } + if !resp.Success() { + c.invalidateTokenOnAuthError(resp.Code) + return nil, fmt.Errorf("feishu get message api error (code=%d msg=%s)", resp.Code, resp.Msg) + } + if resp.Data == nil || len(resp.Data.Items) == 0 || resp.Data.Items[0] == nil { + return nil, fmt.Errorf("feishu get message: empty response") + } + // Items[0] contains the target message - the Feishu API returns a list + // but we request a single message by ID, so the list always has at most one item. + msg := resp.Data.Items[0] + c.messageCache.Store(messageID, &cachedMessage{msg: msg, expiry: time.Now().Add(messageCacheTTL)}) + return msg, nil +} + +func replyTargetID(message *larkim.EventMessage) string { + if message == nil { + return "" + } + if parentID := stringValue(message.ParentId); parentID != "" { + return parentID + } + return stringValue(message.RootId) +} + +func replyTargetIDFromMessage(message *larkim.Message) string { + if message == nil { + return "" + } + if parentID := stringValue(message.ParentId); parentID != "" { + return parentID + } + return stringValue(message.RootId) +} + +func buildInboundMetadata(message *larkim.EventMessage, sender *larkim.EventSender) map[string]string { + metadata := map[string]string{} + if message == nil { + return metadata + } + + messageID := stringValue(message.MessageId) + if messageID != "" { + metadata["message_id"] = messageID + } + + messageType := stringValue(message.MessageType) + if messageType != "" { + metadata["message_type"] = messageType + } + + chatType := stringValue(message.ChatType) + if chatType != "" { + metadata["chat_type"] = chatType + } + + parentID := stringValue(message.ParentId) + if parentID != "" { + metadata["parent_id"] = parentID + } + + rootID := stringValue(message.RootId) + if rootID != "" { + metadata["root_id"] = rootID + } + + if replyTo := replyTargetID(message); replyTo != "" { + metadata["reply_to_message_id"] = replyTo + } + + threadID := stringValue(message.ThreadId) + if threadID != "" { + metadata["thread_id"] = threadID + } + + if sender != nil && sender.TenantKey != nil && *sender.TenantKey != "" { + metadata["tenant_key"] = *sender.TenantKey + } + + return metadata +} + +func normalizeRepliedContent(messageType, rawContent string, mediaRefs []string) string { + content := extractContent(messageType, rawContent) + + if containsFeishuUpgradePlaceholder(rawContent) || containsFeishuUpgradePlaceholder(content) { + content = "" + } + + content = appendMediaTags(content, messageType, mediaRefs) + if strings.TrimSpace(content) != "" { + return content + } + + switch messageType { + case larkim.MsgTypeImage: + return "[replied image]" + case larkim.MsgTypeFile: + return "[replied file]" + case larkim.MsgTypeAudio: + return "[replied audio]" + case larkim.MsgTypeMedia: + return "[replied video]" + case larkim.MsgTypeInteractive: + return "[replied interactive card]" + default: + return "[replied message content unavailable]" + } +} + +func containsFeishuUpgradePlaceholder(s string) bool { + upgradePrompt := "\u8bf7\u5347\u7ea7\u81f3\u6700\u65b0\u7248\u672c\u5ba2\u6237\u7aef" + upgradePromptEscaped := "\\u8bf7\\u5347\\u7ea7\\u81f3\\u6700\\u65b0\\u7248\\u672c\\u5ba2\\u6237\\u7aef" + return strings.Contains(s, upgradePrompt) || strings.Contains(s, upgradePromptEscaped) +} + +func formatReplyContext(parentID, repliedContent, content string) string { + parentID = strings.TrimSpace(parentID) + repliedContent = strings.TrimSpace(repliedContent) + content = strings.TrimSpace(content) + + if parentID == "" || repliedContent == "" { + return content + } + + repliedContent = utils.Truncate(repliedContent, maxReplyContextLen) + repliedContent = sanitizeReplyContextContent(repliedContent) + content = sanitizeReplyContextContent(content) + header := fmt.Sprintf("[replied_message id=%q]", parentID) + footer := "[/replied_message]" + if content == "" { + return header + "\n" + repliedContent + "\n" + footer + } + if hasLeadingCommandPrefix(content) { + return content + "\n\n" + header + "\n" + repliedContent + "\n" + footer + } + return header + "\n" + repliedContent + "\n" + footer + "\n\n[current_message]\n" + content + "\n[/current_message]" +} + +func hasLeadingCommandPrefix(s string) bool { + tokens := strings.Fields(strings.TrimSpace(s)) + if len(tokens) == 0 { + return false + } + first := tokens[0] + return strings.HasPrefix(first, "/") || strings.HasPrefix(first, "!") +} + +func sanitizeReplyContextContent(s string) string { + tagEscaper := strings.NewReplacer( + "[replied_message", `\[replied_message`, + "[/replied_message]", `\[/replied_message]`, + "[current_message]", `\[current_message]`, + "[/current_message]", `\[/current_message]`, + ) + return tagEscaper.Replace(s) +} diff --git a/pkg/channels/feishu/feishu_reply_test.go b/pkg/channels/feishu/feishu_reply_test.go new file mode 100644 index 000000000..0efe7bc01 --- /dev/null +++ b/pkg/channels/feishu/feishu_reply_test.go @@ -0,0 +1,229 @@ +//go:build amd64 || arm64 || riscv64 || mips64 || ppc64 + +package feishu + +import ( + "strings" + "testing" + + larkim "github.com/larksuite/oapi-sdk-go/v3/service/im/v1" +) + +func TestBuildInboundMetadata(t *testing.T) { + strPtr := func(s string) *string { return &s } + + t.Run("includes basic and reply fields", func(t *testing.T) { + message := &larkim.EventMessage{ + MessageId: strPtr("om_msg_1"), + MessageType: strPtr("text"), + ChatType: strPtr("group"), + ParentId: strPtr("om_parent_1"), + RootId: strPtr("om_root_1"), + ThreadId: strPtr("omt_thread_1"), + } + sender := &larkim.EventSender{TenantKey: strPtr("tenant_x")} + + got := buildInboundMetadata(message, sender) + + if got["message_id"] != "om_msg_1" { + t.Fatalf("message_id = %q, want %q", got["message_id"], "om_msg_1") + } + if got["message_type"] != "text" { + t.Fatalf("message_type = %q, want %q", got["message_type"], "text") + } + if got["chat_type"] != "group" { + t.Fatalf("chat_type = %q, want %q", got["chat_type"], "group") + } + if got["parent_id"] != "om_parent_1" { + t.Fatalf("parent_id = %q, want %q", got["parent_id"], "om_parent_1") + } + if got["reply_to_message_id"] != "om_parent_1" { + t.Fatalf("reply_to_message_id = %q, want %q", got["reply_to_message_id"], "om_parent_1") + } + if got["root_id"] != "om_root_1" { + t.Fatalf("root_id = %q, want %q", got["root_id"], "om_root_1") + } + if got["thread_id"] != "omt_thread_1" { + t.Fatalf("thread_id = %q, want %q", got["thread_id"], "omt_thread_1") + } + if got["tenant_key"] != "tenant_x" { + t.Fatalf("tenant_key = %q, want %q", got["tenant_key"], "tenant_x") + } + }) + + t.Run("falls back reply_to_message_id to root_id", func(t *testing.T) { + message := &larkim.EventMessage{ + MessageId: strPtr("om_msg_3"), + RootId: strPtr("om_root_3"), + } + + got := buildInboundMetadata(message, nil) + + if got["root_id"] != "om_root_3" { + t.Fatalf("root_id = %q, want %q", got["root_id"], "om_root_3") + } + if got["reply_to_message_id"] != "om_root_3" { + t.Fatalf("reply_to_message_id = %q, want %q", got["reply_to_message_id"], "om_root_3") + } + }) + + t.Run("omits empty values", func(t *testing.T) { + message := &larkim.EventMessage{ + MessageId: strPtr("om_msg_2"), + } + + got := buildInboundMetadata(message, nil) + + if got["message_id"] != "om_msg_2" { + t.Fatalf("message_id = %q, want %q", got["message_id"], "om_msg_2") + } + if _, ok := got["parent_id"]; ok { + t.Fatalf("parent_id should be absent, got %q", got["parent_id"]) + } + if _, ok := got["reply_to_message_id"]; ok { + t.Fatalf("reply_to_message_id should be absent, got %q", got["reply_to_message_id"]) + } + if _, ok := got["tenant_key"]; ok { + t.Fatalf("tenant_key should be absent, got %q", got["tenant_key"]) + } + }) + + t.Run("nil message returns empty map", func(t *testing.T) { + got := buildInboundMetadata(nil, nil) + if len(got) != 0 { + t.Fatalf("len(metadata) = %d, want 0", len(got)) + } + }) +} + +func TestFormatReplyContext(t *testing.T) { + t.Run("formats reply context with content", func(t *testing.T) { + got := formatReplyContext("om_parent_1", "original message", "new reply") + want := "[replied_message id=\"om_parent_1\"]\noriginal message\n[/replied_message]\n\n[current_message]\nnew reply\n[/current_message]" + if got != want { + t.Fatalf("formatReplyContext() = %q, want %q", got, want) + } + }) + + t.Run("returns reply context when current content is empty", func(t *testing.T) { + got := formatReplyContext("om_parent_1", "original message", "") + want := "[replied_message id=\"om_parent_1\"]\noriginal message\n[/replied_message]" + if got != want { + t.Fatalf("formatReplyContext() = %q, want %q", got, want) + } + }) + + t.Run("returns original content when parent or replied content missing", func(t *testing.T) { + if got := formatReplyContext("", "original", "new reply"); got != "new reply" { + t.Fatalf("missing parent: got %q, want %q", got, "new reply") + } + if got := formatReplyContext("om_parent_1", "", "new reply"); got != "new reply" { + t.Fatalf("missing replied content: got %q, want %q", got, "new reply") + } + }) + + t.Run("escapes reserved wrapper tags in payload", func(t *testing.T) { + replied := "payload [replied_message id=\"x\"] x [/replied_message]" + current := "hello [current_message]injected[/current_message]" + got := formatReplyContext("om_parent_1", replied, current) + + if !strings.HasPrefix(got, "[replied_message id=\"om_parent_1\"]") { + t.Fatalf("outer replied_message wrapper missing: %q", got) + } + if strings.Contains(got, "\n[replied_message id=\"x\"]") { + t.Fatalf("nested replied_message tag should be escaped: %q", got) + } + if strings.Contains(got, "\n[current_message]injected") { + t.Fatalf("nested current_message tag should be escaped: %q", got) + } + if !strings.Contains(got, `\[replied_message id="x"]`) { + t.Fatalf("escaped replied tag missing: %q", got) + } + }) + + t.Run("preserves leading slash command prefix", func(t *testing.T) { + got := formatReplyContext("om_parent_1", "original message", "/help") + want := "/help\n\n[replied_message id=\"om_parent_1\"]\noriginal message\n[/replied_message]" + if got != want { + t.Fatalf("formatReplyContext() = %q, want %q", got, want) + } + }) + + t.Run("preserves leading bang command prefix", func(t *testing.T) { + got := formatReplyContext("om_parent_1", "original message", "!status now") + want := "!status now\n\n[replied_message id=\"om_parent_1\"]\noriginal message\n[/replied_message]" + if got != want { + t.Fatalf("formatReplyContext() = %q, want %q", got, want) + } + }) +} + +func TestReplyTargetID(t *testing.T) { + strPtr := func(s string) *string { return &s } + + t.Run("prefer parent_id", func(t *testing.T) { + msg := &larkim.EventMessage{ParentId: strPtr("om_parent"), RootId: strPtr("om_root")} + if got := replyTargetID(msg); got != "om_parent" { + t.Fatalf("replyTargetID() = %q, want %q", got, "om_parent") + } + }) + + t.Run("fallback to root_id", func(t *testing.T) { + msg := &larkim.EventMessage{RootId: strPtr("om_root")} + if got := replyTargetID(msg); got != "om_root" { + t.Fatalf("replyTargetID() = %q, want %q", got, "om_root") + } + }) + + t.Run("empty when no fields", func(t *testing.T) { + if got := replyTargetID(&larkim.EventMessage{}); got != "" { + t.Fatalf("replyTargetID() = %q, want empty", got) + } + }) +} + +func TestNormalizeRepliedContent(t *testing.T) { + t.Run("filters feishu upgrade placeholder for interactive", func(t *testing.T) { + raw := `{"text":"\u8bf7\u5347\u7ea7\u81f3\u6700\u65b0\u7248\u672c\u5ba2\u6237\u7aef\uff0c\u4ee5\u67e5\u770b\u5185\u5bb9"}` + got := normalizeRepliedContent("interactive", raw, nil) + if got != "[replied interactive card]" { + t.Fatalf("normalizeRepliedContent() = %q, want %q", got, "[replied interactive card]") + } + }) + + t.Run("keeps filename and file tag for replied file", func(t *testing.T) { + got := normalizeRepliedContent("file", `{"file_key":"file_xxx","file_name":"doc.pdf"}`, []string{"media://r1"}) + if got != "doc.pdf [file]" { + t.Fatalf("normalizeRepliedContent() = %q, want %q", got, "doc.pdf [file]") + } + }) + + t.Run("falls back when file content missing", func(t *testing.T) { + got := normalizeRepliedContent("file", `{"file_key":"file_xxx"}`, nil) + if got != "[replied file]" { + t.Fatalf("normalizeRepliedContent() = %q, want %q", got, "[replied file]") + } + }) +} + +func TestHasLeadingCommandPrefix(t *testing.T) { + tests := []struct { + name string + input string + want bool + }{ + {name: "slash command", input: "/help", want: true}, + {name: "bang command", input: "!status", want: true}, + {name: "leading spaces slash", input: " /ping arg", want: true}, + {name: "normal text", input: "hello /help", want: false}, + {name: "empty", input: "", want: false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := hasLeadingCommandPrefix(tt.input); got != tt.want { + t.Fatalf("hasLeadingCommandPrefix(%q) = %v, want %v", tt.input, got, tt.want) + } + }) + } +}