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/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/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 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..9049a5c72 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,517 @@ 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 +} + +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) 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) + } + }) + } +} 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()) })