diff --git a/docs/specs/edit-file-tool/plan.md b/docs/specs/edit-file-tool/plan.md new file mode 100644 index 000000000..929fd428e --- /dev/null +++ b/docs/specs/edit-file-tool/plan.md @@ -0,0 +1,228 @@ +# edit_file Tool Implementation Plan + +## Architecture Overview + +The `edit_file` tool will be integrated into the existing agent filesystem tool infrastructure, following the established three-layer architecture: + +``` +┌─────────────────────────────────────────────────────────────┐ +│ Layer 1: Tool Definition (agentToolManager.ts) │ +│ - Zod schema for parameter validation │ +│ - Tool metadata (name, description, parameters) │ +│ - Registration in filesystem tool registry │ +└─────────────────────────────────────────────────────────────┘ + │ + ▼ +┌─────────────────────────────────────────────────────────────┐ +│ Layer 2: Tool Routing (agentToolManager.ts) │ +│ - Parameter normalization (alias handling) │ +│ - Permission checks (assertWritePermission) │ +│ - Dispatch to filesystem handler │ +└─────────────────────────────────────────────────────────────┘ + │ + ▼ +┌─────────────────────────────────────────────────────────────┐ +│ Layer 3: File Operation (agentFileSystemHandler.ts) │ +│ - Path validation and resolution │ +│ - File content reading │ +│ - Exact text replacement │ +│ - Diff generation and response formatting │ +└─────────────────────────────────────────────────────────────┘ +``` + +## Design Decisions + +### 1. Parameter Alias Handling + +**Decision**: Normalize parameter names at the routing layer (agentToolManager.callFileSystemTool) rather than in Zod schema. + +**Rationale**: +- Zod does not natively support parameter aliases +- Normalizing early allows schema to use canonical names +- Keeps handler implementation clean + +**Implementation**: +```typescript +// Normalize parameter aliases before schema validation +if (toolName === 'edit_file') { + args = { + ...args, + path: args.path ?? args.file_path, + oldText: args.oldText ?? args.old_string, + newText: args.newText ?? args.new_string, + base_directory: args.base_directory, + } +} +``` + +### 2. Text Matching Strategy + +**Decision**: Case-sensitive exact string matching, replace ALL occurrences. + +**Rationale**: +- Consistent with `edit_text` tool's `edit_lines` operation +- Simple mental model for LLMs: "find this exact text, replace with that" +- Replacing all occurrences prevents partial updates which could leave code in inconsistent state + +### 3. Response Format + +**Decision**: JSON response with diff preview, matching existing filesystem tools. + +**Structure**: +```typescript +interface EditFileSuccessResponse { + success: true + originalCode: string // Truncated for large files + updatedCode: string // Truncated for large files + language: string // Detected from file extension + replacements: number // Number of replacements made +} + +interface EditFileErrorResponse { + success: false + error: string +} +``` + +## File Changes + +### Modified Files + +| File | Purpose | Lines Added/Modified | +|------|---------|----------------------| +| `src/main/presenter/agentPresenter/acp/agentToolManager.ts` | Schema, definition, routing | ~80 lines | +| `src/main/presenter/agentPresenter/acp/agentFileSystemHandler.ts` | Handler implementation | ~50 lines | + +### No New Files Required + +The implementation integrates into existing infrastructure without creating new files. + +## Implementation Details + +### Schema Definition (agentToolManager.ts) + +```typescript +edit_file: z.object({ + path: z.string().describe('Path to the file to edit'), + oldText: z + .string() + .max(10000) + .describe('The exact text to find and replace (case-sensitive)'), + newText: z.string().max(10000).describe('The replacement text'), + base_directory: z + .string() + .optional() + .describe('Base directory for resolving relative paths') +}) +``` + +### Tool Definition + +```typescript +{ + type: 'function', + function: { + name: 'edit_file', + description: + 'Make precise edits to files by replacing exact text strings. Use this for simple text replacements when you know the exact content to replace. For regex or complex operations, use edit_text instead.', + parameters: zodToJsonSchema(schemas.edit_file) + }, + server: { + name: 'agent-filesystem', + icons: '📁', + description: 'Agent FileSystem tools' + } +} +``` + +### Handler Implementation (agentFileSystemHandler.ts) + +```typescript +async editFile(args: unknown, baseDirectory?: string): Promise { + const parsed = EditFileArgsSchema.safeParse(args) + if (!parsed.success) { + throw new Error(`Invalid arguments: ${parsed.error}`) + } + + const { path: filePath, oldText, newText } = parsed.data + const validPath = await this.validatePath(filePath, baseDirectory) + + const content = await fs.readFile(validPath, 'utf-8') + + if (!content.includes(oldText)) { + throw new Error(`Cannot find the specified text to replace. The exact text was not found in the file.`) + } + + let replacementCount = 0 + const modifiedContent = content.replaceAll(oldText, () => { + replacementCount++ + return newText + }) + + await fs.writeFile(validPath, modifiedContent, 'utf-8') + + const { originalCode, updatedCode } = this.buildTruncatedDiff(content, modifiedContent, 3) + const language = getLanguageFromFilename(validPath) + + return JSON.stringify({ + success: true, + originalCode, + updatedCode, + language, + replacements: replacementCount + }) +} +``` + +## Test Strategy + +### Unit Tests (test/main/presenter/agentPresenter/acp/) + +Create `agentFileSystemHandler.editFile.test.ts`: + +- **Happy Path**: Replace single occurrence, replace multiple occurrences +- **Error Cases**: File not found, oldText not found, path outside allowed directories +- **Edge Cases**: Empty oldText, empty newText, large text content + +### Integration Tests + +- Verify tool registration and routing +- Verify permission system integration +- Verify response format consistency + +## Security Considerations + +- Path traversal prevention via existing `validatePath()` method +- Write permission enforcement via `assertWritePermission()` +- Allowed directory restriction via `isPathAllowed()` check +- Maximum text length limits (10,000 chars for oldText/newText) + +## Migration & Compatibility + +- No breaking changes to existing tools +- New tool is additive only +- No database or configuration migrations required +- No impact on existing agent workflows + +## Rollback Plan + +If issues are discovered: +1. Remove tool from `isFileSystemTool()` list to disable +2. Remove tool definition from `getFileSystemToolDefinitions()` +3. No data changes to revert (file changes are atomic writes) + +## Performance Considerations + +- File reads are limited by available memory +- `replaceAll()` is O(n*m) where n=content length, m=pattern length +- For very large files (>1MB), consider warning or limiting +- Diff truncation limits output size for large changes + +## Success Metrics + +- Tool is available in agent tool list +- Tool accepts all parameter variants (camelCase and snake_case) +- Tool successfully edits files with exact text matching +- Tool returns proper error messages for invalid inputs +- All tests pass +- Lint and typecheck pass diff --git a/docs/specs/edit-file-tool/spec.md b/docs/specs/edit-file-tool/spec.md new file mode 100644 index 000000000..4caf4c8ad --- /dev/null +++ b/docs/specs/edit-file-tool/spec.md @@ -0,0 +1,77 @@ +# edit_file Tool Specification + +## Overview + +Add a new `edit_file` tool to the agent filesystem tools that enables AI agents to make precise text-based edits to files. This tool provides a simplified interface for exact string replacement, complementing the existing `edit_text` tool which supports regex and line-based operations. + +## User Stories + +### Primary User Story + +As an AI agent interacting with DeepChat, I want to edit specific portions of a file using exact text matching so that I can make precise modifications without worrying about regex escaping or complex operations. + +### Use Cases + +1. **Simple Text Replacement** - Replace a specific function implementation with an updated version +2. **Configuration Updates** - Modify specific configuration values in config files +3. **Bug Fixes** - Replace buggy code snippets with corrected versions +4. **Refactoring** - Update variable names or function calls across files + +## Acceptance Criteria + +### Functional Requirements + +- [ ] Tool accepts `path` (or `file_path`) parameter for target file +- [ ] Tool accepts `oldText` (or `old_string`) parameter for text to find +- [ ] Tool accepts `newText` (or `new_string`) parameter for replacement text +- [ ] Tool optionally accepts `base_directory` for resolving relative paths +- [ ] Tool performs exact string matching (case-sensitive) +- [ ] Tool replaces ALL occurrences of oldText when multiple matches exist +- [ ] Tool returns a JSON response with diff preview (original vs updated) +- [ ] Tool returns clear error message when oldText is not found +- [ ] Tool requires write permission before modifying files +- [ ] Tool validates paths are within allowed directories + +### Non-Functional Requirements + +- [ ] Tool follows existing filesystem tool patterns (schema, handler, registration) +- [ ] Tool response format is consistent with other filesystem tools +- [ ] Error messages are user-friendly and actionable +- [ ] Tool is registered under `agent-filesystem` server namespace + +## Non-Goals + +- Support for regex patterns (use `edit_text` or `text_replace` instead) +- Support for partial/line-based matching (use `edit_text` with `edit_lines` instead) +- Support for dry-run mode (can be added in future if needed) +- Support for multiple file editing in single call +- Support for backup creation (handled at system level if needed) + +## Constraints + +- Maximum text length for `oldText` and `newText` should be reasonable (suggest 10,000 chars) +- Path resolution follows existing `base_directory` pattern +- Must integrate with existing permission system for write operations + +## Open Questions + +None. All clarifications resolved: + +- **Parameter naming**: Support both camelCase (`path`, `oldText`, `newText`) and snake_case (`file_path`, `old_string`, `new_string`) variants for LLM compatibility +- **Multiple matches**: Replace all occurrences (not just first) to match common editing expectations +- **Case sensitivity**: Exact matching is case-sensitive (consistent with `edit_text` behavior) + +## UI/UX Considerations + +- Tool icon should match other filesystem tools (📁) +- Tool description: "Make precise edits to files by replacing exact text strings" +- Error display should highlight what text was not found + +## Related Tools + +| Tool | Purpose | When to Use | +|------|---------|-------------| +| `write_file` | Overwrite entire file | Creating new files or full rewrites | +| `edit_text` | Pattern/line-based editing | Regex replacement or complex edits | +| `text_replace` | Regex-based replacement | Pattern-based text replacement | +| `edit_file` | Exact string replacement | Simple, precise text modifications | diff --git a/docs/specs/edit-file-tool/tasks.md b/docs/specs/edit-file-tool/tasks.md new file mode 100644 index 000000000..c5f11fdb6 --- /dev/null +++ b/docs/specs/edit-file-tool/tasks.md @@ -0,0 +1,118 @@ +# edit_file Tool Implementation Tasks + +## Task List + +### Phase 1: Schema and Definition + +- [ ] **Task 1.1**: Add `edit_file` schema to `fileSystemSchemas` in `agentToolManager.ts` + - Location: `src/main/presenter/agentPresenter/acp/agentToolManager.ts` (line ~69-214) + - Add after `text_replace` schema + - Include: path, oldText, newText, base_directory fields + - Add max length validation (10000 chars) for oldText/newText + +- [ ] **Task 1.2**: Add `edit_file` tool definition to `getFileSystemToolDefinitions()` + - Location: `src/main/presenter/agentPresenter/acp/agentToolManager.ts` (line ~411-630) + - Add after `text_replace` definition + - Description: "Make precise edits to files by replacing exact text strings" + - Icon: 📁 (same as other filesystem tools) + +- [ ] **Task 1.3**: Add `'edit_file'` to `isFileSystemTool()` method + - Location: `src/main/presenter/agentPresenter/acp/agentToolManager.ts` (line ~655-671) + - Add to filesystemTools array + +### Phase 2: Routing and Parameter Normalization + +- [ ] **Task 2.1**: Add parameter normalization for `edit_file` in `callFileSystemTool()` + - Location: `src/main/presenter/agentPresenter/acp/agentToolManager.ts` (line ~673+) + - After schema validation, before switch statement + - Normalize: file_path → path, old_string → oldText, new_string → newText + +- [ ] **Task 2.2**: Add `edit_file` case to switch statement in `callFileSystemTool()` + - Location: `src/main/presenter/agentPresenter/acp/agentToolManager.ts` (line ~716-785) + - Add after `text_replace` case + - Include `assertWritePermission()` check + - Call `fileSystemHandler.editFile()` + +### Phase 3: Handler Implementation + +- [ ] **Task 3.1**: Add `EditFileArgsSchema` to `agentFileSystemHandler.ts` + - Location: `src/main/presenter/agentPresenter/acp/agentFileSystemHandler.ts` + - Add after `TextReplaceArgsSchema` (~line 107) + - Define: path, oldText, newText, base_directory + +- [ ] **Task 3.2**: Implement `editFile()` method in `AgentFileSystemHandler` + - Location: `src/main/presenter/agentPresenter/acp/agentFileSystemHandler.ts` + - Add after `textReplace()` method + - Steps: + 1. Parse and validate arguments + 2. Resolve and validate path + 3. Read file content + 4. Check if oldText exists + 5. Replace all occurrences using replaceAll() + 6. Write modified content + 7. Generate diff and return JSON response + +### Phase 4: Quality Assurance + +- [ ] **Task 4.1**: Run type checking + ```bash + pnpm run typecheck + ``` + +- [ ] **Task 4.2**: Run linting + ```bash + pnpm run lint + ``` + +- [ ] **Task 4.3**: Run formatting + ```bash + pnpm run format + ``` + +- [ ] **Task 4.4**: Run tests + ```bash + pnpm test + ``` + +## Verification Checklist + +### Manual Testing + +- [ ] Tool appears in agent tool list +- [ ] Tool accepts `path`, `oldText`, `newText` parameters +- [ ] Tool accepts `file_path`, `old_string`, `new_string` aliases +- [ ] Tool successfully replaces text in a file +- [ ] Tool replaces ALL occurrences when multiple matches exist +- [ ] Tool returns proper error when oldText not found +- [ ] Tool returns proper error when file not found +- [ ] Tool respects write permissions +- [ ] Tool respects path validation (outside allowed directories) + +### Code Review Checklist + +- [ ] Schema uses appropriate validation (max length, required fields) +- [ ] Parameter aliases are normalized before schema validation +- [ ] Error messages are user-friendly +- [ ] Response format matches other filesystem tools +- [ ] Code follows existing style conventions +- [ ] No unnecessary comments or dead code + +## Commit Suggestion + +``` +feat(agent): add edit_file tool for precise text editing + +Add new filesystem tool that enables AI agents to make precise +text-based edits using exact string matching. + +Features: +- Exact text replacement with case-sensitive matching +- Support for parameter aliases (path/file_path, oldText/old_string, newText/new_string) +- Replace all occurrences of matching text +- JSON diff response with language detection +- Write permission enforcement + +Files modified: +- src/main/presenter/agentPresenter/acp/agentToolManager.ts +- src/main/presenter/agentPresenter/acp/agentFileSystemHandler.ts +``` diff --git a/docs/specs/permission-flow-stabilization/plan.md b/docs/specs/permission-flow-stabilization/plan.md new file mode 100644 index 000000000..ebf54d0be --- /dev/null +++ b/docs/specs/permission-flow-stabilization/plan.md @@ -0,0 +1,107 @@ +# Plan: 权限流程稳定性（多工具/批量）(v2) + +## 范围与原则 + +- **批次语义**：一次 assistant message 产出的 tool call 视为同一批次；permission 与恢复执行必须绑定该批次（messageId）。 +- **强暂停**：出现 permission-required 后，停止执行后续工具与继续对话，直到用户完成该批次内全部权限决策。 +- **顺序与幂等**：恢复后按原始 tool call 顺序执行；同一批次的恢复链路最多触发一次（互斥 + 幂等）。 +- **最小改动**：优先复用 message blocks 作为“执行事实”，session runtime 只做 UI 计数与互斥锁。 + +## 现状问题与根因（结合日志） + +- **批准后仍反复 tool_use / 不继续执行**：恢复执行后的 tool end/result 仅在内存或等待 `StreamUpdateScheduler` 异步落库（600ms），但继续生成会从 DB 重新构建上下文（`prepareConversationContext()`），读到旧 message content，模型看不到 tool 结果而再次 tool_use。 +- **重复启动 loop / 顺序更乱**：恢复链路中存在释放 resume lock 的窗口，导致同一 `messageId` 可能被重复恢复（重复 start loop / start stream）。 +- **预检查不统一/载荷丢失**：agent 工具的 pre-check 被跳过或 permission_request payload 不保真（如 paths/commandInfo），导致执行期才触发 permission-required，破坏批次顺序与可控暂停。 +- **pendingPermissions 状态残留**：空数组/指针未清理会干扰“是否还有 pending”的判断与前端展示。 + +## 目标行为（v2） + +### 状态机（概念） + +``` +generating + └─(permission-required)→ waiting_permission + └─(all permissions resolved)→ generating (resume tools) + └─(tools done + persisted)→ generating (continue model) + └─(end)→ idle +``` + +关键约束： +- `waiting_permission` 期间不允许启动新的 LLM stream，也不允许执行任何后续 tool call。 +- `resume tools` 阶段必须 single-flight（同一 messageId 只能有一个恢复链路在跑）。 +- `continue model` 前必须保证 tool 结果对 DB 读路径可见。 + +### 批次定义与事实来源 + +- **批次 ID**：`assistantMessageId(eventId/messageId)`。 +- **批次事实**：message content 中的 tool_call / permission blocks（顺序与状态）。 +- **session runtime**：仅用于： + - `pendingPermissions[]`（UI/状态栏/下一条 pending 提示） + - `permissionResumeLock`（互斥恢复） + +## 关键实现策略 + +### 1) 恢复链路 single-flight（临界区互斥） + +- `permissionResumeLock` 的作用域：覆盖 **执行工具 -> 同步落库 -> 启动继续生成** 的整个临界区。 +- 恢复过程中不在“每个 tool”之间释放 lock；仅在以下情况释放： + - 执行期再次触发 requiresPermission：立刻停止，释放 lock，回到 `waiting_permission` + - 全部工具执行完成并已同步落库，且已触发继续生成 +- 防重：对同一 `messageId` 的重复 `handlePermissionResponse` 只能有一次进入恢复临界区。 + +### 2) 工具结果可见性（同步落库） + +- 在继续生成前，必须把 `state.message.content` 同步写入 DB： + - 使用 `messageManager.editMessageSilently(messageId, JSON.stringify(state.message.content))` +- 不依赖 `StreamUpdateScheduler` 的定时 DB flush（600ms）作为一致性保障。 +- 写入必须发生在 `startStreamCompletion(conversationId, messageId)` 之前。 + +### 3) 统一 pre-check + payload 保真（MCP + agent） + +- `ToolPresenter.preCheckToolPermission` 对 agent 工具不再直接跳过。 +- `AgentToolManager` 增加 `preCheckToolPermission(toolName, args, conversationId)`： + - 只判断权限需求，不执行工具 + - write 类工具输出 `paths`；execute_command 输出 `commandInfo/commandSignature` +- `ToolCallProcessor.batchPreCheckPermissions`： + - 支持 `permissionType: 'read'|'write'|'all'|'command'` + - 保留并合并 tool 层提供的 `permissionRequest` payload，禁止丢字段 + +### 4) execute_command background 权限一致性 + +- `execute_command` 的 `background: true` 也必须先走 command permission check，禁止绕过。 + +## 工作项拆解（按优先级） + +1) **PermissionHandler：恢复临界区 + 同步落库** +- 文件：`src/main/presenter/agentPresenter/permission/permissionHandler.ts` +- 调整 `resumeToolExecutionAfterPermissions(...)`：lock 不在每个 tool 之间释放，避免重入窗口 +- 调整 `continueAfterToolsExecuted(...)`：`startStreamCompletion` 前 `editMessageSilently` + +2) **SessionManager：pendingPermissions 清理** +- 文件：`src/main/presenter/agentPresenter/session/sessionManager.ts` +- `removePendingPermission(...)`：filter 后 length=0 时把 `pendingPermission/pendingPermissions` 置空 + +3) **统一 pre-check + payload 保真** +- 文件：`src/main/presenter/toolPresenter/index.ts`（agent 工具 pre-check 不再跳过） +- 文件：`src/main/presenter/agentPresenter/acp/agentToolManager.ts`（新增 `preCheckToolPermission`） +- 文件：`src/main/presenter/agentPresenter/loop/toolCallProcessor.ts`（batch pre-check 合并 payload + command union） + +4) **background 命令权限** +- 文件：`src/main/presenter/agentPresenter/acp/agentBashHandler.ts` +- 将 command permission check 提前到 background 分支之前 + +5) **测试（必须覆盖“批准后继续执行”回归）** +- `test/main/presenter/sessionPresenter/permissionHandler.test.ts` + - 断言：恢复后会执行工具、并在继续生成前写 DB（mock `editMessageSilently`/顺序） + - 断言：同 messageId 重复触发只会进入一次恢复链路 +- `test/main/presenter/toolPresenter/toolPresenter.test.ts` + - 断言：agent 工具 pre-check 会被调用并返回 payload +- 新增：`test/main/presenter/agentPresenter/toolCallProcessor.batchPrecheck.test.ts` + - 断言：permission_request payload 不丢失（paths/commandInfo） + +## 手动验收脚本（v2） + +- 单工具：`execute_command`（非白名单/危险命令）-> 批准 -> 只执行一次 -> 继续生成（不再二次 permission-required） +- 多工具 batch：两个 tool 都需要权限 -> 逐个批准 -> 批次恢复按顺序执行 -> 继续生成 +- 部分拒绝：拒绝其中一个 -> 该 tool 产生一致 error tool result，其它照常执行并继续生成 +- background：`execute_command` + `background: true` -> 也必须弹权限 diff --git a/docs/specs/permission-flow-stabilization/spec.md b/docs/specs/permission-flow-stabilization/spec.md new file mode 100644 index 000000000..fa8c5d11d --- /dev/null +++ b/docs/specs/permission-flow-stabilization/spec.md @@ -0,0 +1,119 @@ +# 权限流程稳定性（多工具/批量）(v2) + +## 背景与问题（更新） + +当前 permission 流在多工具/批量场景下出现“批准后仍不继续执行、反复 tool_use、顺序混乱、重复启动 loop”等问题。结合日志与现有代码结构，根因主要来自两类： + +1) **可见性/持久化竞态（DB flush race）** +- permission 批准后会走“恢复执行工具 -> 继续生成”路径； +- 但工具执行产生的 message content 更新通常通过 `StreamUpdateScheduler` 异步落库（默认 600ms 周期）； +- `StreamGenerationHandler.prepareConversationContext()` 会从 DB 重新 `getMessage/getMessageHistory` 组上下文； +- 若在落库前继续生成，模型看不到刚执行的 tool end/result，于是再次 `tool_use`，形成“批准了仍重复要权限/不执行”的表象。 + +2) **恢复链路并发/重入窗口（resume re-entry）** +- 恢复执行中存在释放 resume lock 的窗口，可能导致同一 `messageId` 的恢复链路被重复触发； +- 结果是重复 start loop / 重复继续生成 / 工具执行与 UI 状态交错。 + +此外还有“预检查不统一/载荷不完整”问题会加剧混乱： +- agent 工具 pre-check 被跳过或 permission_request payload 丢失（paths/commandInfo 等），导致执行期才触发 permission-required，破坏 batch 语义与顺序可控性。 + +本规格目标：把 permission 从“提示事件”升级为**严格的执行门闩（gating latch）**，同时约束： +- 何时暂停、何时恢复、恢复只能一次； +- 恢复后工具结果必须对后续续跑模型“可见”（至少 DB 可见）； +- 批次内多权限串行处理后再统一执行。 + +## 目标 + +- 强暂停语义：出现 permission-required 后必须暂停后续工具执行与对话继续，直到用户完成决策。 +- 批次一致性：一次模型输出的多个 tool call 作为一个 batch；permission 决策绑定到该 batch（消息级）。 +- 恢复幂等：同一 `assistant messageId` 的恢复动作最多触发一次，避免重复 resume / 重复 loop。 +- 顺序与可部分拒绝：恢复后按原 tool 顺序执行；允许的执行，拒绝的回填一致错误结果并继续生成。 +- **工具结果可见性保证**：恢复执行工具后，在继续生成前，必须保证 tool 结果已写入 message content 且对“构建上下文的读路径”可见（至少 DB 可见）。 +- 不改变业务语义：不改变工具本身行为，仅修复 permission gating / resume / 持久化一致性。 + +## 非目标 + +- 不做大规模 UI 重构（可后续增强批次分组、一键允许等）。 +- 不新增权限类型（仍为 `read|write|all|command`）。 +- 不改变 MCP autoApprove 策略模型。 +- 不做跨会话/跨设备权限同步。 + +## 术语与批次定义 + +- `Batch`: 同一条 assistant message（`messageId`）中由模型一次输出的 tool_calls 集合。 +- `Permission Block`: message content 中的 `action_type: tool_call_permission` block。 +- `Pending Permission`: session runtime 中的 `pendingPermissions[]` 项。 + +批次键：`(conversationId, messageId)`。 + +## 状态机（必须满足的约束） + +### Pause（进入 waiting_permission） + +当任意 tool call 需要权限时： +- 在 message content 中创建对应 permission block（status=pending） +- session 状态进入 `waiting_permission` +- **停止执行该 batch 中任何 tool**（除非本规格显式允许“边批边跑”，本版本不允许） + +### Decide（用户逐个批准/拒绝） + +每次用户响应： +- 仅更新对应 permission block 状态为 granted/denied +- 更新 `pendingPermissions`（移除已决策项） +- 如果仍存在 pending permission：只发 `PERMISSION_UPDATED` 刷新 UI，不得触发恢复执行 + +### Resume（恢复执行，仅一次） + +当且仅当该 `messageId` 下所有 permission block 都已决策（无 pending）： +- 获取并持有 resume lock（messageId 级互斥） +- 按原 tool 顺序执行： + - granted: callTool -> tool_call running/end + - denied: 生成一致的 error tool result（例如 "User denied the request."） +- **关键：在继续生成前，必须保证 message content 已落库可见** +- 然后继续生成（startStreamCompletion / 或等价续跑） + +### Idempotency（幂等） + +- 同一个 `messageId` 的恢复链路在任意时刻只能有一个在跑； +- 重复点击批准/快速点击不会导致重复恢复； +- 如果恢复过程中再次出现新的 permission-required（执行期触发），应回到 Pause，且不会丢 batch 上下文。 + +## 数据一致性与“可见性”要求（新增） + +### 为什么必须强制落库 + +`prepareConversationContext()` 从 DB 读 message/history。若恢复执行更新只停留在内存（或等待 scheduler 600ms 异步落库），下一次生成构建上下文会读到旧内容，导致模型重复 tool_use。 + +### 规范要求 + +在 `continueAfterToolsExecuted()` 触发继续生成前： +- 必须执行一次“同步落库”动作，确保当前 message content（含 tool end/result、permission block status）对 DB 读路径可见。 + +实现策略（v1）： +- 直接调用 `messageManager.editMessageSilently(messageId, JSON.stringify(contentSnapshot))` 强制落库； +- 不依赖 `StreamUpdateScheduler` 的定时 flush 来保证一致性。 + +## 统一预检查（Pre-check）要求（补全） + +- 所有工具（MCP + agent）都必须在 batch pre-check 阶段尽可能产出 permission-required，避免执行期才触发导致顺序/批次语义被破坏。 +- `permission_request` 必须携带足够 payload 让 PermissionHandler 能正确 approve： + - command: `command`, `commandSignature`, `commandInfo`, `rememberable` + - filesystem write: `paths` +- 禁止 payload 丢失或被覆盖（合并策略：保留 tool 层提供的字段）。 + +## 验收标准（更新） + +- 批次内任意 tool 需要 permission：未决策前不执行任何 tool。 +- 同一 message 多个 permission：逐个决策不触发恢复，直到全部决策完成。 +- 批次恢复只发生一次，不出现同一 messageId 的重复 start loop。 +- 批次恢复后： + - 允许的工具产生 running/end 且结果回填； + - 拒绝的工具产生一致错误结果； + - **继续生成时模型能看到上述结果**，不会再次 tool_use 同一工具来要权限。 +- “批准后仍重复要 permission”的问题不再出现（同一 tool_call_id 不会再次 permission-required）。 + +## 设计决策（v1） + +- 恢复时机：只要 batch 内仍存在 pending permission，就不恢复执行；全部决策后统一恢复。 +- lock 粒度：messageId 级互斥，覆盖“恢复执行工具 + 同步落库 + 继续生成”的整个临界区。 +- 持久化策略：恢复路径强制同步落库，不依赖 scheduler 定时 flush。 diff --git a/docs/specs/permission-flow-stabilization/tasks.md b/docs/specs/permission-flow-stabilization/tasks.md new file mode 100644 index 000000000..8c9f5dc14 --- /dev/null +++ b/docs/specs/permission-flow-stabilization/tasks.md @@ -0,0 +1,130 @@ +# Tasks: 权限流程稳定性（多工具/批量） + +## Phase 1: Session runtime 与类型 + +### Task 1.1: 支持多条 pending permissions + 恢复互斥锁 +**Files:** +- `src/main/presenter/agentPresenter/session/sessionContext.ts` +- `src/main/presenter/agentPresenter/session/sessionManager.ts` + +**Subtasks:** +- [ ] 新增 `runtime.pendingPermissions: Array<{ messageId; toolCallId; permissionType; payload }>` +- [ ] 新增 `runtime.permissionResumeLock: { messageId; startedAt } | undefined` +- [ ] 保留 `runtime.pendingPermission` 作为兼容字段（从 `pendingPermissions[0]` 派生或镜像） +- [ ] `startLoop()`/清理逻辑同步重置新增字段 + +**Acceptance:** +- 能稳定表达同一条消息内的多条 permission 请求，不再被覆盖 + +--- + +## Phase 2: permission-required 事件落盘与暂停 + +### Task 2.1: permission-required 不覆盖、只追加/更新 +**File:** `src/main/presenter/agentPresenter/streaming/llmEventHandler.ts` + +**Subtasks:** +- [ ] `permission-required` 时把请求写入 `runtime.pendingPermissions`（按 `messageId+toolCallId` 去重） +- [ ] session 状态稳定进入 `waiting_permission`（不在未决策时回到 generating/idle） + +**Acceptance:** +- 同批次多 permission 时 UI/状态不会错乱、不会只显示最后一条 + +--- + +## Phase 3: PermissionHandler（决策与恢复） + +### Task 3.1: 决策阶段只更新，不提前恢复 +**File:** `src/main/presenter/agentPresenter/permission/permissionHandler.ts` + +**Subtasks:** +- [ ] 拆分“更新 permission blocks 状态”与“尝试恢复执行”两个步骤 +- [ ] 仅当该 message 内不存在 `tool_call_permission.status=pending` 时才允许进入恢复逻辑 +- [ ] 落盘后同步更新 generatingMessages 内的快照（避免 renderer 看到旧块） +- [ ] `command/agent-filesystem/deepchat-settings` 按精确 scope 处理，不做 serverName 一刀切批量 +- [ ] 仅在安全条件满足时才批量更新（同 serverName 且 permission 层级满足且无额外 scope） + +**Acceptance:** +- 多 permission 场景下，用户确认第 1 条后不会触发 resume/工具执行 + +--- + +### Task 3.2: 恢复执行按 tool_call 顺序、且幂等 +**File:** `src/main/presenter/agentPresenter/permission/permissionHandler.ts` + +**Subtasks:** +- [ ] 引入 `permissionResumeLock`，同一 `conversationId+messageId` 只允许一次恢复链路 +- [ ] 以 `tool_call.status=loading` blocks 作为“待执行队列”事实来源（保持原始顺序） +- [ ] 对被拒绝的 tool call：生成一致的 tool error 回填（不执行 tool） +- [ ] 对允许/无需 permission 的 tool call：串行执行并闭合 tool_call blocks +- [ ] 执行中若再次 `requiresPermission`：立刻暂停并回到 `waiting_permission`（不丢队列） +- [ ] 全部工具完成后，仅继续一次模型生成（避免重复 startLoop/重复 stream） + +**Acceptance:** +- 恢复后执行顺序正确、只恢复一次、不会漏执行“未触发 permission 的 tool call” + +--- + +## Phase 4: 权限层级（all > write > read） + +### Task 4.1: MCP session 权限检查按层级判断 +**File:** `src/main/presenter/mcpPresenter/toolManager.ts` + +**Subtasks:** +- [ ] 实现统一的 permission 比较/包含关系 +- [ ] `checkSessionPermission()` 按层级返回（不再 “任意权限=全部通过”） +- [ ] 持久化 `autoApprove` 逻辑保持一致（`all` 覆盖一切，`write` 覆盖 `read`） + +**Acceptance:** +- 已授予 `write` 时，不应再次因为 `read` 弹窗；但也不会把 `read` 误当成 `write` + +--- + +## Phase 5: Renderer（command permission 交互修正） + +### Task 5.1: “允许一次 / 允许本次会话”区分生效 +**File:** `src/renderer/src/components/message/MessageBlockPermissionRequest.vue` + +**Subtasks:** +- [ ] `Allow once` → `remember=false` +- [ ] `Allow for session` → `remember=true` + +**Acceptance:** +- command permission 可真正按“一次/会话”两种粒度授权 + +--- + +## Phase 6: Tests 与质量门禁 + +### Task 6.1: 主链路单元测试覆盖多 permission/幂等/顺序 +**Files:** +- `test/main/presenter/sessionPresenter/permissionHandler.test.ts` +- `test/main/presenter/mcpPresenter/toolManager.permission.test.ts`（建议新增） + +**Subtasks:** +- [ ] 多 permission：确认第 1 条不恢复、全部 resolved 才恢复 +- [ ] 恢复幂等：重复触发响应只执行一次恢复链路 +- [ ] 顺序：恢复后的执行顺序与 tool_call blocks 一致 +- [ ] 层级：`all > write > read` 覆盖关系正确 +- [ ] 校验并修正历史测试中与现实现不一致的断言（例如“permission block removal”类用例） + +--- + +### Task 6.2: 回归自测清单 +**Subtasks:** +- [ ] 同一轮 2+ 个 MCP tool：其中 1 个需要 permission +- [ ] 同一轮 2+ 个 MCP tool：2 个都需要 permission（同 server / 不同 permissionType） +- [ ] 混合 allow/deny：允许 1 个、拒绝 1 个，能继续回答 +- [ ] command permission：Allow once/Allow for session 都生效 + +--- + +### Task 6.3: 质量门禁 +**Commands:** +```bash +pnpm run format +pnpm run lint +pnpm run typecheck +pnpm test +``` + diff --git a/docs/specs/process-tool/plan.md b/docs/specs/process-tool/plan.md new file mode 100644 index 000000000..00ddce79d --- /dev/null +++ b/docs/specs/process-tool/plan.md @@ -0,0 +1,211 @@ +# Process Tool Implementation Plan + +## Architecture + +### Component Diagram + +``` +┌─────────────────────────────────────────────────────────────────┐ +│ AgentToolManager │ +│ ┌─────────────────┐ ┌─────────────────┐ ┌─────────────────┐ │ +│ │ fileSystem │ │ process │ │ question │ │ +│ │ tools │ │ tool (NEW) │ │ tool │ │ +│ └────────┬────────┘ └────────┬────────┘ └─────────────────┘ │ +│ │ │ │ +│ ┌────────▼────────────────────▼────────┐ │ +│ │ AgentBashHandler (modified) │ │ +│ │ - executeCommand (foreground) │ │ +│ │ - executeCommandBackground (NEW) │ │ +│ └────────┬─────────────────────────────┘ │ +│ │ │ +│ ┌────────▼──────────────────────────────────────────┐ │ +│ │ BackgroundExecSessionManager (NEW) │ │ +│ │ - Map> │ │ +│ │ - spawn processes with stdio pipes │ │ +│ │ - poll/log/write/kill/clear/remove operations │ │ +│ │ - TTL cleanup timer │ │ +│ │ - offload large outputs to files │ │ +│ └───────────────────────────────────────────────────┘ │ +└─────────────────────────────────────────────────────────────────┘ +``` + +### Data Model + +```typescript +interface BackgroundSession { + sessionId: string + conversationId: string + command: string + child: ChildProcess + status: 'running' | 'done' | 'error' | 'killed' + exitCode?: number + createdAt: number + lastAccessedAt: number + outputBuffer: string // In-memory buffer (<10KB) + outputFilePath: string|null // Offloaded content (>10KB) + totalOutputLength: number +} + +interface SessionMeta { + sessionId: string + command: string + status: 'running' | 'done' | 'error' | 'killed' + createdAt: number + lastAccessedAt: number + pid?: number + exitCode?: number + outputLength: number + offloaded: boolean +} +``` + +### Tool Schema + +```typescript +// process tool +{ + action: z.enum(['list', 'poll', 'log', 'write', 'kill', 'clear', 'remove']), + sessionId: z.string().optional(), + offset: z.number().int().min(0).optional(), // log only + limit: z.number().int().min(1).optional(), // log only + data: z.string().optional(), // write only + eof: z.boolean().optional() // write only +} + +// execute_command modification +{ + command: z.string().min(1), + timeout: z.number().min(100).optional(), + description: z.string().min(5).max(100), + background: z.boolean().optional(), // NEW + yieldMs: z.number().min(100).optional() // NEW +} +``` + +## Event Flow + +### Start Background Session + +``` +1. LLM calls execute_command with background=true +2. AgentToolManager routes to AgentBashHandler +3. AgentBashHandler calls BackgroundExecSessionManager.start() +4. Spawn child process with stdio pipes +5. Store session in Map +6. Return {status: "running", sessionId} +``` + +### Poll Output + +``` +1. LLM calls process with action="poll" +2. AgentToolManager routes to process tool handler +3. BackgroundExecSessionManager.poll() returns recent output +4. If offloaded, read last N chars from file +5. Return {status, output, exitCode?, offloaded?} +``` + +### Write Input + +``` +1. LLM calls process with action="write", data="..." +2. BackgroundExecSessionManager.write() writes to child.stdin +3. Optionally close stdin with eof=true +``` + +### Kill Session + +``` +1. LLM calls process with action="kill" +2. BackgroundExecSessionManager.kill() sends SIGTERM +3. Wait 2s, then SIGKILL if still running +4. Update session status +``` + +## File Structure + +``` +src/main/presenter/agentPresenter/acp/ +├── backgroundExecSessionManager.ts # NEW - Session manager +├── agentBashHandler.ts # MODIFY - Add background support +├── agentToolManager.ts # MODIFY - Add process tool +└── index.ts # MODIFY - Export new module + +src/main/presenter/sessionPresenter/ +└── sessionPaths.ts # EXISTING - Used for offload paths +``` + +## Offload Strategy + +``` +Output Collection: +1. Write to memory buffer initially +2. When buffer > 10KB threshold: + - Write buffer to file: ~/.deepchat/sessions//bgexec_.log + - Clear memory buffer +3. Subsequent output appended directly to file + +Poll Response: +- If offloaded: return last 500 chars from file +- Else: return last 500 chars from buffer + +Log Response: +- If offloaded: read from file with offset/limit +- Else: slice from buffer with offset/limit +``` + +## Security Considerations + +1. **Session Isolation**: Map key is `conversationId`, prevents cross-agent access +2. **Path Security**: Offload files use resolved session directory +3. **Resource Limits**: TTL cleanup prevents resource exhaustion +4. **Kill Safety**: SIGTERM before SIGKILL, timeout handling + +## Error Handling + +| Scenario | Response | +|----------|----------| +| Session not found | Error: "Session X not found" | +| Write to non-running session | Error: "Session X is not running" | +| Kill already dead session | No-op (idempotent) | +| Remove with cleanup failure | Log warning, continue removal | +| File read error | Return empty string, log warning | + +## Testing Strategy + +### Unit Tests (BackgroundExecSessionManager) + +- `start()`: Verify session creation, process spawn +- `poll()`: Verify output retrieval, offloading behavior +- `log()`: Verify pagination with offset/limit +- `write()`: Verify stdin writing +- `kill()`: Verify process termination +- `clear()`: Verify buffer/file clearing +- `remove()`: Verify complete cleanup +- `cleanup timer`: Verify TTL expiration + +### Integration Tests + +- End-to-end: execute_command (background) → process:poll → process:kill +- Offload flow: Large output → file creation → file reading +- Error scenarios: Invalid sessionId, terminated process + +## i18n Strings + +```json +{ + "tools": { + "process": { + "sessionNotFound": "Session {sessionId} not found", + "sessionNotRunning": "Session {sessionId} is not running", + "stdinNotAvailable": "Session {sessionId} stdin is not available" + } + } +} +``` + +## Migration Notes + +- No breaking changes to existing execute_command +- New parameters are optional with sensible defaults +- process tool is additive only diff --git a/docs/specs/process-tool/spec.md b/docs/specs/process-tool/spec.md new file mode 100644 index 000000000..c4895d1f2 --- /dev/null +++ b/docs/specs/process-tool/spec.md @@ -0,0 +1,100 @@ +# Process Tool Specification + +## Overview + +Add a new agent tool `process` for managing background exec sessions. This tool allows agents to run long-running commands in the background and interact with them asynchronously. + +## User Stories + +### US-1: Start Background Command +As an AI agent, I want to start a command in the background so that I can run long-running tasks without blocking the conversation. + +**Acceptance Criteria:** +- Agent can execute `execute_command` with `background: true` parameter +- Command returns immediately with a `sessionId` and `status: "running"` +- Process continues running after tool returns + +### US-2: Monitor Background Output +As an AI agent, I want to poll the output of a background command so that I can monitor its progress. + +**Acceptance Criteria:** +- `process` tool with `action: "poll"` returns recent output +- Output is truncated to last N characters (configurable, default 500) +- Returns current status: "running", "done", "error", or "killed" + +### US-3: Read Full Output +As an AI agent, I want to read the full output of a completed command so that I can analyze the results. + +**Acceptance Criteria:** +- `process` tool with `action: "log"` supports pagination via `offset` and `limit` +- Large outputs are automatically offloaded to files +- Agent can use file tools to read offloaded content + +### US-4: Send Input to Background Process +As an AI agent, I want to send input to a running background process so that I can interact with interactive commands. + +**Acceptance Criteria:** +- `process` tool with `action: "write"` sends data to stdin +- Optional `eof: true` closes stdin + +### US-5: Manage Background Sessions +As an AI agent, I want to list, kill, and clean up background sessions so that I can manage resources. + +**Acceptance Criteria:** +- `action: "list"` shows all sessions for current agent +- `action: "kill"` terminates a running session +- `action: "clear"` clears output buffer/file +- `action: "remove"` completely removes a session + +## Constraints + +### Security +- Sessions are isolated by `conversationId` (agent-scoped) +- Session IDs are cryptographically random (nanoid) +- No cross-agent session access + +### Resource Management +- Sessions are in-memory only (lost on restart) +- Automatic TTL cleanup (default 30 minutes inactivity) +- Maximum runtime limit (default 30 minutes) +- Large outputs offloaded to files (>10KB threshold) + +### Configuration +```typescript +interface ExecToolsConfig { + backgroundMs: number // Default yield window (10000) + timeoutSec: number // Max runtime (1800) + cleanupMs: number // Session TTL (1800000) + maxOutputChars: number // Poll output limit (500) +} +``` + +Environment variables: +- `PI_BASH_YIELD_MS` +- `PI_BASH_MAX_OUTPUT_CHARS` +- `OPENCLAW_BASH_PENDING_MAX_OUTPUT_CHARS` (compatibility) +- `PI_BASH_JOB_TTL_MS` + +## Non-Goals + +- PTY/terminal emulation (not needed - pipe mode is sufficient) +- Cross-agent session sharing +- Persistent sessions across restarts +- Real-time streaming output (poll-based only) + +## Open Questions + +| Question | Decision | +|----------|----------| +| PTY or pipe mode? | Pipe mode (agent-controlled) | +| Offload large outputs? | Yes, >10KB threshold | +| Always show process tool? | Yes, always visible | +| Default poll output size? | 500 characters | + +## Business Value + +Enables agents to: +1. Run long-running builds/tests without blocking +2. Monitor server processes +3. Handle interactive CLI tools +4. Better resource management for complex workflows diff --git a/docs/specs/process-tool/tasks.md b/docs/specs/process-tool/tasks.md new file mode 100644 index 000000000..de5c18019 --- /dev/null +++ b/docs/specs/process-tool/tasks.md @@ -0,0 +1,143 @@ +# Process Tool Implementation Tasks + +## Phase 1: Core Infrastructure + +### Task 1.1: Create BackgroundExecSessionManager +**File:** `src/main/presenter/agentPresenter/acp/backgroundExecSessionManager.ts` + +**Subtasks:** +- [ ] Define BackgroundSession and SessionMeta interfaces +- [ ] Implement session storage Map structure +- [ ] Implement `start()` method with process spawning +- [ ] Implement output handling (buffer → file offload) +- [ ] Implement `poll()` method (recent output only) +- [ ] Implement `log()` method (pagination support) +- [ ] Implement `write()` method (stdin writing) +- [ ] Implement `kill()` method (SIGTERM → SIGKILL) +- [ ] Implement `clear()` method (buffer/file clearing) +- [ ] Implement `remove()` method (complete cleanup) +- [ ] Implement `list()` method (session metadata) +- [ ] Implement TTL cleanup timer +- [ ] Add environment variable config support +- [ ] Export singleton instance + +**Acceptance:** +- Can start a background process +- Can poll output +- Can kill process +- Cleanup works correctly + +--- + +### Task 1.2: Export from acp/index.ts +**File:** `src/main/presenter/agentPresenter/acp/index.ts` + +**Subtasks:** +- [ ] Add export for BackgroundExecSessionManager +- [ ] Add export for backgroundExecSessionManager singleton + +--- + +## Phase 2: Tool Integration + +### Task 2.1: Modify AgentBashHandler +**File:** `src/main/presenter/agentPresenter/acp/agentBashHandler.ts` + +**Subtasks:** +- [ ] Update ExecuteCommandArgsSchema with `background` and `yieldMs` params +- [ ] Modify `executeCommand()` to detect background mode +- [ ] Add `executeCommandBackground()` method +- [ ] Import and use BackgroundExecSessionManager +- [ ] Return `{status: "running", sessionId}` for background mode + +**Acceptance:** +- `execute_command` with `background: true` returns sessionId +- Foreground mode unchanged + +--- + +### Task 2.2: Add Process Tool to AgentToolManager +**File:** `src/main/presenter/agentPresenter/acp/agentToolManager.ts` + +**Subtasks:** +- [ ] Add process tool schema definition +- [ ] Add process tool to `getAllToolDefinitions()` +- [ ] Add `isProcessTool()` helper +- [ ] Add `callProcessTool()` method +- [ ] Route process tool in `callTool()` switch +- [ ] Import BackgroundExecSessionManager + +**Process Actions to Implement:** +- [ ] `list` - return session list +- [ ] `poll` - return recent output +- [ ] `log` - return paginated output +- [ ] `write` - write to stdin +- [ ] `kill` - terminate process +- [ ] `clear` - clear output +- [ ] `remove` - delete session + +**Acceptance:** +- process tool appears in tool list +- All actions work correctly +- Proper error handling + +--- + +## Phase 3: Polish & Quality + +### Task 3.1: Add i18n Strings +**Files:** +- `src/renderer/src/i18n/en-US.json` +- `src/renderer/src/i18n/zh-CN.json` + +**Subtasks:** +- [ ] Add error message keys for process tool +- [ ] Add tool description strings + +--- + +### Task 3.2: Write Tests +**File:** `test/main/presenter/agentPresenter/acp/backgroundExecSessionManager.test.ts` + +**Subtasks:** +- [ ] Test session start +- [ ] Test poll output +- [ ] Test log pagination +- [ ] Test write to stdin +- [ ] Test kill process +- [ ] Test clear/remove +- [ ] Test list sessions +- [ ] Test TTL cleanup +- [ ] Test offload behavior + +--- + +### Task 3.3: Code Quality +**Command:** +```bash +pnpm run format +pnpm run lint +pnpm run typecheck +``` + +--- + +## Task Dependencies + +``` +Task 1.1 ──┬──→ Task 2.1 ──┬──→ Task 2.2 + │ │ + └──→ Task 1.2 ──┘ + +Task 2.2 ──┬──→ Task 3.1 + ├──→ Task 3.2 + └──→ Task 3.3 +``` + +## Definition of Done + +- [ ] All tasks complete +- [ ] Tests passing +- [ ] Lint/format/typecheck passing +- [ ] Spec/plan documents complete +- [ ] Manual testing verified diff --git a/src/main/events.ts b/src/main/events.ts index f54f14ed9..873d01942 100644 --- a/src/main/events.ts +++ b/src/main/events.ts @@ -68,7 +68,8 @@ export const CONVERSATION_EVENTS = { export const STREAM_EVENTS = { RESPONSE: 'stream:response', // 替代 stream-response END: 'stream:end', // 替代 stream-end - ERROR: 'stream:error' // 替代 stream-error + ERROR: 'stream:error', // 替代 stream-error + PERMISSION_UPDATED: 'stream:permission-updated' // 权限状态更新，通知前端刷新UI } // 系统相关事件 diff --git a/src/main/presenter/agentPresenter/acp/agentBashHandler.ts b/src/main/presenter/agentPresenter/acp/agentBashHandler.ts index ad7c8a954..2a780c5dc 100644 --- a/src/main/presenter/agentPresenter/acp/agentBashHandler.ts +++ b/src/main/presenter/agentPresenter/acp/agentBashHandler.ts @@ -13,6 +13,7 @@ import { } from '../../permission/commandPermissionService' import { getShellEnvironment, getUserShell } from './shellEnvHelper' import { registerCommandProcess, unregisterCommandProcess } from './commandProcessTracker' +import { backgroundExecSessionManager } from './backgroundExecSessionManager' const COMMAND_MAX_OUTPUT_LENGTH = 30000 const COMMAND_DEFAULT_TIMEOUT_MS = 120000 @@ -21,7 +22,9 @@ const COMMAND_KILL_GRACE_MS = 5000 const ExecuteCommandArgsSchema = z.object({ command: z.string().min(1), timeout: z.number().min(100).optional(), - description: z.string().min(5).max(100) + description: z.string().min(5).max(100), + background: z.boolean().optional().default(false), + yieldMs: z.number().min(100).optional() }) export interface ExecuteCommandOptions { @@ -43,13 +46,22 @@ export class AgentBashHandler { this.commandPermissionHandler = commandPermissionHandler } - async executeCommand(args: unknown, options: ExecuteCommandOptions = {}): Promise { + async executeCommand( + args: unknown, + options: ExecuteCommandOptions = {} + ): Promise { const parsed = ExecuteCommandArgsSchema.safeParse(args) if (!parsed.success) { throw new Error(`Invalid arguments: ${parsed.error}`) } - const { command, timeout } = parsed.data + const { command, timeout, background } = parsed.data + + // Handle background execution + if (background) { + return this.executeCommandBackground(command, timeout, options) + } + if (this.commandPermissionHandler) { const permissionCheck = this.commandPermissionHandler.checkPermission( options.conversationId, @@ -283,6 +295,59 @@ export class AgentBashHandler { }) } + private async executeCommandBackground( + command: string, + timeout: number | undefined, + options: ExecuteCommandOptions + ): Promise<{ status: 'running'; sessionId: string }> { + const cwd = this.allowedDirectories[0] + const conversationId = options.conversationId + + if (!conversationId) { + throw new Error('Background execution requires a conversation ID') + } + + if (this.commandPermissionHandler) { + const permissionCheck = this.commandPermissionHandler.checkPermission(conversationId, command) + if (!permissionCheck.allowed) { + const commandInfo = this.commandPermissionHandler.buildCommandInfo(command) + throw new CommandPermissionRequiredError( + 'components.messageBlockPermissionRequest.description.commandWithRisk', + { + toolName: 'execute_command', + serverName: 'agent-filesystem', + permissionType: 'command', + description: 'Execute command requires approval.', + command, + commandSignature: commandInfo.signature, + commandInfo, + conversationId + } + ) + } + } + + // Start background session + const result = await backgroundExecSessionManager.start(conversationId, command, cwd, { + timeout: timeout ?? COMMAND_DEFAULT_TIMEOUT_MS + }) + + // Emit initial terminal snippet + await this.emitTerminalSnippet(conversationId, { + id: result.sessionId, + status: 'running', + command, + cwd, + output: '', + truncated: false, + exitCode: null, + startedAt: Date.now(), + timestamp: Date.now() + }) + + return { status: 'running', sessionId: result.sessionId } + } + private async emitTerminalSnippet( conversationId: string | undefined, snippet: { @@ -306,4 +371,41 @@ export class AgentBashHandler { logger.warn('[AgentBashHandler] Failed to emit terminal snippet', { error }) } } + + /** + * Pre-check command permission without executing + * Returns permission info if permission is needed, null if no permission needed + */ + checkCommandPermission( + command: string, + conversationId?: string + ): { + needsPermission: boolean + description?: string + signature?: string + commandInfo?: { + command: string + riskLevel: 'low' | 'medium' | 'high' | 'critical' + suggestion: string + signature?: string + baseCommand?: string + } + } { + if (!this.commandPermissionHandler) { + return { needsPermission: false } + } + + const permissionCheck = this.commandPermissionHandler.checkPermission(conversationId, command) + if (permissionCheck.allowed) { + return { needsPermission: false } + } + + const commandInfo = this.commandPermissionHandler.buildCommandInfo(command) + return { + needsPermission: true, + description: `Command "${command}" requires permission`, + signature: commandInfo.signature, + commandInfo + } + } } diff --git a/src/main/presenter/agentPresenter/acp/agentFileSystemHandler.ts b/src/main/presenter/agentPresenter/acp/agentFileSystemHandler.ts index 0623fca40..5554edc7c 100644 --- a/src/main/presenter/agentPresenter/acp/agentFileSystemHandler.ts +++ b/src/main/presenter/agentPresenter/acp/agentFileSystemHandler.ts @@ -106,6 +106,12 @@ const TextReplaceArgsSchema = z.object({ dryRun: z.boolean().default(false) }) +const EditFileArgsSchema = z.object({ + path: z.string(), + oldText: z.string().max(10000), + newText: z.string().max(10000) +}) + const DirectoryTreeArgsSchema = z.object({ path: z.string(), depth: z.number().int().min(0).max(3).default(1) @@ -1090,6 +1096,51 @@ export class AgentFileSystemHandler { return JSON.stringify(response) } + async editFile(args: unknown, baseDirectory?: string): Promise { + const parsed = EditFileArgsSchema.safeParse(args) + if (!parsed.success) { + throw new Error(`Invalid arguments: ${parsed.error}`) + } + + const { path: filePath, oldText, newText } = parsed.data + const validPath = await this.validatePath(filePath, baseDirectory) + + const content = await fs.readFile(validPath, 'utf-8') + const normalizedOldText = this.normalizeLineEndings(oldText) + const normalizedNewText = this.normalizeLineEndings(newText) + const normalizedContent = this.normalizeLineEndings(content) + + if (!normalizedContent.includes(normalizedOldText)) { + throw new Error( + `Cannot find the specified text to replace. The exact text was not found in the file.` + ) + } + + let replacementCount = 0 + + const modifiedContent = normalizedContent.replaceAll(normalizedOldText, () => { + replacementCount++ + return normalizedNewText + }) + + await fs.writeFile(validPath, modifiedContent, 'utf-8') + + const { originalCode, updatedCode } = this.buildTruncatedDiff( + normalizedContent, + modifiedContent, + 3 + ) + const language = getLanguageFromFilename(validPath) + const response: DiffToolResponse = { + success: true, + originalCode, + updatedCode, + language, + replacements: replacementCount + } + return JSON.stringify(response) + } + async directoryTree(args: unknown, baseDirectory?: string): Promise { const parsed = DirectoryTreeArgsSchema.safeParse(args) if (!parsed.success) { diff --git a/src/main/presenter/agentPresenter/acp/agentToolManager.ts b/src/main/presenter/agentPresenter/acp/agentToolManager.ts index c1de4c026..8d7cdcae3 100644 --- a/src/main/presenter/agentPresenter/acp/agentToolManager.ts +++ b/src/main/presenter/agentPresenter/acp/agentToolManager.ts @@ -66,6 +66,7 @@ export class AgentToolManager { private readonly configPresenter: IConfigPresenter private skillTools: SkillTools | null = null private chatSettingsHandler: ChatSettingsToolHandler | null = null + private readonly fileSystemSchemas = { read_file: z.object({ paths: z.array(z.string()).min(1), @@ -186,6 +187,15 @@ export class AgentToolManager { dryRun: z.boolean().default(false), base_directory: z.string().optional().describe('Base directory for resolving relative paths.') }), + edit_file: z.object({ + path: z.string().describe('Path to the file to edit'), + oldText: z + .string() + .max(10000) + .describe('The exact text to find and replace (case-sensitive)'), + newText: z.string().max(10000).describe('The replacement text'), + base_directory: z.string().optional().describe('Base directory for resolving relative paths.') + }), directory_tree: z.object({ path: z.string(), depth: z @@ -209,7 +219,46 @@ export class AgentToolManager { .max(600000) .optional() .describe('Optional timeout in milliseconds'), - description: z.string().min(5).max(100).describe('Brief description of what the command does') + description: z + .string() + .min(5) + .max(100) + .describe( + 'Brief description of what the command does (e.g., "Install dependencies", "Start dev server")' + ), + background: z + .boolean() + .optional() + .describe( + 'Run the command in the background (recommended for commands taking >10s). Returns immediately with sessionId for use with process tool.' + ), + yieldMs: z + .number() + .min(100) + .optional() + .describe( + 'Maximum time in milliseconds to wait for command output in foreground mode (default 120s). Ignored when background is true.' + ) + }), + process: z.object({ + action: z + .enum(['list', 'poll', 'log', 'write', 'kill', 'clear', 'remove']) + .describe( + 'Action to perform: list (all sessions), poll (recent output), log (full output with pagination), write (send to stdin), kill (terminate), clear (empty buffer), remove (cleanup)' + ), + sessionId: z + .string() + .optional() + .describe('Session ID (required for most actions except list)'), + offset: z.number().int().min(0).optional().describe('Starting offset for log action'), + limit: z + .number() + .int() + .min(1) + .optional() + .describe('Maximum characters to return for log action'), + data: z.string().optional().describe('Data to write to stdin (write action only)'), + eof: z.boolean().optional().describe('Send EOF after writing data (write action only)') }) } @@ -350,6 +399,11 @@ export class AgentToolManager { } } + // Route to process tool + if (this.isProcessTool(toolName)) { + return await this.callProcessTool(toolName, args, conversationId) + } + // Route to FileSystem tools if (this.isFileSystemTool(toolName)) { if (!this.fileSystemHandler) { @@ -410,7 +464,7 @@ export class AgentToolManager { private getFileSystemToolDefinitions(): MCPToolDefinition[] { const schemas = this.fileSystemSchemas - return [ + const defs: MCPToolDefinition[] = [ { type: 'function', function: { @@ -608,12 +662,30 @@ export class AgentToolManager { description: 'Agent FileSystem tools' } }, + { + type: 'function', + function: { + name: 'edit_file', + description: + 'Make precise edits to files by replacing exact text strings. Use this for simple text replacements when you know the exact content to replace. For regex or complex operations, use edit_text instead.', + parameters: zodToJsonSchema(schemas.edit_file) as { + type: string + properties: Record + required?: string[] + } + }, + server: { + name: 'agent-filesystem', + icons: '📁', + description: 'Agent FileSystem tools' + } + }, { type: 'function', function: { name: 'execute_command', description: - 'Execute a shell command in the workspace directory. Prefer file system tools for read/write/search operations.', + 'Execute a shell command in the workspace directory. For long-running commands (builds, tests, servers, installations), use background: true to run asynchronously and get a session ID. Then use the process tool to poll output, send input, or manage the session. For quick commands that complete within seconds, run without background mode.', parameters: zodToJsonSchema(schemas.execute_command) as { type: string properties: Record @@ -625,8 +697,27 @@ export class AgentToolManager { icons: '📁', description: 'Agent FileSystem tools' } + }, + { + type: 'function', + function: { + name: 'process', + description: + 'Manage background exec sessions created by execute_command with background: true. Use poll to check output and status, log to get full output with pagination, write to send input to stdin, kill to terminate, and remove to clean up completed sessions.', + parameters: zodToJsonSchema(schemas.process) as { + type: string + properties: Record + required?: string[] + } + }, + server: { + name: 'agent-filesystem', + icons: '⚙️', + description: 'Agent FileSystem tools' + } } ] + return defs } private getQuestionToolDefinitions(): MCPToolDefinition[] { @@ -665,16 +756,118 @@ export class AgentToolManager { 'get_file_info', 'grep_search', 'text_replace', - 'execute_command' + 'edit_file', + 'execute_command', + 'process' ] return filesystemTools.includes(toolName) } + private isProcessTool(toolName: string): boolean { + return toolName === 'process' + } + + private async callProcessTool( + _toolName: string, + args: Record, + conversationId?: string + ): Promise { + if (!conversationId) { + throw new Error('process tool requires a conversation ID') + } + + const { backgroundExecSessionManager } = await import('./backgroundExecSessionManager') + + const validationResult = this.fileSystemSchemas.process.safeParse(args) + if (!validationResult.success) { + throw new Error(`Invalid arguments for process: ${validationResult.error.message}`) + } + + const { action, sessionId, offset, limit, data, eof } = validationResult.data + + switch (action) { + case 'list': { + const sessions = backgroundExecSessionManager.list(conversationId) + return { + content: JSON.stringify({ status: 'ok', sessions }, null, 2) + } + } + + case 'poll': { + if (!sessionId) { + throw new Error('sessionId is required for poll action') + } + const result = backgroundExecSessionManager.poll(conversationId, sessionId) + return { + content: JSON.stringify(result, null, 2) + } + } + + case 'log': { + if (!sessionId) { + throw new Error('sessionId is required for log action') + } + const result = backgroundExecSessionManager.log(conversationId, sessionId, offset, limit) + return { + content: JSON.stringify(result, null, 2) + } + } + + case 'write': { + if (!sessionId) { + throw new Error('sessionId is required for write action') + } + backgroundExecSessionManager.write(conversationId, sessionId, data ?? '', eof) + return { + content: JSON.stringify({ status: 'ok', sessionId }) + } + } + + case 'kill': { + if (!sessionId) { + throw new Error('sessionId is required for kill action') + } + await backgroundExecSessionManager.kill(conversationId, sessionId) + return { + content: JSON.stringify({ status: 'ok', sessionId }) + } + } + + case 'clear': { + if (!sessionId) { + throw new Error('sessionId is required for clear action') + } + backgroundExecSessionManager.clear(conversationId, sessionId) + return { + content: JSON.stringify({ status: 'ok', sessionId }) + } + } + + case 'remove': { + if (!sessionId) { + throw new Error('sessionId is required for remove action') + } + await backgroundExecSessionManager.remove(conversationId, sessionId) + return { + content: JSON.stringify({ status: 'ok', sessionId }) + } + } + + default: + throw new Error(`Unknown process action: ${action}`) + } + } + private async callFileSystemTool( toolName: string, args: Record, conversationId?: string ): Promise { + // Handle process tool separately + if (this.isProcessTool(toolName)) { + return this.callProcessTool(toolName, args, conversationId) + } + if (!this.fileSystemHandler) { throw new Error('FileSystem handler not initialized') } @@ -684,6 +877,17 @@ export class AgentToolManager { throw new Error(`No schema found for FileSystem tool: ${toolName}`) } + // Normalize parameter aliases for edit_file tool + if (toolName === 'edit_file') { + args = { + ...args, + path: args.path ?? args.file_path, + oldText: args.oldText ?? args.old_string, + newText: args.newText ?? args.new_string, + base_directory: args.base_directory + } + } + const validationResult = schema.safeParse(args) if (!validationResult.success) { throw new Error(`Invalid arguments for ${toolName}: ${validationResult.error.message}`) @@ -773,12 +977,25 @@ export class AgentToolManager { conversationId ) return { content: await fileSystemHandler.textReplace(parsedArgs, baseDirectory) } + case 'edit_file': + this.assertWritePermission( + toolName, + parsedArgs, + baseDirectory, + fileSystemHandler, + conversationId + ) + return { content: await fileSystemHandler.editFile(parsedArgs, baseDirectory) } case 'execute_command': if (!this.bashHandler) { throw new Error('Bash handler not initialized for execute_command tool') } + const commandResult = await this.bashHandler.executeCommand(parsedArgs, { + conversationId + }) return { - content: await this.bashHandler.executeCommand(parsedArgs, { conversationId }) + content: + typeof commandResult === 'string' ? commandResult : JSON.stringify(commandResult) } default: throw new Error(`Unknown FileSystem tool: ${toolName}`) @@ -992,6 +1209,152 @@ export class AgentToolManager { return toolName === 'skill_list' || toolName === 'skill_control' } + /** + * Pre-check tool permissions for agent tools + * Returns permission request info if permission is needed, null if no permission needed + */ + async preCheckToolPermission( + toolName: string, + args: Record, + conversationId?: string + ): Promise<{ + needsPermission: true + toolName: string + serverName: string + permissionType: 'read' | 'write' | 'all' | 'command' + description: string + paths?: string[] + command?: string + commandSignature?: string + commandInfo?: { + command: string + riskLevel: 'low' | 'medium' | 'high' | 'critical' + suggestion: string + signature?: string + baseCommand?: string + } + conversationId?: string + } | null> { + // Only file system write operations and command execution need pre-check + const writeTools = [ + 'write_file', + 'create_directory', + 'move_files', + 'edit_text', + 'text_replace', + 'edit_file' + ] + const readTools = [ + 'read_file', + 'list_directory', + 'directory_tree', + 'glob_search', + 'grep_search' + ] + + // Check for file system write operations + if (this.isFileSystemTool(toolName)) { + if (!this.fileSystemHandler) { + throw new Error('FileSystem handler not initialized') + } + + // Handle command tools separately (they use command permission service) + if (toolName === 'execute_command') { + if (!this.bashHandler) { + return null + } + + const command = (args.command as string) || '' + if (!command) { + return null + } + + // Use bash handler's checkCommandPermission if available + if (this.bashHandler.checkCommandPermission) { + const result = await this.bashHandler.checkCommandPermission(command, conversationId) + if (result.needsPermission) { + return { + needsPermission: true, + toolName, + serverName: 'agent-filesystem', + permissionType: 'command', + description: result.description || `Command "${command}" requires permission`, + command, + commandSignature: result.signature, + commandInfo: result.commandInfo, + conversationId + } + } + } + return null + } + + // Handle process tool + if (toolName === 'process') { + return null + } + + // For file system operations, check if write permission is needed + const isWriteOperation = writeTools.includes(toolName) + const isReadOperation = readTools.includes(toolName) + + if (!isWriteOperation && !isReadOperation) { + return null + } + + // Get workdir and allowed directories + let dynamicWorkdir: string | null = null + if (conversationId) { + try { + dynamicWorkdir = await this.getWorkdirForConversation(conversationId) + } catch (error) { + logger.warn('[AgentToolManager] Failed to get workdir for permission check:', { + conversationId, + error + }) + } + } + + const workspaceRoot = + dynamicWorkdir ?? this.agentWorkspacePath ?? this.getDefaultAgentWorkspacePath() + const allowedDirectories = this.buildAllowedDirectories(workspaceRoot, conversationId) + const fileSystemHandler = new AgentFileSystemHandler(allowedDirectories, { conversationId }) + + // Collect target paths + const targets = this.collectWriteTargets(toolName, args) + if (targets.length === 0 && isWriteOperation) { + // Check for path in read operations too + const pathArg = (args.path as string) || (args.paths as string[])?.[0] + if (pathArg) { + targets.push(pathArg) + } + } + + // Check each path + const denied: string[] = [] + for (const target of targets) { + const resolved = fileSystemHandler.resolvePath(target, undefined) + if (!fileSystemHandler.isPathAllowedAbsolute(resolved)) { + denied.push(target) + } + } + + if (denied.length > 0) { + return { + needsPermission: true, + toolName, + serverName: 'agent-filesystem', + permissionType: isWriteOperation ? 'write' : 'read', + description: `${isWriteOperation ? 'Write' : 'Read'} access requires approval for: ${denied.join(', ')}`, + paths: denied, + conversationId + } + } + } + + return null + } + private isChatSettingsTool(toolName: string): boolean { return ( toolName === CHAT_SETTINGS_TOOL_NAMES.toggle || diff --git a/src/main/presenter/agentPresenter/acp/backgroundExecSessionManager.ts b/src/main/presenter/agentPresenter/acp/backgroundExecSessionManager.ts new file mode 100644 index 000000000..527196bbb --- /dev/null +++ b/src/main/presenter/agentPresenter/acp/backgroundExecSessionManager.ts @@ -0,0 +1,725 @@ +import { spawn, type ChildProcess } from 'child_process' +import fs from 'fs' +import path from 'path' +import { nanoid } from 'nanoid' +import logger from '@shared/logger' +import { getShellEnvironment, getUserShell } from './shellEnvHelper' +import { resolveSessionDir } from '../../sessionPresenter/sessionPaths' + +// Configuration with environment variable support +const getConfig = () => ({ + backgroundMs: parseInt(process.env.PI_BASH_YIELD_MS || '10000', 10), + timeoutSec: parseInt(process.env.PI_BASH_TIMEOUT_SEC || '1800', 10), + cleanupMs: parseInt(process.env.PI_BASH_JOB_TTL_MS || '1800000', 10), + maxOutputChars: + parseInt( + process.env.OPENCLAW_BASH_PENDING_MAX_OUTPUT_CHARS || + process.env.PI_BASH_MAX_OUTPUT_CHARS || + '500', + 10 + ) || 500, + offloadThresholdChars: 10000 // Offload to file when output exceeds this +}) + +export interface SessionMeta { + sessionId: string + command: string + status: 'running' | 'done' | 'error' | 'killed' + createdAt: number + lastAccessedAt: number + pid?: number + exitCode?: number + outputLength: number + offloaded: boolean +} + +interface BackgroundSession { + sessionId: string + conversationId: string + command: string + child: ChildProcess + status: 'running' | 'done' | 'error' | 'killed' + exitCode?: number + errorMessage?: string + createdAt: number + lastAccessedAt: number + outputBuffer: string + outputFilePath: string | null + outputWriteQueue: Promise + totalOutputLength: number + stdoutEof: boolean + stderrEof: boolean + killTimeoutId?: NodeJS.Timeout +} + +interface StartSessionResult { + sessionId: string + status: 'running' +} + +interface PollResult { + status: 'running' | 'done' | 'error' | 'killed' + output: string + exitCode?: number + offloaded?: boolean + outputFilePath?: string +} + +interface LogResult { + status: 'running' | 'done' | 'error' | 'killed' + output: string + totalLength: number + exitCode?: number + offloaded?: boolean + outputFilePath?: string +} + +export class BackgroundExecSessionManager { + private sessions = new Map>() + private cleanupIntervalId?: NodeJS.Timeout + + constructor() { + this.startCleanupTimer() + } + + /** + * Start a new background exec session + */ + async start( + conversationId: string, + command: string, + cwd: string, + options?: { + timeout?: number + env?: Record + } + ): Promise { + const config = getConfig() + const sessionId = `bg_${nanoid(12)}` + const { shell, args } = getUserShell() + const shellEnv = await getShellEnvironment() + + // Ensure session directory exists for offload + const sessionDir = resolveSessionDir(conversationId) + if (sessionDir) { + fs.mkdirSync(sessionDir, { recursive: true }) + } + + const outputFilePath = sessionDir ? path.join(sessionDir, `bgexec_${sessionId}.log`) : null + + const child = spawn(shell, [...args, command], { + cwd, + env: { + ...process.env, + ...shellEnv, + ...options?.env + }, + stdio: ['pipe', 'pipe', 'pipe'] + }) + + const now = Date.now() + const session: BackgroundSession = { + sessionId, + conversationId, + command, + child, + status: 'running', + createdAt: now, + lastAccessedAt: now, + outputBuffer: '', + outputFilePath, + outputWriteQueue: Promise.resolve(), + totalOutputLength: 0, + stdoutEof: false, + stderrEof: false + } + + // Set up output handling + this.setupOutputHandling(session, config) + + // Set up process exit handling + this.setupExitHandling(session, config) + + // Set up timeout + const timeout = options?.timeout ?? config.timeoutSec * 1000 + if (timeout > 0) { + session.killTimeoutId = setTimeout(() => { + this.killInternal(session, 'timeout') + }, timeout) + } + + // Store session + if (!this.sessions.has(conversationId)) { + this.sessions.set(conversationId, new Map()) + } + this.sessions.get(conversationId)!.set(sessionId, session) + + logger.info(`[BackgroundExec] Started session ${sessionId} for conversation ${conversationId}`) + + return { sessionId, status: 'running' } + } + + /** + * List all sessions for a conversation + */ + list(conversationId: string): SessionMeta[] { + const conversationSessions = this.sessions.get(conversationId) + if (!conversationSessions) return [] + + return Array.from(conversationSessions.values()).map((session) => ({ + sessionId: session.sessionId, + command: session.command, + status: session.status, + createdAt: session.createdAt, + lastAccessedAt: session.lastAccessedAt, + pid: session.child.pid, + exitCode: session.exitCode, + outputLength: session.totalOutputLength, + offloaded: + session.outputFilePath !== null && + session.totalOutputLength > getConfig().offloadThresholdChars + })) + } + + /** + * Poll for new output (returns recent output only) + */ + poll(conversationId: string, sessionId: string): PollResult { + const session = this.getSession(conversationId, sessionId) + session.lastAccessedAt = Date.now() + + const config = getConfig() + const isOffloaded = + session.outputFilePath !== null && session.totalOutputLength > config.offloadThresholdChars + + if (isOffloaded && session.outputFilePath) { + // Return only last N characters from file + const output = this.readLastCharsFromFile(session.outputFilePath, config.maxOutputChars) + return { + status: session.status, + output, + exitCode: session.exitCode, + offloaded: true, + outputFilePath: session.outputFilePath + } + } + + // Return recent output from buffer + const output = this.getRecentOutput(session.outputBuffer, config.maxOutputChars) + return { + status: session.status, + output, + exitCode: session.exitCode, + offloaded: false + } + } + + /** + * Get full output log with pagination + */ + log(conversationId: string, sessionId: string, offset = 0, limit = 1000): LogResult { + const session = this.getSession(conversationId, sessionId) + session.lastAccessedAt = Date.now() + + const config = getConfig() + const isOffloaded = + session.outputFilePath !== null && session.totalOutputLength > config.offloadThresholdChars + + let output: string + if (isOffloaded && session.outputFilePath) { + output = this.readFromFile(session.outputFilePath, offset, limit) + } else { + output = session.outputBuffer.slice(offset, offset + limit) + } + + return { + status: session.status, + output, + totalLength: session.totalOutputLength, + exitCode: session.exitCode, + offloaded: isOffloaded, + outputFilePath: session.outputFilePath || undefined + } + } + + /** + * Write data to session stdin + */ + write(conversationId: string, sessionId: string, data: string, eof = false): void { + const session = this.getSession(conversationId, sessionId) + + if (session.status !== 'running') { + throw new Error(`Session ${sessionId} is not running`) + } + + if (!session.child.stdin || session.child.stdin.destroyed) { + throw new Error(`Session ${sessionId} stdin is not available`) + } + + session.child.stdin.write(data) + if (eof) { + session.child.stdin.end() + } + + session.lastAccessedAt = Date.now() + } + + /** + * Kill a running session + */ + async kill(conversationId: string, sessionId: string): Promise { + const session = this.getSession(conversationId, sessionId) + await this.killInternal(session, 'user') + } + + /** + * Clear output buffer/file + */ + clear(conversationId: string, sessionId: string): void { + const session = this.getSession(conversationId, sessionId) + + session.outputBuffer = '' + session.totalOutputLength = 0 + + if (session.outputFilePath) { + this.queueOutputWrite(session, '', 'truncate') + } + + session.lastAccessedAt = Date.now() + } + + /** + * Remove a session completely + */ + async remove(conversationId: string, sessionId: string): Promise { + const conversationSessions = this.sessions.get(conversationId) + if (!conversationSessions) { + throw new Error(`No sessions found for conversation ${conversationId}`) + } + + const session = conversationSessions.get(sessionId) + if (!session) { + throw new Error(`Session ${sessionId} not found`) + } + + // Kill if still running + if (session.status === 'running') { + await this.killInternal(session, 'remove') + } + + // Ensure queued writes are completed before deleting files. + await session.outputWriteQueue.catch((error) => { + logger.warn(`[BackgroundExec] Failed while draining output write queue:`, error) + }) + + // Clean up output file + if (session.outputFilePath && fs.existsSync(session.outputFilePath)) { + try { + fs.unlinkSync(session.outputFilePath) + } catch (error) { + logger.warn( + `[BackgroundExec] Failed to remove output file ${session.outputFilePath}:`, + error + ) + } + } + + // Clear timeout + if (session.killTimeoutId) { + clearTimeout(session.killTimeoutId) + } + + // Remove from map + conversationSessions.delete(sessionId) + if (conversationSessions.size === 0) { + this.sessions.delete(conversationId) + } + + logger.info(`[BackgroundExec] Removed session ${sessionId}`) + } + + /** + * Clean up all sessions for a conversation + */ + async cleanupConversation(conversationId: string): Promise { + const conversationSessions = this.sessions.get(conversationId) + if (!conversationSessions) return + + const sessionIds = Array.from(conversationSessions.keys()) + await Promise.all(sessionIds.map((id) => this.remove(conversationId, id).catch(() => {}))) + } + + /** + * Shutdown all sessions + */ + async shutdown(): Promise { + if (this.cleanupIntervalId) { + clearInterval(this.cleanupIntervalId) + } + + const allSessions: Array<{ conversationId: string; sessionId: string }> = [] + for (const [conversationId, sessions] of this.sessions) { + for (const sessionId of sessions.keys()) { + allSessions.push({ conversationId, sessionId }) + } + } + + await Promise.all( + allSessions.map(({ conversationId, sessionId }) => + this.remove(conversationId, sessionId).catch(() => {}) + ) + ) + } + + // Private methods + + private getSession(conversationId: string, sessionId: string): BackgroundSession { + const conversationSessions = this.sessions.get(conversationId) + if (!conversationSessions) { + throw new Error(`No sessions found for conversation ${conversationId}`) + } + + const session = conversationSessions.get(sessionId) + if (!session) { + throw new Error(`Session ${sessionId} not found`) + } + + return session + } + + private setupOutputHandling( + session: BackgroundSession, + config: ReturnType + ): void { + const stdoutHandler = (data: Buffer) => { + this.appendOutput(session, data.toString('utf-8'), config) + } + + const stderrHandler = (data: Buffer) => { + this.appendOutput(session, data.toString('utf-8'), config) + } + + session.child.stdout?.on('data', stdoutHandler) + session.child.stderr?.on('data', stderrHandler) + + session.child.stdout?.on('end', () => { + session.stdoutEof = true + }) + + session.child.stderr?.on('end', () => { + session.stderrEof = true + }) + } + + private appendOutput( + session: BackgroundSession, + data: string, + config: ReturnType + ): void { + session.totalOutputLength += data.length + + const shouldOffload = + session.outputFilePath !== null && session.totalOutputLength > config.offloadThresholdChars + + if (shouldOffload) { + const chunk = session.outputBuffer + data + session.outputBuffer = '' + this.queueOutputWrite(session, chunk, 'append') + } else { + // Keep in buffer + session.outputBuffer += data + } + } + + private setupExitHandling( + session: BackgroundSession, + config: ReturnType + ): void { + session.child.on('exit', (code, signal) => { + if (session.killTimeoutId) { + clearTimeout(session.killTimeoutId) + } + + if (signal === 'SIGTERM' || signal === 'SIGKILL') { + session.status = 'killed' + } else if (code !== 0 && code !== null) { + session.status = 'error' + } else { + session.status = 'done' + } + + session.exitCode = code ?? undefined + + // Flush any remaining output + if (session.outputFilePath && session.totalOutputLength > config.offloadThresholdChars) { + try { + const remainingStdout = session.child.stdout?.read?.() + const remainingStderr = session.child.stderr?.read?.() + if (remainingStdout) { + this.appendOutput(session, remainingStdout.toString('utf-8'), config) + } + if (remainingStderr) { + this.appendOutput(session, remainingStderr.toString('utf-8'), config) + } + } catch (error) { + logger.warn(`[BackgroundExec] Failed to flush remaining output:`, error) + } + } + + logger.info( + `[BackgroundExec] Session ${session.sessionId} exited with code ${code}, signal ${signal}` + ) + }) + + session.child.on('error', (error) => { + session.status = 'error' + session.errorMessage = error.message + logger.error(`[BackgroundExec] Session ${session.sessionId} error:`, error) + }) + } + + private async killInternal(session: BackgroundSession, reason: string): Promise { + if (session.status !== 'running') return + + logger.info(`[BackgroundExec] Killing session ${session.sessionId} (reason: ${reason})`) + + // Clear timeout + if (session.killTimeoutId) { + clearTimeout(session.killTimeoutId) + } + + // Try graceful kill first + const gracefulKill = new Promise((resolve) => { + const timeout = setTimeout(() => { + resolve() // Timeout, will force kill + }, 2000) + + session.child.once('exit', () => { + clearTimeout(timeout) + resolve() + }) + + try { + session.child.kill('SIGTERM') + } catch { + resolve() + } + }) + + await gracefulKill + + // Force kill if still running + if (session.status === 'running') { + try { + session.child.kill('SIGKILL') + } catch (error) { + logger.warn(`[BackgroundExec] Failed to force kill session ${session.sessionId}:`, error) + } + } + + session.status = 'killed' + } + + private getRecentOutput(buffer: string, maxChars: number): string { + if (buffer.length <= maxChars) return buffer + return buffer.slice(-maxChars) + } + + private readLastCharsFromFile(filePath: string, maxChars: number): string { + try { + const stats = fs.statSync(filePath) + const fileSize = stats.size + + // Estimate bytes needed (assuming UTF-8, worst case 4 bytes per char) + const bytesToRead = Math.min(maxChars * 4, fileSize) + const startPosition = Math.max(0, fileSize - bytesToRead) + + const fd = fs.openSync(filePath, 'r') + try { + const buffer = Buffer.alloc(bytesToRead) + fs.readSync(fd, buffer, 0, bytesToRead, startPosition) + const content = buffer.toString('utf-8') + // If we read from middle of file, find first newline to start clean + if (startPosition > 0 && content.length > 0) { + const firstNewline = content.indexOf('\n') + if (firstNewline > 0) { + return content.slice(firstNewline + 1) + } + } + return content + } finally { + fs.closeSync(fd) + } + } catch (error) { + logger.warn(`[BackgroundExec] Failed to read from output file:`, error) + return '' + } + } + + private readFromFile(filePath: string, offset: number, limit: number): string { + try { + const safeOffset = Math.max(0, Math.floor(offset)) + const safeLimit = Math.max(0, Math.floor(limit)) + if (safeLimit === 0) { + return '' + } + + const fd = fs.openSync(filePath, 'r') + try { + const fileSize = fs.fstatSync(fd).size + if (fileSize === 0) { + return '' + } + + const { startByte, endByte } = this.resolveUtf8ByteRange( + fd, + fileSize, + safeOffset, + safeLimit + ) + if (endByte <= startByte) { + return '' + } + + const bytesToRead = endByte - startByte + const buffer = Buffer.alloc(bytesToRead) + const bytesRead = fs.readSync(fd, buffer, 0, bytesToRead, startByte) + if (bytesRead <= 0) { + return '' + } + return buffer.subarray(0, bytesRead).toString('utf-8') + } finally { + fs.closeSync(fd) + } + } catch (error) { + logger.warn(`[BackgroundExec] Failed to read from output file:`, error) + return '' + } + } + + private queueOutputWrite( + session: BackgroundSession, + data: string, + mode: 'append' | 'truncate' + ): void { + if (!session.outputFilePath) { + if (mode === 'append' && data) { + session.outputBuffer += data + } + return + } + + const outputFilePath = session.outputFilePath + session.outputWriteQueue = session.outputWriteQueue + .then(async () => { + if (mode === 'truncate') { + await fs.promises.writeFile(outputFilePath, data, 'utf-8') + return + } + if (data.length === 0) { + return + } + await fs.promises.appendFile(outputFilePath, data, 'utf-8') + }) + .catch((error) => { + logger.warn(`[BackgroundExec] Failed to write output file (${mode}):`, error) + if (mode === 'append' && data.length > 0) { + session.outputBuffer += data + } + }) + } + + private resolveUtf8ByteRange( + fd: number, + fileSize: number, + offset: number, + limit: number + ): { startByte: number; endByte: number } { + const targetStart = offset + const targetEnd = offset + limit + let startByte = targetStart === 0 ? 0 : -1 + let endByte = -1 + let charCount = 0 + let currentBytePos = 0 + + const chunkSize = 64 * 1024 + const chunkBuffer = Buffer.alloc(chunkSize) + + while (currentBytePos < fileSize && endByte === -1) { + const bytesToRead = Math.min(chunkSize, fileSize - currentBytePos) + const bytesRead = fs.readSync(fd, chunkBuffer, 0, bytesToRead, currentBytePos) + if (bytesRead <= 0) { + break + } + + for (let i = 0; i < bytesRead; i++) { + const byte = chunkBuffer[i] + // UTF-8 character starts at non-continuation byte. + if ((byte & 0xc0) !== 0x80) { + const absoluteBytePos = currentBytePos + i + if (startByte === -1 && charCount === targetStart) { + startByte = absoluteBytePos + } + if (charCount === targetEnd) { + endByte = absoluteBytePos + break + } + charCount++ + } + } + + currentBytePos += bytesRead + } + + if (startByte === -1) { + startByte = fileSize + } + if (endByte === -1) { + endByte = fileSize + } + if (endByte < startByte) { + endByte = startByte + } + + return { startByte, endByte } + } + + private startCleanupTimer(): void { + // Run cleanup every 5 minutes + this.cleanupIntervalId = setInterval( + () => { + this.runCleanup() + }, + 5 * 60 * 1000 + ) + } + + private runCleanup(): void { + const config = getConfig() + const now = Date.now() + const expiredSessions: Array<{ conversationId: string; sessionId: string }> = [] + + for (const [conversationId, sessions] of this.sessions) { + for (const [sessionId, session] of sessions) { + // Clean up sessions that have been inactive for cleanupMs + if (now - session.lastAccessedAt > config.cleanupMs) { + expiredSessions.push({ conversationId, sessionId }) + } + // Also clean up completed sessions after a shorter period (5 minutes) + else if (session.status !== 'running' && now - session.lastAccessedAt > 5 * 60 * 1000) { + expiredSessions.push({ conversationId, sessionId }) + } + } + } + + for (const { conversationId, sessionId } of expiredSessions) { + logger.info(`[BackgroundExec] Auto-removing expired session ${sessionId}`) + this.remove(conversationId, sessionId).catch((error) => { + logger.warn(`[BackgroundExec] Failed to remove expired session:`, error) + }) + } + } +} + +// Singleton instance +export const backgroundExecSessionManager = new BackgroundExecSessionManager() diff --git a/src/main/presenter/agentPresenter/acp/index.ts b/src/main/presenter/agentPresenter/acp/index.ts index 388ae8c8c..2d0a24489 100644 --- a/src/main/presenter/agentPresenter/acp/index.ts +++ b/src/main/presenter/agentPresenter/acp/index.ts @@ -16,6 +16,11 @@ export { AcpTerminalManager } from './acpTerminalManager' export { AgentFileSystemHandler } from './agentFileSystemHandler' export { AgentToolManager, type AgentToolCallResult } from './agentToolManager' export { AgentBashHandler } from './agentBashHandler' +export { + BackgroundExecSessionManager, + backgroundExecSessionManager, + type SessionMeta +} from './backgroundExecSessionManager' export { registerCommandProcess, unregisterCommandProcess, diff --git a/src/main/presenter/agentPresenter/index.ts b/src/main/presenter/agentPresenter/index.ts index 2038a046c..f966104d6 100644 --- a/src/main/presenter/agentPresenter/index.ts +++ b/src/main/presenter/agentPresenter/index.ts @@ -186,7 +186,8 @@ export class AgentPresenter implements IAgentPresenter { this.trackGeneratingMessage(assistantMessage, agentId, tabId) await this.updateConversationAfterUserMessage(agentId) - await this.sessionManager.startLoop(agentId, assistantMessage.id) + // Normal flow: skip lock acquisition (lock is only for permission resume) + await this.sessionManager.startLoop(agentId, assistantMessage.id, { skipLockAcquisition: true }) void this.streamGenerationHandler .startStreamCompletion(agentId, assistantMessage.id, selectedVariantsMap) @@ -216,7 +217,8 @@ export class AgentPresenter implements IAgentPresenter { this.trackGeneratingMessage(assistantMessage, agentId) await this.updateConversationAfterUserMessage(agentId) - await this.sessionManager.startLoop(agentId, assistantMessage.id) + // Normal flow: skip lock acquisition (lock is only for permission resume) + await this.sessionManager.startLoop(agentId, assistantMessage.id, { skipLockAcquisition: true }) void this.streamGenerationHandler .continueStreamCompletion(agentId, messageId, selectedVariantsMap) @@ -266,7 +268,10 @@ export class AgentPresenter implements IAgentPresenter { )) as AssistantMessage this.trackGeneratingMessage(assistantMessage, message.conversationId) - await this.sessionManager.startLoop(message.conversationId, assistantMessage.id) + // Normal flow: skip lock acquisition (lock is only for permission resume) + await this.sessionManager.startLoop(message.conversationId, assistantMessage.id, { + skipLockAcquisition: true + }) void this.streamGenerationHandler .startStreamCompletion(message.conversationId, messageId, selectedVariantsMap) diff --git a/src/main/presenter/agentPresenter/loop/agentLoopHandler.ts b/src/main/presenter/agentPresenter/loop/agentLoopHandler.ts index 65dcc51c0..c44a77df4 100644 --- a/src/main/presenter/agentPresenter/loop/agentLoopHandler.ts +++ b/src/main/presenter/agentPresenter/loop/agentLoopHandler.ts @@ -54,6 +54,9 @@ export class AgentLoopHandler { callTool: async (request: MCPToolCall) => { return await this.getToolPresenter().callTool(request) }, + preCheckToolPermission: async (request: MCPToolCall) => { + return await this.getToolPresenter().preCheckToolPermission(request) + }, onToolCallFinished: ({ toolServerName, conversationId }) => { if (toolServerName !== 'agent-filesystem') return this.notifyWorkspaceFilesChanged(conversationId) diff --git a/src/main/presenter/agentPresenter/loop/toolCallHandler.ts b/src/main/presenter/agentPresenter/loop/toolCallHandler.ts index f3dd3dd82..69c429194 100644 --- a/src/main/presenter/agentPresenter/loop/toolCallHandler.ts +++ b/src/main/presenter/agentPresenter/loop/toolCallHandler.ts @@ -191,6 +191,24 @@ export class ToolCallHandler { } } + // CRITICAL FIX: Create the tool_call block so processToolCallUpdate/End can find it + // This ensures frontend state updates correctly after permission is granted + this.finalizeLastBlock(state) + state.message.content.push({ + type: 'tool_call', + content: '', + status: 'loading', + timestamp: currentTime, + tool_call: { + id: event.tool_call_id, + name: event.tool_call_name, + params: event.tool_call_params || '', + server_name: event.tool_call_server_name, + server_icons: event.tool_call_server_icons, + server_description: event.tool_call_server_description + } + }) + this.searchingMessages.delete(event.eventId) state.isSearching = false state.pendingToolCall = this.buildPendingToolCall(event) diff --git a/src/main/presenter/agentPresenter/loop/toolCallProcessor.ts b/src/main/presenter/agentPresenter/loop/toolCallProcessor.ts index c47117399..dbc7ed339 100644 --- a/src/main/presenter/agentPresenter/loop/toolCallProcessor.ts +++ b/src/main/presenter/agentPresenter/loop/toolCallProcessor.ts @@ -16,6 +16,7 @@ import { presenter } from '@/presenter' interface ToolCallProcessorOptions { getAllToolDefinitions: (context: ToolCallExecutionContext) => Promise callTool: (request: MCPToolCall) => Promise<{ content: unknown; rawData: MCPToolResponse }> + preCheckToolPermission?: (request: MCPToolCall) => Promise onToolCallFinished?: (info: { toolName: string toolCallId: string @@ -43,6 +44,50 @@ interface ToolCallProcessResult { needContinueConversation: boolean } +interface ToolCall { + id: string + name: string + arguments: string +} + +type PermissionType = 'read' | 'write' | 'all' | 'command' + +interface CommandInfoPayload { + command: string + riskLevel: 'low' | 'medium' | 'high' | 'critical' + suggestion: string + signature?: string + baseCommand?: string +} + +interface PermissionRequestPayload { + needsPermission: true + toolName: string + serverName: string + permissionType: PermissionType + description: string + command?: string + commandSignature?: string + commandInfo?: CommandInfoPayload + paths?: string[] + providerId?: string + requestId?: string + sessionId?: string + agentId?: string + agentName?: string + conversationId?: string + rememberable?: boolean + [key: string]: unknown +} + +interface PermissionRequestInfo { + toolCall: ToolCall + serverName: string + serverIcons?: string + serverDescription?: string + payload: PermissionRequestPayload +} + const TOOL_OUTPUT_OFFLOAD_THRESHOLD = 5000 const TOOL_OUTPUT_PREVIEW_LENGTH = 1024 const QUESTION_ERROR_KEY = 'common.error.invalidQuestionRequest' @@ -71,6 +116,51 @@ export class ToolCallProcessor { let toolDefinitions = await this.options.getAllToolDefinitions(context) + // Step 1: Pre-check all tool permissions in batch + // If any tool requires permission, we pause and request permission for all at once + const permissionCheckResult = await this.batchPreCheckPermissions(context, toolDefinitions) + + if (permissionCheckResult.hasPendingPermissions) { + // Yield permission request event for all tools that need permission + for (const permissionRequest of permissionCheckResult.permissionRequests) { + const permissionPayload = { + ...permissionRequest.payload, + toolName: permissionRequest.toolCall.name, + serverName: permissionRequest.serverName, + permissionType: permissionRequest.payload.permissionType, + description: permissionRequest.payload.description, + conversationId: permissionRequest.payload.conversationId ?? context.conversationId, + // Mark this as part of a batch + isBatchPermission: true, + totalInBatch: permissionCheckResult.permissionRequests.length + } + + yield { + type: 'response', + data: { + eventId: context.eventId, + tool_call: 'permission-required', + tool_call_id: permissionRequest.toolCall.id, + tool_call_name: permissionRequest.toolCall.name, + tool_call_params: permissionRequest.toolCall.arguments, + tool_call_server_name: permissionRequest.serverName, + tool_call_server_icons: permissionRequest.serverIcons, + tool_call_server_description: permissionRequest.serverDescription, + tool_call_response: permissionRequest.payload.description, + permission_request: permissionPayload + } + } + } + + // Stop here and wait for user to grant permissions + // The loop will be restarted after permissions are granted + needContinueConversation = false + return { + toolCallCount, + needContinueConversation + } + } + const resolveToolDefinition = async ( toolName: string ): Promise => { @@ -619,4 +709,76 @@ export class ToolCallProcessor { private stringifyToolContent(content: unknown): string { return typeof content === 'string' ? content : JSON.stringify(content) } + + /** + * Batch pre-check permissions for all tool calls + * Returns info about tools that need permission, or empty if all have permission + */ + private async batchPreCheckPermissions( + context: ToolCallExecutionContext, + toolDefinitions: MCPToolDefinition[] + ): Promise<{ + hasPendingPermissions: boolean + permissionRequests: PermissionRequestInfo[] + }> { + // If no permission pre-check function provided, skip batch check + if (!this.options.preCheckToolPermission) { + return { hasPendingPermissions: false, permissionRequests: [] } + } + + const permissionRequests: PermissionRequestInfo[] = [] + const toolNameToDefMap = new Map(toolDefinitions.map((t) => [t.function.name, t])) + + for (const toolCall of context.toolCalls) { + const toolDef = toolNameToDefMap.get(toolCall.name) + if (!toolDef) continue + + // Skip question tool for permission check + if (toolCall.name === QUESTION_TOOL_NAME) continue + + const mcpToolInput: MCPToolCall = { + id: toolCall.id, + type: 'function', + function: { + name: toolCall.name, + arguments: toolCall.arguments + }, + server: toolDef.server, + conversationId: context.conversationId + } + + try { + const permissionResult = await this.options.preCheckToolPermission(mcpToolInput) + if (permissionResult) { + const permissionPayload: PermissionRequestPayload = { + ...permissionResult, + toolName: permissionResult.toolName || toolCall.name, + serverName: permissionResult.serverName || toolDef.server.name, + permissionType: permissionResult.permissionType, + description: permissionResult.description + } + + // Preserve the full permission payload (paths and custom fields included) + permissionRequests.push({ + toolCall, + serverName: permissionPayload.serverName, + serverIcons: toolDef.server?.icons, + serverDescription: toolDef.server?.description, + payload: permissionPayload + }) + } + } catch (error) { + console.warn( + `[ToolCallProcessor] Failed to pre-check permission for ${toolCall.name}:`, + error + ) + // If pre-check fails, we'll let the actual execution handle it + } + } + + return { + hasPendingPermissions: permissionRequests.length > 0, + permissionRequests + } + } } diff --git a/src/main/presenter/agentPresenter/permission/permissionHandler.ts b/src/main/presenter/agentPresenter/permission/permissionHandler.ts index 6e1db94e0..297bdf0d4 100644 --- a/src/main/presenter/agentPresenter/permission/permissionHandler.ts +++ b/src/main/presenter/agentPresenter/permission/permissionHandler.ts @@ -7,16 +7,67 @@ import type { MCPToolDefinition, MCPToolResponse } from '@shared/presenter' -import { - buildContinueToolCallContext, - buildPostToolExecutionContext, - type PendingToolCall -} from '../message/messageBuilder' +import { buildPostToolExecutionContext, type PendingToolCall } from '../message/messageBuilder' import type { GeneratingMessageState } from '../streaming/types' import type { StreamGenerationHandler } from '../streaming/streamGenerationHandler' import type { LLMEventHandler } from '../streaming/llmEventHandler' import { BaseHandler, type ThreadHandlerContext } from '../../searchPresenter/handlers/baseHandler' import { CommandPermissionService } from '../../permission/commandPermissionService' +import { eventBus, SendTarget } from '@/eventbus' +import { STREAM_EVENTS } from '@/events' + +// Permission level hierarchy: all > write > read +// 'command' is a special type that only matches itself +const PERMISSION_LEVELS: Record = { + all: 3, + write: 2, + read: 1, + command: 0 // command only matches command (exact match required) +} + +function isPermissionSufficient(granted: string, required: string): boolean { + // Special case: command permission only applies to command-type permissions + if (granted === 'command' || required === 'command') { + return granted === required + } + return (PERMISSION_LEVELS[granted] || 0) >= (PERMISSION_LEVELS[required] || 0) +} + +function canBatchUpdate( + targetPermission: AssistantMessageBlock, + grantedPermission: AssistantMessageBlock, + grantedPermissionType: string +): boolean { + if (targetPermission.status !== 'pending') return false + if (targetPermission.action_type !== 'tool_call_permission') return false + + const targetServerName = targetPermission.extra?.serverName as string + const grantedServerName = grantedPermission.extra?.serverName as string + + // Must be same server + if (targetServerName !== grantedServerName) return false + + // CRITICAL FIX: Only batch the exact same tool call (same tool_call.id) + // This ensures user approval of one tool doesn't auto-approve other tools from the same server + if (targetPermission.tool_call?.id !== grantedPermission.tool_call?.id) return false + + // Check permission type hierarchy + const targetPermissionType = (targetPermission.extra?.permissionType as string) || 'read' + if (!isPermissionSufficient(grantedPermissionType, targetPermissionType)) return false + + // For special permission types, still require exact tool call matching (already checked above) + const targetType = targetPermission.extra?.permissionType as string + if ( + targetType === 'command' || + targetServerName === 'agent-filesystem' || + targetServerName === 'deepchat-settings' + ) { + // Additional safety: these types should never be batched across different tool calls + return targetPermission.tool_call?.id === grantedPermission.tool_call?.id + } + + return true +} export class PermissionHandler extends BaseHandler { private readonly generatingMessages: Map @@ -81,137 +132,168 @@ export class PermissionHandler extends BaseHandler { throw new Error(`Message not found or not assistant message (${messageId})`) } - const content = message.content as AssistantMessageBlock[] - const permissionBlock = content.find( - (block) => - block.type === 'action' && - block.action_type === 'tool_call_permission' && - block.tool_call?.id === toolCallId + const conversationId = message.conversationId + + // Step 1: Update permission blocks status (separate from resume) + const { updatedCount, targetPermissionBlock } = await this.updatePermissionBlocks( + messageId, + toolCallId, + granted, + permissionType ) - if (!permissionBlock) { - throw new Error( - `Permission block not found (messageId: ${messageId}, toolCallId: ${toolCallId})` + console.log(`[PermissionHandler] Updated ${updatedCount} permission block(s)`) + + // Debug: if updatedCount is 0, print details + if (updatedCount === 0) { + const content = message.content as AssistantMessageBlock[] + const pendingBlocks = content.filter( + (b) => + b.type === 'action' && + b.action_type === 'tool_call_permission' && + b.status === 'pending' ) + console.log( + '[PermissionHandler] Debug - All pending permission blocks:', + pendingBlocks.map((b) => ({ + toolCallId: b.tool_call?.id, + status: b.status, + serverName: b.extra?.serverName + })) + ) + console.log('[PermissionHandler] Debug - Looking for toolCallId:', toolCallId) } - const parsedPermissionRequest = this.parsePermissionRequest(permissionBlock) - const isAcpPermission = this.isAcpPermissionBlock(permissionBlock, parsedPermissionRequest) - - permissionBlock.status = granted ? 'granted' : 'denied' - if (permissionBlock.extra) { - permissionBlock.extra.needsUserAction = false - if (granted) { - permissionBlock.extra.grantedPermissions = permissionType - } + // Step 2: Remove this permission from pending list (only if we actually updated something) + if (updatedCount > 0) { + presenter.sessionManager.removePendingPermission(conversationId, messageId, toolCallId) + this.notifyFrontendPermissionUpdate(conversationId, messageId) + } else { + console.warn( + '[PermissionHandler] No permission blocks were updated, skipping removal from pending list' + ) + // Still need to notify frontend to refresh in case there's a mismatch + this.notifyFrontendPermissionUpdate(conversationId, messageId) + return } - const generatingState = this.generatingMessages.get(messageId) - if (generatingState) { - const permissionIndex = generatingState.message.content.findIndex( - (block) => - block.type === 'action' && - block.action_type === 'tool_call_permission' && - block.tool_call?.id === toolCallId + // Step 3: Check if this is ACP permission (special handling) + if (targetPermissionBlock) { + const parsedPermissionRequest = this.parsePermissionRequest(targetPermissionBlock) + const isAcpPermission = this.isAcpPermissionBlock( + targetPermissionBlock, + parsedPermissionRequest ) - if (permissionIndex !== -1) { - const statePermissionBlock = generatingState.message.content[permissionIndex] - generatingState.message.content[permissionIndex] = { - ...statePermissionBlock, - ...permissionBlock, - extra: permissionBlock.extra - ? { - ...permissionBlock.extra - } - : undefined, - tool_call: permissionBlock.tool_call - ? { - ...permissionBlock.tool_call - } - : undefined - } + if (isAcpPermission) { + await this.handleAcpPermissionFlow( + messageId, + targetPermissionBlock, + parsedPermissionRequest, + granted + ) + presenter.sessionManager.clearPendingPermission(conversationId) + presenter.sessionManager.setStatus(conversationId, 'generating') + return } } - await this.ctx.messageManager.editMessage(messageId, JSON.stringify(content)) - - if (isAcpPermission) { - await this.handleAcpPermissionFlow( - messageId, - permissionBlock, - parsedPermissionRequest, - granted - ) - presenter.sessionManager.clearPendingPermission(message.conversationId) - presenter.sessionManager.setStatus(message.conversationId, 'generating') - return - } - - if (permissionType === 'command') { + // Step 4: Handle specific permission types (command, agent-filesystem, deepchat-settings) + if (targetPermissionBlock && permissionType === 'command') { if (granted) { - const conversationId = message.conversationId - const command = this.getCommandFromPermissionBlock(permissionBlock) + const command = this.getCommandFromPermissionBlock(targetPermissionBlock) if (!command) { throw new Error(`Unable to extract command from permission block (${messageId})`) } const signature = this.commandPermissionHandler.extractCommandSignature(command) this.commandPermissionHandler.approve(conversationId, signature, remember) - await this.restartAgentLoopAfterPermission(messageId, toolCallId) - } else { - await this.continueAfterPermissionDenied(messageId, permissionBlock) } - return - } - - if (granted) { - const serverName = permissionBlock?.extra?.serverName as string + // Continue to check for resume (don't return early) + } else if (targetPermissionBlock && granted && permissionType !== 'command') { + const serverName = targetPermissionBlock?.extra?.serverName as string if (!serverName) { throw new Error(`Server name not found in permission block (${messageId})`) } if (serverName === 'agent-filesystem') { + const parsedPermissionRequest = this.parsePermissionRequest(targetPermissionBlock) const paths = this.getStringArrayFromObject(parsedPermissionRequest, 'paths') if (paths.length === 0) { console.warn('[PermissionHandler] Missing filesystem paths in permission request') - await this.continueAfterPermissionDenied(messageId, permissionBlock) - return + // Mark as denied and continue + await this.updatePermissionBlocks(messageId, toolCallId, false, permissionType) + } else { + presenter.filePermissionService?.approve(conversationId, paths, remember) } - presenter.filePermissionService?.approve(message.conversationId, paths, remember) - await this.restartAgentLoopAfterPermission(messageId, toolCallId) - return - } - - if (serverName === 'deepchat-settings') { + } else if (serverName === 'deepchat-settings') { + const parsedPermissionRequest = this.parsePermissionRequest(targetPermissionBlock) const toolName = this.getStringFromObject(parsedPermissionRequest, 'toolName') || - this.getStringFromObject(permissionBlock.extra as Record, 'toolName') + this.getStringFromObject( + targetPermissionBlock.extra as Record, + 'toolName' + ) if (!toolName) { console.warn('[PermissionHandler] Missing tool name in settings permission request') - await this.continueAfterPermissionDenied(messageId, permissionBlock) - return + await this.updatePermissionBlocks(messageId, toolCallId, false, permissionType) + } else { + presenter.settingsPermissionService?.approve(conversationId, toolName, remember) + } + } else { + // MCP server permission + try { + await this.getMcpPresenter().grantPermission( + serverName, + permissionType, + remember, + conversationId + ) + await this.waitForMcpServiceReady(serverName) + } catch (error) { + console.error('[PermissionHandler] Failed to grant MCP permission:', error) + throw error } - presenter.settingsPermissionService?.approve(message.conversationId, toolName, remember) - await this.restartAgentLoopAfterPermission(messageId, toolCallId) - return } + } - try { - await this.getMcpPresenter().grantPermission(serverName, permissionType, remember) - await this.waitForMcpServiceReady(serverName) - } catch (error) { - permissionBlock.status = 'error' - await this.ctx.messageManager.editMessage(messageId, JSON.stringify(content)) - throw error - } + // Step 5: Check if there are still pending permissions in this message + const hasPendingPermissions = await this.hasPendingPermissionsInMessage(messageId) + if (hasPendingPermissions) { + console.log( + '[PermissionHandler] Still has pending permissions, waiting for all to be resolved' + ) + // Notify frontend to refresh permission UI (show next pending permission) + this.notifyFrontendPermissionUpdate(conversationId, messageId) + return + } - await this.restartAgentLoopAfterPermission(messageId, toolCallId) - } else { - await this.continueAfterPermissionDenied(messageId, permissionBlock) + // Step 6: All permissions resolved - try to acquire resume lock and execute + const lockAcquired = presenter.sessionManager.acquirePermissionResumeLock( + conversationId, + messageId + ) + if (!lockAcquired) { + console.log('[PermissionHandler] Resume already in progress for this message, skipping') + return } + + // Step 7: Resume tool execution (idempotent - only one resume per message) + // Resume all resolved tool calls in this message (granted and denied handling) + // CRITICAL: Lock is released inside resumeToolExecutionAfterPermissions (success or error) + await this.resumeToolExecutionAfterPermissions(messageId, granted) } catch (error) { console.error('[PermissionHandler] Failed to handle permission response:', error) + // CRITICAL: Ensure lock is released on error (belt-and-suspenders approach) + try { + const conversationId = await this.getConversationIdFromMessage(messageId) + if (conversationId) { + presenter.sessionManager.releasePermissionResumeLock(conversationId) + } + } catch (lockError) { + console.warn('[PermissionHandler] Failed to release lock during error handling:', lockError) + } + try { const message = await this.ctx.messageManager.getMessage(messageId) if (message) { @@ -225,6 +307,583 @@ export class PermissionHandler extends BaseHandler { } } + /** + * Update permission block(s) status + * Returns the number of updated blocks and the target permission block + */ + private async updatePermissionBlocks( + messageId: string, + toolCallId: string, + granted: boolean, + permissionType: string + ): Promise<{ updatedCount: number; targetPermissionBlock: AssistantMessageBlock | undefined }> { + const message = await this.ctx.messageManager.getMessage(messageId) + if (!message || message.role !== 'assistant') { + throw new Error(`Message not found or not assistant message (${messageId})`) + } + + const content = message.content as AssistantMessageBlock[] + const targetPermissionBlock = content.find( + (block) => + block.type === 'action' && + block.action_type === 'tool_call_permission' && + block.tool_call?.id === toolCallId + ) + + if (!targetPermissionBlock) { + throw new Error( + `Permission block not found (messageId: ${messageId}, toolCallId: ${toolCallId})` + ) + } + + let updatedCount = 0 + + // Batch update: only update blocks that can be safely batched + for (const block of content) { + if (canBatchUpdate(block, targetPermissionBlock, permissionType)) { + block.status = granted ? 'granted' : 'denied' + if (block.extra) { + block.extra.needsUserAction = false + if (granted) { + // Only store valid MCP permission types; 'command' is handled separately + if ( + permissionType === 'read' || + permissionType === 'write' || + permissionType === 'all' + ) { + block.extra.grantedPermissions = permissionType + } + } + } + updatedCount++ + } + } + + // Update generating state snapshot + const generatingState = this.generatingMessages.get(messageId) + if (generatingState) { + for (let i = 0; i < generatingState.message.content.length; i++) { + const block = generatingState.message.content[i] + if ( + block.type === 'action' && + block.action_type === 'tool_call_permission' && + block.status === 'pending' + ) { + // Check if this block was updated in the main content + const updatedBlock = content.find( + (b) => + b.type === 'action' && + b.action_type === 'tool_call_permission' && + b.tool_call?.id === block.tool_call?.id + ) + if (updatedBlock && updatedBlock.status !== 'pending') { + generatingState.message.content[i] = { + ...block, + status: updatedBlock.status, + extra: updatedBlock.extra + } + } + } + } + } + + await this.ctx.messageManager.editMessage(messageId, JSON.stringify(content)) + + return { updatedCount, targetPermissionBlock } + } + + /** + * Resume stream completion when no tools need to be executed + */ + private async resumeStreamCompletion(conversationId: string, messageId: string): Promise { + // Use streamGenerationHandler's startStreamCompletion which handles the full context + await this.streamGenerationHandler.startStreamCompletion(conversationId, messageId) + } + + /** + * Check if there are still pending permissions in the message + */ + private async hasPendingPermissionsInMessage(messageId: string): Promise { + const message = await this.ctx.messageManager.getMessage(messageId) + if (!message || message.role !== 'assistant') { + return false + } + + const content = message.content as AssistantMessageBlock[] + return content.some( + (block) => + block.type === 'action' && + block.action_type === 'tool_call_permission' && + block.status === 'pending' + ) + } + + /** + * Get conversation ID from message ID + */ + private async getConversationIdFromMessage(messageId: string): Promise { + const message = await this.ctx.messageManager.getMessage(messageId) + if (!message) { + return null + } + return message.conversationId + } + + /** + * Resume tool execution after permission is granted + * CRITICAL SECTION: Lock is held throughout the entire resume flow + * - Early-exit checks prevent stale execution + * - Synchronous flush before executing tools + * - Lock released only at single exit point + * - All tools executed atomically (no lock release between tools) + */ + private async resumeToolExecutionAfterPermissions( + messageId: string, + _lastGranted: boolean, + grantedToolCallId?: string + ): Promise { + console.log( + '[PermissionHandler] Resuming tool execution after permissions', + messageId, + 'grantedToolCallId:', + grantedToolCallId + ) + + const conversationId = await this.getConversationIdFromMessage(messageId) + if (!conversationId) { + throw new Error(`Message not found (${messageId})`) + } + + // CRITICAL SECTION: Lock must be held throughout this entire method + // Early-exit checks: Validate session state before proceeding + const session = presenter.sessionManager.getSessionSync(conversationId) + if (!session) { + console.warn('[PermissionHandler] Session not found, skipping resume:', conversationId) + presenter.sessionManager.releasePermissionResumeLock(conversationId) + return + } + + // Verify the lock is still valid (same message) + const currentLock = presenter.sessionManager.getPermissionResumeLock(conversationId) + if (!currentLock || currentLock.messageId !== messageId) { + console.warn( + '[PermissionHandler] Lock mismatch or expired, skipping resume. Expected:', + messageId, + 'Got:', + currentLock?.messageId + ) + // CRITICAL: Always release lock if we don't proceed - it was acquired in handlePermissionResponse + if (currentLock) { + presenter.sessionManager.releasePermissionResumeLock(conversationId) + } + return + } + + // Ensure status is appropriate for tool execution + // Transition from waiting_permission to generating since we're resuming + const currentStatus = presenter.sessionManager.getStatus(conversationId) + if (currentStatus === 'waiting_permission') { + console.log('[PermissionHandler] Transitioning session from waiting_permission to generating') + presenter.sessionManager.setStatus(conversationId, 'generating') + } else if ( + currentStatus === 'idle' && + presenter.sessionManager.hasPendingPermissions(conversationId, messageId) + ) { + console.warn( + '[PermissionHandler] Session was idle during permission resume, forcing generating status' + ) + presenter.sessionManager.setStatus(conversationId, 'generating') + } else if (currentStatus !== 'generating') { + console.warn( + '[PermissionHandler] Session status not suitable for resume. Status:', + currentStatus + ) + presenter.sessionManager.releasePermissionResumeLock(conversationId) + return + } + + try { + // Step 1: Start the agent loop with pending permissions preservation + // skipLockAcquisition: PermissionHandler already holds the lock + await presenter.sessionManager.startLoop(conversationId, messageId, { + preservePendingPermissions: true, + skipLockAcquisition: true + }) + + // Step 2: Re-fetch message to get updated permission block statuses + const updatedMessage = await this.ctx.messageManager.getMessage(messageId) + if (!updatedMessage) { + throw new Error(`Message not found after permission update (${messageId})`) + } + + // Step 3: Get tool calls to execute + const toolCallsToExecute = this.getToolCallsToExecute( + updatedMessage.content as AssistantMessageBlock[], + grantedToolCallId + ) + + if (toolCallsToExecute.length === 0) { + console.log( + '[PermissionHandler] No tool calls to execute, continuing with stream completion' + ) + await this.resumeStreamCompletion(conversationId, messageId) + // SINGLE EXIT POINT: Release lock + presenter.sessionManager.releasePermissionResumeLock(conversationId) + return + } + + // Step 4: Set up generating state + let state = this.generatingMessages.get(messageId) + if (!state) { + state = { + message: updatedMessage as AssistantMessage, + conversationId, + startTime: Date.now(), + firstTokenTime: null, + promptTokens: 0, + reasoningStartTime: null, + reasoningEndTime: null, + lastReasoningTime: null + } + this.generatingMessages.set(messageId, state) + } else { + // CRITICAL FIX: Sync state.message.content with database to ensure tool_call blocks + // created by the frontend are available for processToolCallEnd to update + state.message.content = (updatedMessage as AssistantMessage).content + } + + // Step 5: SYNCHRONOUS FLUSH before executing tools + // This ensures all pending UI updates are persisted to DB before tool execution + await this.llmEventHandler.flushStreamUpdates(messageId) + + // Step 6: Execute tools sequentially (lock held throughout - NO RELEASE BETWEEN TOOLS) + let hasNewPermissionRequest = false + for (const toolCall of toolCallsToExecute) { + const canContinue = await this.executeSingleToolCall(state, toolCall, conversationId) + + if (!canContinue) { + // Permission required again - but we keep the lock until end of critical section + hasNewPermissionRequest = true + break + } + } + + // Ensure tool_call end/error updates are persisted before rebuilding next-turn context. + await this.llmEventHandler.flushStreamUpdates(messageId) + + // Step 7: Check if there are still pending permissions + const stillHasPending = await this.hasPendingPermissionsInMessage(messageId) + if (stillHasPending || hasNewPermissionRequest) { + console.log( + '[PermissionHandler] Tool(s) executed but more permissions pending, releasing lock and waiting' + ) + // SINGLE EXIT POINT: Release lock + presenter.sessionManager.releasePermissionResumeLock(conversationId) + this.notifyFrontendPermissionUpdate(conversationId, messageId) + return + } + + // Step 8: All permissions resolved, continue with stream completion + await this.continueAfterToolsExecuted(state, conversationId, messageId) + // SINGLE EXIT POINT: Release lock after successful completion + presenter.sessionManager.releasePermissionResumeLock(conversationId) + } catch (error) { + console.error('[PermissionHandler] Failed to resume tool execution:', error) + this.generatingMessages.delete(messageId) + + try { + const message = await this.ctx.messageManager.getMessage(messageId) + if (message) { + await this.ctx.messageManager.handleMessageError(messageId, String(error)) + } + } catch (updateError) { + console.error('[PermissionHandler] Failed to update message error status:', updateError) + } + + // SINGLE EXIT POINT: Ensure lock is released on error + presenter.sessionManager.releasePermissionResumeLock(conversationId) + throw error + } + } + + /** + * Get tool calls that should be executed + * If specificToolCallId is provided, only returns that specific tool call + * Otherwise returns all granted tool calls + */ + private getToolCallsToExecute( + content: AssistantMessageBlock[], + specificToolCallId?: string + ): PendingToolCall[] { + const toolCalls: PendingToolCall[] = [] + + for (const block of content) { + if (block.type !== 'tool_call' || !block.tool_call) continue + + const toolCallId = block.tool_call.id + if (!toolCallId) continue + // Only resume unfinished tool calls. + // Completed blocks (success/error) are historical records and must not be re-executed. + if (block.status !== 'loading') continue + + // If specific tool call ID is specified, only process that one + if (specificToolCallId && toolCallId !== specificToolCallId) { + continue + } + + // Find the associated permission block + const permissionBlock = content.find( + (b) => + b.type === 'action' && + b.action_type === 'tool_call_permission' && + b.tool_call?.id === toolCallId + ) + + // If there's a permission block, check its status + if (permissionBlock) { + if (permissionBlock.status === 'granted') { + const pendingCall = this.buildPendingToolCallFromBlock(block) + if (pendingCall) toolCalls.push(pendingCall) + } else if (permissionBlock.status === 'denied') { + // Denied - will generate error response later + const pendingCall = this.buildPendingToolCallFromBlock(block) + if (pendingCall) { + toolCalls.push({ ...pendingCall, denied: true } as PendingToolCall) + } + } + // If pending, skip (should not happen after permission resolution) + } else { + // No permission block needed - execute directly + const pendingCall = this.buildPendingToolCallFromBlock(block) + if (pendingCall) toolCalls.push(pendingCall) + } + } + + console.log( + `[PermissionHandler] Found ${toolCalls.length} tool calls to execute` + + (specificToolCallId ? ` (specific: ${specificToolCallId})` : ' (all granted)') + ) + return toolCalls + } + + private buildPendingToolCallFromBlock(block: AssistantMessageBlock): PendingToolCall | undefined { + if (!block.tool_call) return undefined + + const { id, name, params } = block.tool_call + if (!id || !name) { + console.warn('[PermissionHandler] Incomplete tool call info:', block.tool_call) + return undefined + } + + return { + id, + name, + params: params || '{}', + serverName: block.tool_call.server_name, + serverIcons: block.tool_call.server_icons, + serverDescription: block.tool_call.server_description + } + } + + /** + * Execute a single tool call + * Returns false if permission is required again + */ + private async executeSingleToolCall( + state: GeneratingMessageState, + toolCall: PendingToolCall, + conversationId: string + ): Promise { + // Check if this tool was denied + const message = await this.ctx.messageManager.getMessage(state.message.id) + if (!message) { + console.warn( + '[PermissionHandler] Message not found while executing tool call, aborting execution:', + state.message.id + ) + return false + } + const content = message.content as AssistantMessageBlock[] + const permissionBlock = content.find( + (b) => + b.type === 'action' && + b.action_type === 'tool_call_permission' && + b.tool_call?.id === toolCall.id + ) + + if (permissionBlock?.status === 'denied') { + // Generate error response for denied tool + await this.llmEventHandler.handleLLMAgentResponse({ + eventId: state.message.id, + tool_call: 'error', + tool_call_id: toolCall.id, + tool_call_name: toolCall.name, + tool_call_params: toolCall.params, + tool_call_response: 'User denied the request.', + tool_call_server_name: toolCall.serverName, + tool_call_server_icons: toolCall.serverIcons, + tool_call_server_description: toolCall.serverDescription + } as any) + return true + } + + // Normal tool execution + try { + const { conversation } = await this.streamGenerationHandler.prepareConversationContext( + conversationId, + state.message.id + ) + + let toolDef: MCPToolDefinition | undefined + try { + const { chatMode, agentWorkspacePath } = + await presenter.sessionManager.resolveWorkspaceContext( + conversationId, + conversation.settings.modelId + ) + const toolDefinitions = await this.getToolPresenter().getAllToolDefinitions({ + enabledMcpTools: conversation.settings.enabledMcpTools, + chatMode, + supportsVision: false, + agentWorkspacePath, + conversationId + }) + toolDef = toolDefinitions.find((definition) => { + if (definition.function.name !== toolCall.name) return false + if (toolCall.serverName) { + return definition.server.name === toolCall.serverName + } + return true + }) + } catch (error) { + console.error('[PermissionHandler] Failed to load tool definitions:', error) + } + + if (!toolDef) { + console.warn('[PermissionHandler] Tool definition not found:', toolCall.name) + return true // Continue with next tool + } + + const resolvedServer = toolDef.server + + // Emit running state + await this.llmEventHandler.handleLLMAgentResponse({ + eventId: state.message.id, + tool_call: 'running', + tool_call_id: toolCall.id, + tool_call_name: toolCall.name, + tool_call_params: toolCall.params, + tool_call_server_name: resolvedServer?.name || toolCall.serverName, + tool_call_server_icons: resolvedServer?.icons || toolCall.serverIcons, + tool_call_server_description: resolvedServer?.description || toolCall.serverDescription + } as any) + + // Execute tool + let toolContent = '' + let toolRawData: MCPToolResponse | null = null + try { + const toolCallResult = await this.getToolPresenter().callTool({ + id: toolCall.id, + type: 'function', + function: { + name: toolCall.name, + arguments: toolCall.params + }, + server: resolvedServer, + conversationId + }) + toolContent = + typeof toolCallResult.content === 'string' + ? toolCallResult.content + : JSON.stringify(toolCallResult.content) + toolRawData = toolCallResult.rawData + } catch (toolError) { + console.error('[PermissionHandler] Failed to execute tool:', toolError) + await this.llmEventHandler.handleLLMAgentResponse({ + eventId: state.message.id, + tool_call: 'error', + tool_call_id: toolCall.id, + tool_call_name: toolCall.name, + tool_call_params: toolCall.params, + tool_call_response: toolError instanceof Error ? toolError.message : String(toolError), + tool_call_server_name: resolvedServer?.name || toolCall.serverName, + tool_call_server_icons: resolvedServer?.icons || toolCall.serverIcons, + tool_call_server_description: resolvedServer?.description || toolCall.serverDescription + } as any) + return true // Continue with next tool + } + + // Check if permission is required again + if (toolRawData?.requiresPermission) { + // Add this permission to pending list and set session status + presenter.sessionManager.addPendingPermission(conversationId, { + messageId: state.message.id, + toolCallId: toolCall.id, + permissionType: + (toolRawData.permissionRequest?.permissionType as + | 'read' + | 'write' + | 'all' + | 'command') || 'read', + payload: toolRawData.permissionRequest ?? {} + }) + presenter.sessionManager.setStatus(conversationId, 'waiting_permission') + + await this.llmEventHandler.handleLLMAgentResponse({ + eventId: state.message.id, + tool_call: 'permission-required', + tool_call_id: toolCall.id, + tool_call_name: toolCall.name, + tool_call_params: toolCall.params, + tool_call_server_name: + toolRawData.permissionRequest?.serverName || + resolvedServer?.name || + toolCall.serverName, + tool_call_server_icons: resolvedServer?.icons || toolCall.serverIcons, + tool_call_server_description: resolvedServer?.description || toolCall.serverDescription, + tool_call_response: toolContent, + permission_request: toolRawData.permissionRequest + } as any) + return false // Stop execution, permission required + } + + // Tool completed successfully + await this.llmEventHandler.handleLLMAgentResponse({ + eventId: state.message.id, + tool_call: 'end', + tool_call_id: toolCall.id, + tool_call_name: toolCall.name, + tool_call_params: toolCall.params, + tool_call_response: toolContent, + tool_call_server_name: resolvedServer?.name || toolCall.serverName, + tool_call_server_icons: resolvedServer?.icons || toolCall.serverIcons, + tool_call_server_description: resolvedServer?.description || toolCall.serverDescription, + tool_call_response_raw: toolRawData ?? undefined + } as any) + + return true + } catch (error) { + console.error('[PermissionHandler] Error executing single tool call:', error) + return true // Continue with next tool + } + } + + /** + * Continue with model generation after all tools are executed + */ + private async continueAfterToolsExecuted( + _state: GeneratingMessageState, + conversationId: string, + messageId: string + ): Promise { + // Simplified: use streamGenerationHandler which handles full context + await this.streamGenerationHandler.startStreamCompletion(conversationId, messageId) + } + + /** + * Restart agent loop after permission is granted + * UNIFIED FLOW: Uses resumeToolExecutionAfterPermissions for all cases + */ async restartAgentLoopAfterPermission(messageId: string, toolCallId?: string): Promise { console.log('[PermissionHandler] Restarting agent loop after permission', messageId) @@ -234,10 +893,10 @@ export class PermissionHandler extends BaseHandler { throw new Error(`Message not found (${messageId})`) } - const conversationId = message.conversationId - await presenter.sessionManager.startLoop(conversationId, messageId) - const content = message.content as AssistantMessageBlock[] + // const conversationId = message.conversationId + // Check server permissions for logging/debugging + const content = message.content as AssistantMessageBlock[] const permissionBlock = content.find( (block) => block.type === 'action' && @@ -259,47 +918,12 @@ export class PermissionHandler extends BaseHandler { console.warn('[PermissionHandler] Granted permission block missing; continuing', messageId) } - const pendingToolCallFromPermission = - this.buildPendingToolCallFromPermissionBlock(permissionBlock) - const pendingToolCallFromId = toolCallId - ? this.buildPendingToolCallFromToolCallId(content, toolCallId) - : undefined - const fallbackPendingToolCall = - pendingToolCallFromPermission ?? - pendingToolCallFromId ?? - this.findPendingToolCallAfterPermission(content) - - const state = this.generatingMessages.get(messageId) - if (state) { - if (!state.pendingToolCall && fallbackPendingToolCall) { - state.pendingToolCall = fallbackPendingToolCall - } - if (state.pendingToolCall) { - await this.resumeAfterPermissionWithPendingToolCall( - state, - message as AssistantMessage, - conversationId - ) - } else { - await this.resumeStreamCompletion(conversationId, messageId) - } - return - } - - const assistantMessage = message as AssistantMessage - this.generatingMessages.set(messageId, { - message: assistantMessage, - conversationId, - startTime: Date.now(), - firstTokenTime: null, - promptTokens: 0, - reasoningStartTime: null, - reasoningEndTime: null, - lastReasoningTime: null, - pendingToolCall: fallbackPendingToolCall - }) - - await this.streamGenerationHandler.startStreamCompletion(conversationId, messageId) + // UNIFIED FLOW: Use resumeToolExecutionAfterPermissions for all cases + // This method handles: + // 1. Setting up the generating state + // 2. Finding and executing all granted tool calls + // 3. Continuing with stream completion + await this.resumeToolExecutionAfterPermissions(messageId, true) } catch (error) { console.error('[PermissionHandler] Failed to restart agent loop:', error) this.generatingMessages.delete(messageId) @@ -327,7 +951,11 @@ export class PermissionHandler extends BaseHandler { } const conversationId = message.conversationId - await presenter.sessionManager.startLoop(conversationId, messageId) + // Permission denied flow: skip lock acquisition (not part of permission resume critical section) + await presenter.sessionManager.startLoop(conversationId, messageId, { + preservePendingPermissions: true, + skipLockAcquisition: true + }) const content = message.content as AssistantMessageBlock[] const deniedPermissionBlock = resolvedPermissionBlock || @@ -453,287 +1081,6 @@ export class PermissionHandler extends BaseHandler { } } - async resumeStreamCompletion(conversationId: string, messageId: string): Promise { - const state = this.generatingMessages.get(messageId) - if (!state) { - await this.streamGenerationHandler.startStreamCompletion(conversationId, undefined) - return - } - - try { - await presenter.sessionManager.startLoop(conversationId, messageId) - const conversation = await this.ctx.sqlitePresenter.getConversation(conversationId) - if (!conversation) { - throw new Error(`Conversation not found (${conversationId})`) - } - - const pendingToolCall = this.findPendingToolCallAfterPermission(state.message.content) - if (!pendingToolCall) { - await this.streamGenerationHandler.startStreamCompletion(conversationId, messageId) - return - } - - const { contextMessages, userMessage } = - await this.streamGenerationHandler.prepareConversationContext(conversationId, messageId) - - const modelConfig = this.ctx.configPresenter.getModelConfig( - conversation.settings.modelId, - conversation.settings.providerId - ) - - const finalContent = await buildContinueToolCallContext({ - conversation, - contextMessages, - userMessage, - pendingToolCall, - modelConfig - }) - - const stream = this.llmProviderPresenter.startStreamCompletion( - conversation.settings.providerId, - finalContent, - conversation.settings.modelId, - messageId, - conversation.settings.temperature, - conversation.settings.maxTokens, - conversation.settings.enabledMcpTools, - conversation.settings.thinkingBudget, - conversation.settings.reasoningEffort, - conversation.settings.verbosity, - conversation.settings.enableSearch, - conversation.settings.forcedSearch, - conversation.settings.searchStrategy, - conversationId - ) - - for await (const event of stream) { - const msg = event.data - if (event.type === 'response') { - await this.llmEventHandler.handleLLMAgentResponse(msg) - } else if (event.type === 'error') { - await this.llmEventHandler.handleLLMAgentError(msg) - } else if (event.type === 'end') { - await this.llmEventHandler.handleLLMAgentEnd(msg) - } - } - } catch (error) { - console.error('[PermissionHandler] Failed to resume stream completion:', error) - this.generatingMessages.delete(messageId) - - try { - await this.ctx.messageManager.handleMessageError(messageId, String(error)) - } catch (updateError) { - console.error('[PermissionHandler] Failed to update message error status:', updateError) - } - - throw error - } - } - - async resumeAfterPermissionWithPendingToolCall( - state: GeneratingMessageState, - message: AssistantMessage, - conversationId: string - ): Promise { - const pendingToolCall = state.pendingToolCall - if (!pendingToolCall || !pendingToolCall.id || !pendingToolCall.name) { - await this.resumeStreamCompletion(conversationId, message.id) - return - } - - try { - const { conversation, contextMessages, userMessage } = - await this.streamGenerationHandler.prepareConversationContext(conversationId, message.id) - - const { - providerId, - modelId, - temperature, - maxTokens, - enabledMcpTools, - thinkingBudget, - reasoningEffort, - verbosity, - enableSearch, - forcedSearch, - searchStrategy - } = conversation.settings - - const modelConfig = this.ctx.configPresenter.getModelConfig(modelId, providerId) - if (!modelConfig) { - await this.resumeStreamCompletion(conversationId, message.id) - return - } - - let toolDef: MCPToolDefinition | undefined - try { - const { chatMode, agentWorkspacePath } = - await presenter.sessionManager.resolveWorkspaceContext( - conversationId, - conversation.settings.modelId - ) - const toolDefinitions = await this.getToolPresenter().getAllToolDefinitions({ - enabledMcpTools, - chatMode, - supportsVision: false, - agentWorkspacePath, - conversationId - }) - toolDef = toolDefinitions.find((definition) => { - if (definition.function.name !== pendingToolCall.name) { - return false - } - if (pendingToolCall.serverName) { - return definition.server.name === pendingToolCall.serverName - } - return true - }) - } catch (error) { - console.error('[PermissionHandler] Failed to load tool definitions:', error) - } - - if (!toolDef) { - await this.resumeStreamCompletion(conversationId, message.id) - return - } - - const resolvedServer = toolDef?.server - await this.llmEventHandler.handleLLMAgentResponse({ - eventId: message.id, - tool_call: 'running', - tool_call_id: pendingToolCall.id, - tool_call_name: pendingToolCall.name, - tool_call_params: pendingToolCall.params, - tool_call_server_name: resolvedServer?.name || pendingToolCall.serverName, - tool_call_server_icons: resolvedServer?.icons || pendingToolCall.serverIcons, - tool_call_server_description: - resolvedServer?.description || pendingToolCall.serverDescription - } as any) - - let toolContent = '' - let toolRawData: MCPToolResponse | null = null - try { - const toolCallResult = await this.getToolPresenter().callTool({ - id: pendingToolCall.id, - type: 'function', - function: { - name: pendingToolCall.name, - arguments: pendingToolCall.params - }, - server: resolvedServer, - conversationId - }) - toolContent = - typeof toolCallResult.content === 'string' - ? toolCallResult.content - : JSON.stringify(toolCallResult.content) - toolRawData = toolCallResult.rawData - } catch (toolError) { - console.error('[PermissionHandler] Failed to execute pending tool call:', toolError) - await this.llmEventHandler.handleLLMAgentResponse({ - eventId: message.id, - tool_call: 'error', - tool_call_id: pendingToolCall.id, - tool_call_name: pendingToolCall.name, - tool_call_params: pendingToolCall.params, - tool_call_response: toolError instanceof Error ? toolError.message : String(toolError), - tool_call_server_name: resolvedServer?.name || pendingToolCall.serverName, - tool_call_server_icons: resolvedServer?.icons || pendingToolCall.serverIcons, - tool_call_server_description: - resolvedServer?.description || pendingToolCall.serverDescription - } as any) - throw toolError - } - - if (toolRawData?.requiresPermission) { - await this.llmEventHandler.handleLLMAgentResponse({ - eventId: message.id, - tool_call: 'permission-required', - tool_call_id: pendingToolCall.id, - tool_call_name: pendingToolCall.name, - tool_call_params: pendingToolCall.params, - tool_call_server_name: - toolRawData.permissionRequest?.serverName || - resolvedServer?.name || - pendingToolCall.serverName, - tool_call_server_icons: resolvedServer?.icons || pendingToolCall.serverIcons, - tool_call_server_description: - resolvedServer?.description || pendingToolCall.serverDescription, - tool_call_response: toolContent, - permission_request: toolRawData.permissionRequest - } as any) - return - } - - await this.llmEventHandler.handleLLMAgentResponse({ - eventId: message.id, - tool_call: 'end', - tool_call_id: pendingToolCall.id, - tool_call_name: pendingToolCall.name, - tool_call_params: pendingToolCall.params, - tool_call_response: toolContent, - tool_call_server_name: resolvedServer?.name || pendingToolCall.serverName, - tool_call_server_icons: resolvedServer?.icons || pendingToolCall.serverIcons, - tool_call_server_description: - resolvedServer?.description || pendingToolCall.serverDescription, - tool_call_response_raw: toolRawData ?? undefined - } as any) - - state.pendingToolCall = undefined - - const finalContent = await buildPostToolExecutionContext({ - conversation, - contextMessages, - userMessage, - currentAssistantMessage: state.message, - completedToolCall: { - ...pendingToolCall, - response: toolContent - }, - modelConfig - }) - - const stream = this.llmProviderPresenter.startStreamCompletion( - providerId, - finalContent, - modelId, - message.id, - temperature, - maxTokens, - enabledMcpTools, - thinkingBudget, - reasoningEffort, - verbosity, - enableSearch, - forcedSearch, - searchStrategy, - conversation.id - ) - - for await (const event of stream) { - const msg = event.data - if (event.type === 'response') { - await this.llmEventHandler.handleLLMAgentResponse(msg) - } else if (event.type === 'error') { - await this.llmEventHandler.handleLLMAgentError(msg) - } else if (event.type === 'end') { - await this.llmEventHandler.handleLLMAgentEnd(msg) - } - } - } catch (error) { - console.error('[PermissionHandler] Failed to resume pending tool call:', error) - this.generatingMessages.delete(message.id) - - try { - await this.ctx.messageManager.handleMessageError(message.id, String(error)) - } catch (updateError) { - console.error('[PermissionHandler] Failed to update message error status:', updateError) - } - - throw error - } - } - async waitForMcpServiceReady(serverName: string, maxWaitTime: number = 3000): Promise { const startTime = Date.now() const checkInterval = 100 @@ -764,69 +1111,6 @@ export class PermissionHandler extends BaseHandler { }) } - findPendingToolCallAfterPermission( - content: AssistantMessageBlock[] - ): PendingToolCall | undefined { - const grantedPermissionBlock = content.find( - (block) => - block.type === 'action' && - block.action_type === 'tool_call_permission' && - block.status === 'granted' - ) - - return this.buildPendingToolCallFromPermissionBlock(grantedPermissionBlock) - } - private buildPendingToolCallFromPermissionBlock( - block?: AssistantMessageBlock - ): PendingToolCall | undefined { - if (!block?.tool_call) { - return undefined - } - - const { id, name, params } = block.tool_call - if (!id || !name || !params) { - console.warn('[PermissionHandler] Incomplete tool call info:', block.tool_call) - return undefined - } - - return { - id, - name, - params, - serverName: block.tool_call.server_name, - serverIcons: block.tool_call.server_icons, - serverDescription: block.tool_call.server_description - } - } - - private buildPendingToolCallFromToolCallId( - content: AssistantMessageBlock[], - toolCallId: string - ): PendingToolCall | undefined { - const toolCallBlock = content.find( - (block) => block.type === 'tool_call' && block.tool_call?.id === toolCallId - ) - - if (!toolCallBlock || toolCallBlock.type !== 'tool_call' || !toolCallBlock.tool_call) { - return undefined - } - - const { id, name, params } = toolCallBlock.tool_call - if (!id || !name || !params) { - console.warn('[PermissionHandler] Incomplete tool call info:', toolCallBlock.tool_call) - return undefined - } - - return { - id, - name, - params, - serverName: toolCallBlock.tool_call.server_name, - serverIcons: toolCallBlock.tool_call.server_icons, - serverDescription: toolCallBlock.tool_call.server_description - } - } - private parsePermissionRequest(block: AssistantMessageBlock): Record | null { const raw = this.getExtraString(block, 'permissionRequest') if (!raw) { @@ -933,4 +1217,32 @@ export class PermissionHandler extends BaseHandler { console.warn('[PermissionHandler] No command found in permission block') return undefined } + + /** + * Notify frontend to refresh permission UI + * Called when a permission is processed and there may be more pending permissions + */ + private notifyFrontendPermissionUpdate(conversationId: string, messageId: string): void { + try { + const pendingPermissions = + presenter.sessionManager.getPendingPermissions(conversationId) ?? [] + const nextPermission = pendingPermissions[0] + console.log('[PermissionHandler] Notifying frontend of permission update:', { + conversationId, + messageId, + remainingCount: pendingPermissions.length, + nextToolCallId: nextPermission?.toolCallId + }) + // Always notify so renderer can clear stale permission UI when count becomes zero. + eventBus.sendToRenderer(STREAM_EVENTS.PERMISSION_UPDATED, SendTarget.ALL_WINDOWS, { + conversationId, + messageId, + type: 'permission_update', + pendingCount: pendingPermissions.length, + nextPermission + }) + } catch (error) { + console.error('[PermissionHandler] Failed to notify frontend:', error) + } + } } diff --git a/src/main/presenter/agentPresenter/session/sessionContext.ts b/src/main/presenter/agentPresenter/session/sessionContext.ts index b8f479321..e9a8cc997 100644 --- a/src/main/presenter/agentPresenter/session/sessionContext.ts +++ b/src/main/presenter/agentPresenter/session/sessionContext.ts @@ -17,6 +17,18 @@ export type SessionContextResolved = { acpWorkdirMap?: Record } +export type PendingPermission = { + messageId: string + toolCallId: string + permissionType: 'read' | 'write' | 'all' | 'command' + payload: unknown +} + +export type PermissionResumeLock = { + messageId: string + startedAt: number +} + export type SessionContext = { sessionId: string agentId: string @@ -29,11 +41,9 @@ export type SessionContext = { currentMessageId?: string toolCallCount: number userStopRequested: boolean - pendingPermission?: { - toolCallId: string - permissionType: 'read' | 'write' | 'all' | 'command' - payload: unknown - } + pendingPermission?: PendingPermission + pendingPermissions?: PendingPermission[] + permissionResumeLock?: PermissionResumeLock pendingQuestion?: { messageId: string toolCallId: string diff --git a/src/main/presenter/agentPresenter/session/sessionManager.ts b/src/main/presenter/agentPresenter/session/sessionManager.ts index 7740e1d3a..3ff4b6672 100644 --- a/src/main/presenter/agentPresenter/session/sessionManager.ts +++ b/src/main/presenter/agentPresenter/session/sessionManager.ts @@ -3,7 +3,12 @@ import path from 'path' import { app } from 'electron' import type { IConfigPresenter, ISessionPresenter } from '@shared/presenter' import type { AssistantMessageBlock } from '@shared/chat' -import type { SessionContext, SessionContextResolved, SessionStatus } from './sessionContext' +import type { + PendingPermission, + SessionContext, + SessionContextResolved, + SessionStatus +} from './sessionContext' import { resolveSessionContext } from './sessionResolver' type WorkspaceContext = { @@ -140,7 +145,11 @@ export class SessionManager { } } - async startLoop(agentId: string, messageId: string): Promise { + async startLoop( + agentId: string, + messageId: string, + options?: { preservePendingPermissions?: boolean; skipLockAcquisition?: boolean } + ): Promise { const session = await this.getSession(agentId) session.status = 'generating' session.updatedAt = Date.now() @@ -149,8 +158,31 @@ export class SessionManager { runtime.currentMessageId = messageId runtime.toolCallCount = 0 runtime.userStopRequested = false - runtime.pendingPermission = undefined + + // CRITICAL: Acquire permission resume lock BEFORE clearing pending permissions + // This ensures atomic state transition during permission resume flow + // skipLockAcquisition is used when PermissionHandler already holds the lock + if (!options?.skipLockAcquisition) { + const hasExistingLock = this.acquirePermissionResumeLock(agentId, messageId) + if (!hasExistingLock) { + console.warn( + `[SessionManager] Lock already exists for message ${messageId}, skipping startLoop initialization` + ) + return + } + } + + // CRITICAL: Only clear pending permissions if not preserving them + // This is essential for multi-tool permission scenarios where we need to + // execute one tool while waiting for approval of others + if (!options?.preservePendingPermissions) { + runtime.pendingPermission = undefined + runtime.pendingPermissions = undefined + } runtime.pendingQuestion = undefined + + // Note: lock is held via permissionResumeLock, will be released in PermissionHandler + // after all tools in this resume batch are processed } setStatus(agentId: string, status: SessionStatus): void { @@ -160,6 +192,12 @@ export class SessionManager { session.updatedAt = Date.now() } + getStatus(agentId: string): SessionStatus | null { + const session = this.sessions.get(agentId) + if (!session) return null + return session.status + } + updateRuntime(agentId: string, updates: Partial): void { const session = this.sessions.get(agentId) if (!session) return @@ -177,13 +215,77 @@ export class SessionManager { } clearPendingPermission(agentId: string): void { - this.updateRuntime(agentId, { pendingPermission: undefined }) + this.updateRuntime(agentId, { pendingPermission: undefined, pendingPermissions: undefined }) } clearPendingQuestion(agentId: string): void { this.updateRuntime(agentId, { pendingQuestion: undefined }) } + addPendingPermission(agentId: string, permission: PendingPermission): void { + const session = this.sessions.get(agentId) + if (!session) return + const runtime = this.ensureRuntime(session) + if (!runtime.pendingPermissions) { + runtime.pendingPermissions = [] + } + const existingIndex = runtime.pendingPermissions.findIndex( + (p) => p.messageId === permission.messageId && p.toolCallId === permission.toolCallId + ) + if (existingIndex >= 0) { + runtime.pendingPermissions[existingIndex] = permission + } else { + runtime.pendingPermissions.push(permission) + } + runtime.pendingPermission = runtime.pendingPermissions[0] + session.updatedAt = Date.now() + } + + removePendingPermission(agentId: string, messageId: string, toolCallId: string): void { + const session = this.sessions.get(agentId) + if (!session) return + const runtime = this.ensureRuntime(session) + if (runtime.pendingPermissions) { + runtime.pendingPermissions = runtime.pendingPermissions.filter( + (p) => !(p.messageId === messageId && p.toolCallId === toolCallId) + ) + runtime.pendingPermission = runtime.pendingPermissions[0] + } + session.updatedAt = Date.now() + } + + getPendingPermissions(agentId: string): PendingPermission[] | undefined { + const session = this.sessions.get(agentId) + if (!session) return undefined + const runtime = this.ensureRuntime(session) + return runtime.pendingPermissions + } + + hasPendingPermissions(agentId: string, messageId?: string): boolean { + const pendingPerms = this.getPendingPermissions(agentId) + if (!pendingPerms || pendingPerms.length === 0) return false + if (messageId) { + return pendingPerms.some((p) => p.messageId === messageId) + } + return true + } + + acquirePermissionResumeLock(agentId: string, messageId: string): boolean { + const session = this.sessions.get(agentId) + if (!session) return false + const runtime = this.ensureRuntime(session) + if (runtime.permissionResumeLock?.messageId === messageId) { + return false + } + runtime.permissionResumeLock = { messageId, startedAt: Date.now() } + session.updatedAt = Date.now() + return true + } + + releasePermissionResumeLock(agentId: string): void { + this.updateRuntime(agentId, { permissionResumeLock: undefined }) + } + private ensureRuntime(session: SessionContext): NonNullable { if (!session.runtime) { session.runtime = { @@ -201,6 +303,31 @@ export class SessionManager { return session.runtime } + getPermissionResumeLock(agentId: string): { messageId: string; startedAt: number } | undefined { + const session = this.sessions.get(agentId) + if (!session) return undefined + const runtime = this.ensureRuntime(session) + return runtime.permissionResumeLock + } + + /** + * Remove a session and clean up all pending state. + * Critical for preventing stale permission data from affecting new sessions. + */ + removeSession(agentId: string): void { + const session = this.sessions.get(agentId) + if (session?.runtime) { + // Clear pending permissions to prevent stale data + session.runtime.pendingPermissions = undefined + session.runtime.pendingPermission = undefined + // Clear permission resume lock + session.runtime.permissionResumeLock = undefined + // Clear pending question + session.runtime.pendingQuestion = undefined + } + this.sessions.delete(agentId) + } + private async hydratePendingQuestion(session: SessionContext): Promise { const runtime = this.ensureRuntime(session) if (runtime.pendingQuestionInitialized) return diff --git a/src/main/presenter/agentPresenter/streaming/llmEventHandler.ts b/src/main/presenter/agentPresenter/streaming/llmEventHandler.ts index c7b76821e..be753e660 100644 --- a/src/main/presenter/agentPresenter/streaming/llmEventHandler.ts +++ b/src/main/presenter/agentPresenter/streaming/llmEventHandler.ts @@ -211,14 +211,13 @@ export class LLMEventHandler { await this.toolCallHandler.processToolCallUpdate(state, msg) break case 'permission-required': - presenter.sessionManager.updateRuntime(state.conversationId, { - pendingPermission: { - toolCallId: tool_call_id || '', - permissionType: - (msg.permission_request?.permissionType as 'read' | 'write' | 'all' | 'command') || - 'read', - payload: msg.permission_request ?? {} - } + presenter.sessionManager.addPendingPermission(state.conversationId, { + messageId: eventId, + toolCallId: tool_call_id || '', + permissionType: + (msg.permission_request?.permissionType as 'read' | 'write' | 'all' | 'command') || + 'read', + payload: msg.permission_request ?? {} }) presenter.sessionManager.setStatus(state.conversationId, 'waiting_permission') try { @@ -664,4 +663,16 @@ export class LLMEventHandler { finalizeLastBlock(state: GeneratingMessageState): void { finalizeAssistantMessageBlocks(state.message.content) } + + /** + * Flush all pending stream updates for a message + * Used during permission resume to ensure UI state is synchronized with DB + */ + async flushStreamUpdates(eventId: string): Promise { + try { + await this.streamUpdateScheduler.flushAll(eventId, 'final') + } catch (error) { + console.error('[LLMEventHandler] Failed to flush stream updates:', error) + } + } } diff --git a/src/main/presenter/agentPresenter/streaming/streamGenerationHandler.ts b/src/main/presenter/agentPresenter/streaming/streamGenerationHandler.ts index 39d8accb2..4a950dafc 100644 --- a/src/main/presenter/agentPresenter/streaming/streamGenerationHandler.ts +++ b/src/main/presenter/agentPresenter/streaming/streamGenerationHandler.ts @@ -59,7 +59,10 @@ export class StreamGenerationHandler extends BaseHandler { try { state.isCancelled = false - await presenter.sessionManager.startLoop(conversationId, state.message.id) + // Normal flow: skip lock acquisition (lock is only for permission resume) + await presenter.sessionManager.startLoop(conversationId, state.message.id, { + skipLockAcquisition: true + }) const { conversation, userMessage, contextMessages } = await this.prepareConversationContext( conversationId, @@ -198,7 +201,10 @@ export class StreamGenerationHandler extends BaseHandler { try { state.isCancelled = false - await presenter.sessionManager.startLoop(conversationId, state.message.id) + // Normal flow: skip lock acquisition (lock is only for permission resume) + await presenter.sessionManager.startLoop(conversationId, state.message.id, { + skipLockAcquisition: true + }) const queryMessage = await this.ctx.messageManager.getMessage(queryMsgId) if (!queryMessage) { diff --git a/src/main/presenter/llmProviderPresenter/baseProvider.ts b/src/main/presenter/llmProviderPresenter/baseProvider.ts index 0d8f646f7..90ec469dc 100644 --- a/src/main/presenter/llmProviderPresenter/baseProvider.ts +++ b/src/main/presenter/llmProviderPresenter/baseProvider.ts @@ -30,7 +30,7 @@ import logger from '@shared/logger' */ export abstract class BaseLLMProvider { // Maximum tool calls limit in a single conversation turn - protected static readonly MAX_TOOL_CALLS = 200 + protected static readonly MAX_TOOL_CALLS = 12800 protected static readonly DEFAULT_MODEL_FETCH_TIMEOUT = 12000 // Increased to 12 seconds as universal default protected provider: LLM_PROVIDER diff --git a/src/main/presenter/mcpPresenter/index.ts b/src/main/presenter/mcpPresenter/index.ts index 105b86662..56c1f991f 100644 --- a/src/main/presenter/mcpPresenter/index.ts +++ b/src/main/presenter/mcpPresenter/index.ts @@ -586,6 +586,29 @@ export class McpPresenter implements IMCPPresenter { return { content: formattedContent, rawData: toolCallResult } } + /** + * Pre-check tool permissions without executing the tool + * Delegates to ToolManager for the actual permission check + */ + async preCheckToolPermission(request: MCPToolCall): Promise<{ + needsPermission: true + toolName: string + serverName: string + permissionType: 'read' | 'write' | 'all' | 'command' + description: string + command?: string + commandSignature?: string + commandInfo?: { + command: string + riskLevel: 'low' | 'medium' | 'high' | 'critical' + suggestion: string + signature?: string + baseCommand?: string + } + } | null> { + return await this.toolManager.preCheckToolPermission(request) + } + async handleSamplingRequest(request: McpSamplingRequestPayload): Promise { if (!request || !request.requestId) { throw new Error('Invalid sampling request: missing requestId') @@ -1230,13 +1253,14 @@ export class McpPresenter implements IMCPPresenter { async grantPermission( serverName: string, permissionType: 'read' | 'write' | 'all', - remember: boolean = false + remember: boolean = false, + conversationId?: string ): Promise { try { console.log( - `[MCP] Granting ${permissionType} permission for server: ${serverName}, remember: ${remember}` + `[MCP] Granting ${permissionType} permission for server: ${serverName}, remember: ${remember}, conversationId: ${conversationId}` ) - await this.toolManager.grantPermission(serverName, permissionType, remember) + await this.toolManager.grantPermission(serverName, permissionType, remember, conversationId) console.log( `[MCP] Successfully granted ${permissionType} permission for server: ${serverName}` ) diff --git a/src/main/presenter/mcpPresenter/toolManager.ts b/src/main/presenter/mcpPresenter/toolManager.ts index 2266558b5..0c380da9d 100644 --- a/src/main/presenter/mcpPresenter/toolManager.ts +++ b/src/main/presenter/mcpPresenter/toolManager.ts @@ -21,6 +21,8 @@ export class ToolManager { private cachedToolDefinitions: MCPToolDefinition[] | null = null private toolNameToTargetMap: Map | null = null + // Session-scoped permission cache: conversationId -> Set of "serverName:permissionType" + private sessionPermissions = new Map>() constructor(configPresenter: IConfigPresenter, serverManager: ServerManager) { this.configPresenter = configPresenter @@ -272,23 +274,33 @@ export class ToolManager { private checkToolPermission( originalToolName: string, serverName: string, - autoApprove: string[] + autoApprove: string[], + conversationId?: string ): boolean { console.log( `[ToolManager] Checking permissions for tool '${originalToolName}' on server '${serverName}' with autoApprove:`, - autoApprove + autoApprove, + `conversationId: ${conversationId}` ) - // 如果有 'all' 权限，则允许所有操作 + const permissionType = this.determinePermissionType(originalToolName) + console.log(`[ToolManager] Tool '${originalToolName}' requires '${permissionType}' permission`) + + // 1. 优先检查 session 级别的内存权限（当前会话自动执行） + if (conversationId && this.checkSessionPermission(conversationId, serverName, permissionType)) { + console.log( + `[ToolManager] Permission granted via session cache: server '${serverName}' has '${permissionType}' permission` + ) + return true + } + + // 2. 检查持久化的 'all' 权限 if (autoApprove.includes('all')) { console.log(`[ToolManager] Permission granted: server '${serverName}' has 'all' permissions`) return true } - const permissionType = this.determinePermissionType(originalToolName) - console.log(`[ToolManager] Tool '${originalToolName}' requires '${permissionType}' permission`) - - // Check if the specific permission type is approved + // 3. 检查持久化的特定权限类型 if (autoApprove.includes(permissionType)) { console.log( `[ToolManager] Permission granted: server '${serverName}' has '${permissionType}' permission` @@ -302,6 +314,73 @@ export class ToolManager { return false } + /** + * Pre-check tool permissions without executing the tool + * Returns permission requirement info if permission is needed, null if already has permission + */ + async preCheckToolPermission(toolCall: MCPToolCall): Promise<{ + needsPermission: true + toolName: string + serverName: string + permissionType: 'read' | 'write' | 'all' | 'command' + description: string + command?: string + commandSignature?: string + commandInfo?: { + command: string + riskLevel: 'low' | 'medium' | 'high' | 'critical' + suggestion: string + signature?: string + baseCommand?: string + } + } | null> { + const finalName = toolCall.function.name + + // Ensure definitions and map are loaded/cached + await this.getAllToolDefinitions() + + if (!this.toolNameToTargetMap) { + console.error('[ToolManager] Tool target map is not available for permission check.') + return null + } + + const targetInfo = this.toolNameToTargetMap.get(finalName) + + if (!targetInfo) { + console.error(`[ToolManager] Tool '${finalName}' not found for permission check.`) + return null + } + + const { originalName } = targetInfo + const toolServerName = targetInfo.client.serverName + + // Get server config to check auto-approve settings + const servers = await this.configPresenter.getMcpServers() + const serverConfig = servers[toolServerName] + const autoApprove = serverConfig?.autoApprove || [] + + // Check permission using existing logic + const hasPermission = this.checkToolPermission( + originalName, + toolServerName, + autoApprove, + toolCall.conversationId + ) + + if (hasPermission) { + return null // Already has permission + } + + const permissionType = this.determinePermissionType(originalName) + return { + needsPermission: true, + toolName: originalName, + serverName: toolServerName, + permissionType, + description: `Allow ${originalName} to perform ${permissionType} operations on ${toolServerName}?` + } + } + async callTool(toolCall: MCPToolCall): Promise { try { const finalName = toolCall.function.name @@ -413,8 +492,13 @@ export class ToolManager { `Checking permissions for tool '${originalName}' on server '${toolServerName}' with autoApprove:`, autoApprove ) - // Use originalName and toolServerName for permission check - const hasPermission = this.checkToolPermission(originalName, toolServerName, autoApprove) + // Use originalName and toolServerName for permission check, pass conversationId for session cache + const hasPermission = this.checkToolPermission( + originalName, + toolServerName, + autoApprove, + toolCall.conversationId + ) if (!hasPermission) { console.warn( @@ -433,6 +517,7 @@ export class ToolManager { toolName: originalName, serverName: toolServerName, permissionType, + conversationId: toolCall.conversationId, description: `Allow ${originalName} to perform ${permissionType} operations on ${toolServerName}?` } } @@ -537,22 +622,71 @@ export class ToolManager { async grantPermission( serverName: string, permissionType: 'read' | 'write' | 'all', - remember: boolean = true + remember: boolean = true, + conversationId?: string ): Promise { console.log( - `[ToolManager] Granting permission: ${permissionType} for server: ${serverName}, remember: ${remember}` + `[ToolManager] Granting permission: ${permissionType} for server: ${serverName}, remember: ${remember}, conversationId: ${conversationId}` ) if (remember) { // Persist to configuration await this.updateServerPermissions(serverName, permissionType) } else { - // Store in temporary session storage - // TODO: Implement temporary permission storage - console.log(`[ToolManager] Temporary permission granted (session-scoped)`) + // Store in temporary session storage (memory only) + if (conversationId) { + const key = `${serverName}:${permissionType}` + const existing = this.sessionPermissions.get(conversationId) ?? new Set() + existing.add(key) + this.sessionPermissions.set(conversationId, existing) + console.log( + `[ToolManager] Session permission stored: ${key} for conversation ${conversationId}` + ) + } else { + console.log(`[ToolManager] Temporary permission granted (no conversationId)`) + } } } + // 检查会话级别的权限 + // 当前会话权限遵循层级：all > write > read + checkSessionPermission( + conversationId: string, + serverName: string, + permissionType: 'read' | 'write' | 'all' + ): boolean { + const sessionPerms = this.sessionPermissions.get(conversationId) + if (!sessionPerms) return false + + const permissionLevelMap: Record<'read' | 'write' | 'all', number> = { + read: 1, + write: 2, + all: 3 + } + const requiredLevel = permissionLevelMap[permissionType] + const prefix = `${serverName}:` + + for (const permKey of sessionPerms) { + if (!permKey.startsWith(prefix)) continue + + const storedPermission = permKey.slice(prefix.length) as 'read' | 'write' | 'all' + const storedLevel = permissionLevelMap[storedPermission] + if (storedLevel >= requiredLevel) { + console.log( + `[ToolManager] Session auto-execute: server '${serverName}' has granted permission '${permKey}' in conversation '${conversationId}', required='${permissionType}'` + ) + return true + } + } + + return false + } + + // 清除会话的临时权限 + clearSessionPermissions(conversationId: string): void { + this.sessionPermissions.delete(conversationId) + } + private async updateServerPermissions( serverName: string, permissionType: 'read' | 'write' | 'all' diff --git a/src/main/presenter/toolPresenter/index.ts b/src/main/presenter/toolPresenter/index.ts index 1a0c2b8af..73b5ec2f5 100644 --- a/src/main/presenter/toolPresenter/index.ts +++ b/src/main/presenter/toolPresenter/index.ts @@ -13,6 +13,32 @@ import { AgentToolManager, type AgentToolCallResult } from '../agentPresenter/ac import { jsonrepair } from 'jsonrepair' import { CommandPermissionService } from '../permission' +interface PreCheckedPermissionResult { + needsPermission: true + toolName: string + serverName: string + permissionType: 'read' | 'write' | 'all' | 'command' + description: string + paths?: string[] + command?: string + commandSignature?: string + commandInfo?: { + command: string + riskLevel: 'low' | 'medium' | 'high' | 'critical' + suggestion: string + signature?: string + baseCommand?: string + } + providerId?: string + requestId?: string + sessionId?: string + agentId?: string + agentName?: string + conversationId?: string + rememberable?: boolean + [key: string]: unknown +} + export interface IToolPresenter { getAllToolDefinitions(context: { enabledMcpTools?: string[] @@ -22,6 +48,7 @@ export interface IToolPresenter { conversationId?: string }): Promise callTool(request: MCPToolCall): Promise<{ content: unknown; rawData: MCPToolResponse }> + preCheckToolPermission?(request: MCPToolCall): Promise buildToolSystemPrompt(context: { conversationId?: string }): string } @@ -157,6 +184,67 @@ export class ToolPresenter implements IToolPresenter { return await this.options.mcpPresenter.callTool(request) } + /** + * Pre-check tool permissions without executing the tool + * Routes to the appropriate source based on tool mapping + */ + async preCheckToolPermission(request: MCPToolCall): Promise { + const toolName = request.function.name + const source = this.mapper.getToolSource(toolName) + + if (!source) { + console.warn(`[ToolPresenter] Tool ${toolName} not found for permission check`) + return null + } + + if (source === 'agent') { + // Agent tools: delegate to AgentToolManager for pre-check + if (!this.agentToolManager) { + return null + } + + let args: Record = {} + const argsString = request.function.arguments || '' + if (argsString.trim().length > 0) { + try { + args = JSON.parse(argsString) as Record + } catch (error) { + console.warn( + '[ToolPresenter] Failed to parse tool arguments for pre-check, trying jsonrepair:', + error + ) + try { + args = JSON.parse(jsonrepair(argsString)) as Record + } catch (error) { + console.warn( + '[ToolPresenter] Failed to repair tool arguments for pre-check, using empty args.', + error + ) + args = {} + } + } + } + + const result = await this.agentToolManager.preCheckToolPermission( + toolName, + args, + request.conversationId + ) + if (!result) { + return null + } + return result + } + + // Route to MCP for permission pre-check + if (this.options.mcpPresenter.preCheckToolPermission) { + return await this.options.mcpPresenter.preCheckToolPermission(request) + } + + // If MCP presenter doesn't support preCheckToolPermission, skip it + return null + } + private resolveAgentToolResponse(response: AgentToolCallResult | string): AgentToolCallResult { if (typeof response === 'string') { return { content: response } diff --git a/src/renderer/src/components/message/MessageBlockPermissionRequest.vue b/src/renderer/src/components/message/MessageBlockPermissionRequest.vue index 064a72a56..09a7c0ae3 100644 --- a/src/renderer/src/components/message/MessageBlockPermissionRequest.vue +++ b/src/renderer/src/components/message/MessageBlockPermissionRequest.vue @@ -459,6 +459,7 @@ const grantPermissionOnce = async () => { } const grantPermissionForSession = async () => { + // Allow for current session: remember=true means session-scoped storage await submitPermission(true, true) } diff --git a/src/renderer/src/events.ts b/src/renderer/src/events.ts index d1f9f4bca..74760f8ee 100644 --- a/src/renderer/src/events.ts +++ b/src/renderer/src/events.ts @@ -49,7 +49,8 @@ export const CONVERSATION_EVENTS = { export const STREAM_EVENTS = { RESPONSE: 'stream:response', // 替代 stream-response END: 'stream:end', // 替代 stream-end - ERROR: 'stream:error' // 替代 stream-error + ERROR: 'stream:error', // 替代 stream-error + PERMISSION_UPDATED: 'stream:permission-updated' // 权限状态更新，通知前端刷新UI } // 应用更新相关事件 diff --git a/src/renderer/src/stores/chat.ts b/src/renderer/src/stores/chat.ts index 499768d39..d14286e64 100644 --- a/src/renderer/src/stores/chat.ts +++ b/src/renderer/src/stores/chat.ts @@ -957,7 +957,23 @@ export const useChatStore = defineStore('chat', () => { // 处理 init 事件：创建骨架消息行 if (stream_kind === 'init') { - if (hasCachedMessage(eventId)) { + if (getGeneratingMessagesCache().has(eventId)) { + return + } + + const existingCachedMessage = getCachedMessage(eventId) + if (existingCachedMessage && existingCachedMessage.role === 'assistant') { + const threadId = + conversationId ?? existingCachedMessage.conversationId ?? getActiveThreadId() ?? '' + if (threadId) { + generatingThreadIds.value.add(threadId) + generatingThreadIds.value = new Set(generatingThreadIds.value) + updateThreadWorkingStatus(threadId, 'working') + getGeneratingMessagesCache().set(eventId, { + message: existingCachedMessage as AssistantMessage, + threadId + }) + } return } @@ -994,6 +1010,15 @@ export const useChatStore = defineStore('chat', () => { if (!skeleton.is_variant) { ensureMessageId(eventId) } + if (skeleton.conversationId) { + generatingThreadIds.value.add(skeleton.conversationId) + generatingThreadIds.value = new Set(generatingThreadIds.value) + updateThreadWorkingStatus(skeleton.conversationId, 'working') + getGeneratingMessagesCache().set(eventId, { + message: skeleton, + threadId: skeleton.conversationId + }) + } return } @@ -1381,6 +1406,33 @@ export const useChatStore = defineStore('chat', () => { // 获取最新的消息并处理 extra 信息 const updatedMessage = await threadP.getMessage(msg.eventId) const enrichedMessage = await enrichMessageWithExtra(updatedMessage) + const assistantContent = (enrichedMessage as AssistantMessage).content + const hasPendingUserAction = + Array.isArray(assistantContent) && + assistantContent.some( + (block) => + block.type === 'action' && + (block.action_type === 'tool_call_permission' || + block.action_type === 'question_request') && + block.status === 'pending' + ) + + // Keep the session in working state while waiting for user permission/question feedback. + if (hasPendingUserAction) { + getGeneratingMessagesCache().set(msg.eventId, { + message: enrichedMessage as AssistantMessage, + threadId: cached.threadId + }) + generatingThreadIds.value.add(cached.threadId) + generatingThreadIds.value = new Set(generatingThreadIds.value) + updateThreadWorkingStatus(cached.threadId, 'working') + + if (getActiveThreadId() === cached.threadId) { + cacheMessageForView(enrichedMessage as AssistantMessage | UserMessage) + ensureMessageId(enrichedMessage.id) + } + return + } getGeneratingMessagesCache().delete(msg.eventId) generatingThreadIds.value.delete(cached.threadId) @@ -2173,6 +2225,7 @@ export const useChatStore = defineStore('chat', () => { window.electron.ipcRenderer.removeAllListeners(STREAM_EVENTS.RESPONSE) window.electron.ipcRenderer.removeAllListeners(STREAM_EVENTS.END) window.electron.ipcRenderer.removeAllListeners(STREAM_EVENTS.ERROR) + window.electron.ipcRenderer.removeAllListeners(STREAM_EVENTS.PERMISSION_UPDATED) window.electron.ipcRenderer.on(STREAM_EVENTS.RESPONSE, (_, msg) => { handleStreamResponse(msg) @@ -2185,6 +2238,16 @@ export const useChatStore = defineStore('chat', () => { window.electron.ipcRenderer.on(STREAM_EVENTS.ERROR, (_, msg) => { handleStreamError(msg) }) + + // 监听权限更新事件，刷新消息以显示最新的权限状态 + window.electron.ipcRenderer.on( + STREAM_EVENTS.PERMISSION_UPDATED, + (_, payload: { messageId: string }) => { + console.log('[Chat Store] Permission updated, refreshing message:', payload.messageId) + // 触发消息缓存刷新，使UI显示下一个待处理的权限 + bumpMessageCacheVersion() + } + ) } return { diff --git a/src/shared/types/presenters/legacy.presenters.d.ts b/src/shared/types/presenters/legacy.presenters.d.ts index a3e0940c9..33c823cf7 100644 --- a/src/shared/types/presenters/legacy.presenters.d.ts +++ b/src/shared/types/presenters/legacy.presenters.d.ts @@ -1619,6 +1619,22 @@ export interface IMCPPresenter { getPrompt(prompt: PromptListEntry, args?: Record): Promise readResource(resource: ResourceListEntry): Promise callTool(request: MCPToolCall): Promise<{ content: string; rawData: MCPToolResponse }> + preCheckToolPermission?(request: MCPToolCall): Promise<{ + needsPermission: true + toolName: string + serverName: string + permissionType: 'read' | 'write' | 'all' | 'command' + description: string + command?: string + commandSignature?: string + commandInfo?: { + command: string + riskLevel: 'low' | 'medium' | 'high' | 'critical' + suggestion: string + signature?: string + baseCommand?: string + } + } | null> handleSamplingRequest(request: McpSamplingRequestPayload): Promise submitSamplingDecision(decision: McpSamplingDecision): Promise cancelSamplingRequest(requestId: string, reason?: string): Promise @@ -1630,7 +1646,8 @@ export interface IMCPPresenter { grantPermission( serverName: string, permissionType: 'read' | 'write' | 'all', - remember?: boolean + remember?: boolean, + conversationId?: string ): Promise // NPM Registry management methods getNpmRegistryStatus?(): Promise<{ diff --git a/test/main/presenter/agentPresenter/acp/agentFileSystemHandler.test.ts b/test/main/presenter/agentPresenter/acp/agentFileSystemHandler.test.ts index 579c34a67..2ff651940 100644 --- a/test/main/presenter/agentPresenter/acp/agentFileSystemHandler.test.ts +++ b/test/main/presenter/agentPresenter/acp/agentFileSystemHandler.test.ts @@ -2,7 +2,7 @@ import { describe, it, expect, beforeEach, afterEach } from 'vitest' import * as fs from 'fs/promises' import * as path from 'path' import * as os from 'os' -import { AgentFileSystemHandler } from '@/presenter/agentPresenter/acp' +import { AgentFileSystemHandler } from '@/presenter/agentPresenter/acp/agentFileSystemHandler' describe('AgentFileSystemHandler diff responses', () => { let testDir: string @@ -134,4 +134,28 @@ describe('AgentFileSystemHandler diff responses', () => { it('rejects directoryTree depth above max', async () => { await expect(handler.directoryTree({ path: testDir, depth: 4 })).rejects.toThrow() }) + + it('normalizes line endings when matching oldText in editFile', async () => { + const filePath = path.join(testDir, 'crlf.txt') + await fs.writeFile(filePath, 'line1\r\nline2\r\n', 'utf-8') + + const responseText = await handler.editFile({ + path: filePath, + oldText: 'line1\nline2\n', + newText: 'line1\nline2-updated\n' + }) + + const response = JSON.parse(responseText) as { + success: boolean + replacements: number + updatedCode: string + } + + expect(response.success).toBe(true) + expect(response.replacements).toBe(1) + expect(response.updatedCode).toContain('line2-updated') + + const updatedContent = await fs.readFile(filePath, 'utf-8') + expect(updatedContent).toContain('line2-updated') + }) }) diff --git a/test/main/presenter/agentPresenter/loop/toolCallProcessor.test.ts b/test/main/presenter/agentPresenter/loop/toolCallProcessor.test.ts index 556ef1d53..5b5bd9a5a 100644 --- a/test/main/presenter/agentPresenter/loop/toolCallProcessor.test.ts +++ b/test/main/presenter/agentPresenter/loop/toolCallProcessor.test.ts @@ -3,7 +3,7 @@ import fs from 'fs/promises' import os from 'os' import path from 'path' import { app } from 'electron' -import { ToolCallProcessor } from '@/presenter/agentPresenter/loop' +import { ToolCallProcessor } from '@/presenter/agentPresenter/loop/toolCallProcessor' import type { ChatMessage, MCPToolDefinition, @@ -11,6 +11,14 @@ import type { ModelConfig } from '@shared/presenter' +vi.mock('@/presenter', () => ({ + presenter: { + hooksNotifications: { + dispatchEvent: vi.fn() + } + } +})) + describe('ToolCallProcessor tool output offload', () => { let tempHome: string let getPathSpy: ReturnType @@ -99,7 +107,7 @@ describe('ToolCallProcessor question tool', () => { const questionToolDef = { type: 'function', function: { - name: 'question', + name: 'deepchat_question', description: 'question tool', parameters: { type: 'object', @@ -145,7 +153,7 @@ describe('ToolCallProcessor question tool', () => { const conversationMessages: ChatMessage[] = [{ role: 'assistant', content: 'hello' }] const iterator = processor.process({ eventId: 'event-question-1', - toolCalls: [{ id: 'tool-q1', name: 'question', arguments: basicQuestionArgs }], + toolCalls: [{ id: 'tool-q1', name: 'deepchat_question', arguments: basicQuestionArgs }], enabledMcpTools: [], conversationMessages, modelConfig: { functionCall: true } as ModelConfig, @@ -189,7 +197,7 @@ describe('ToolCallProcessor question tool', () => { const iterator = processor.process({ eventId: 'event-question-2', toolCalls: [ - { id: 'tool-q1', name: 'question', arguments: basicQuestionArgs }, + { id: 'tool-q1', name: 'deepchat_question', arguments: basicQuestionArgs }, { id: 'tool-2', name: 'execute_command', arguments: '{}' } ], enabledMcpTools: [], @@ -220,3 +228,83 @@ describe('ToolCallProcessor question tool', () => { expect(callTool).toHaveBeenCalled() }) }) + +describe('ToolCallProcessor batch permission pre-check', () => { + it('preserves extended permission payload fields for batch permission requests', async () => { + const toolDefinition = { + type: 'function', + function: { + name: 'edit_file', + description: 'edit file', + parameters: { + type: 'object', + properties: {} + } + }, + server: { + name: 'agent-filesystem', + icons: '📁', + description: 'Agent filesystem' + } + } as MCPToolDefinition + + const callTool = vi.fn(async () => ({ + content: 'ok', + rawData: { content: 'ok' } as MCPToolResponse + })) + const preCheckToolPermission = vi.fn(async () => ({ + needsPermission: true as const, + toolName: 'edit_file', + serverName: 'agent-filesystem', + permissionType: 'write' as const, + description: 'Write access requires approval', + paths: ['src/main.ts'], + customMeta: { + source: 'batch-precheck' + } + })) + + const processor = new ToolCallProcessor({ + getAllToolDefinitions: async () => [toolDefinition], + callTool, + preCheckToolPermission + }) + + const conversationMessages: ChatMessage[] = [{ role: 'assistant', content: 'hello' }] + const iterator = processor.process({ + eventId: 'event-batch-permission', + toolCalls: [{ id: 'tool-1', name: 'edit_file', arguments: '{"path":"src/main.ts"}' }], + enabledMcpTools: [], + conversationMessages, + modelConfig: { functionCall: true } as ModelConfig, + abortSignal: new AbortController().signal, + currentToolCallCount: 0, + maxToolCalls: 5, + conversationId: 'conv-batch-permission' + }) + + const events: any[] = [] + let result: any = null + while (true) { + const { value, done } = await iterator.next() + if (done) { + result = value + break + } + events.push(value) + } + + const permissionEvent = events.find( + (event) => event.type === 'response' && event.data?.tool_call === 'permission-required' + ) + expect(permissionEvent).toBeDefined() + expect(permissionEvent.data.permission_request.paths).toEqual(['src/main.ts']) + expect(permissionEvent.data.permission_request.customMeta).toEqual({ + source: 'batch-precheck' + }) + expect(permissionEvent.data.permission_request.isBatchPermission).toBe(true) + expect(permissionEvent.data.permission_request.totalInBatch).toBe(1) + expect(callTool).not.toHaveBeenCalled() + expect(result.needContinueConversation).toBe(false) + }) +}) diff --git a/test/main/presenter/agentPresenter/permission/permissionHandler.resume.test.ts b/test/main/presenter/agentPresenter/permission/permissionHandler.resume.test.ts new file mode 100644 index 000000000..7958b821b --- /dev/null +++ b/test/main/presenter/agentPresenter/permission/permissionHandler.resume.test.ts @@ -0,0 +1,162 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { AssistantMessage, AssistantMessageBlock } from '@shared/chat' +import type { ILlmProviderPresenter, IMCPPresenter, IToolPresenter } from '@shared/presenter' +import { PermissionHandler } from '@/presenter/agentPresenter/permission/permissionHandler' +import type { ThreadHandlerContext } from '@/presenter/searchPresenter/handlers/baseHandler' +import type { MessageManager } from '@/presenter/sessionPresenter/managers/messageManager' +import type { SearchManager } from '@/presenter/searchPresenter/managers/searchManager' +import type { StreamGenerationHandler } from '@/presenter/agentPresenter/streaming/streamGenerationHandler' +import type { LLMEventHandler } from '@/presenter/agentPresenter/streaming/llmEventHandler' +import type { GeneratingMessageState } from '@/presenter/agentPresenter/streaming/types' +import { CommandPermissionService } from '@/presenter/permission' + +const presenterMock = vi.hoisted(() => ({ + sessionManager: { + removePendingPermission: vi.fn(), + acquirePermissionResumeLock: vi.fn(), + releasePermissionResumeLock: vi.fn(), + getPendingPermissions: vi.fn().mockReturnValue([]), + clearPendingPermission: vi.fn(), + setStatus: vi.fn() + }, + filePermissionService: { + approve: vi.fn() + } +})) + +vi.mock('@/presenter', () => ({ + presenter: presenterMock +})) + +describe('PermissionHandler resume behavior', () => { + beforeEach(() => { + vi.clearAllMocks() + presenterMock.sessionManager.acquirePermissionResumeLock.mockReturnValue(true) + }) + + it('resumes all resolved tool calls after the final permission decision', async () => { + const conversationId = 'conv-resume' + const messageId = 'msg-resume' + let content: AssistantMessageBlock[] = [ + { + type: 'tool_call', + status: 'loading', + timestamp: Date.now(), + tool_call: { id: 'tool-1', name: 'read_file', params: '{}' } + }, + { + type: 'action', + action_type: 'tool_call_permission', + status: 'pending', + timestamp: Date.now(), + content: 'Permission required', + tool_call: { id: 'tool-1', name: 'read_file', params: '{}' }, + extra: { + needsUserAction: true, + serverName: 'agent-filesystem', + permissionType: 'read', + permissionRequest: JSON.stringify({ + toolName: 'read_file', + serverName: 'agent-filesystem', + permissionType: 'read', + description: 'Read access requires approval', + paths: ['src/a.txt'] + }) + } + }, + { + type: 'tool_call', + status: 'loading', + timestamp: Date.now(), + tool_call: { id: 'tool-2', name: 'read_file', params: '{}' } + }, + { + type: 'action', + action_type: 'tool_call_permission', + status: 'pending', + timestamp: Date.now(), + content: 'Permission required', + tool_call: { id: 'tool-2', name: 'read_file', params: '{}' }, + extra: { + needsUserAction: true, + serverName: 'agent-filesystem', + permissionType: 'read', + permissionRequest: JSON.stringify({ + toolName: 'read_file', + serverName: 'agent-filesystem', + permissionType: 'read', + description: 'Read access requires approval', + paths: ['src/b.txt'] + }) + } + } + ] + + const getCurrentMessage = (): AssistantMessage => + ({ + id: messageId, + conversationId, + role: 'assistant', + content + }) as AssistantMessage + + const messageManager = { + getMessage: vi.fn(async () => getCurrentMessage()), + editMessage: vi.fn(async (_id: string, nextContent: string) => { + content = JSON.parse(nextContent) as AssistantMessageBlock[] + return getCurrentMessage() + }), + handleMessageError: vi.fn() + } as unknown as MessageManager + + const ctx: ThreadHandlerContext = { + sqlitePresenter: {} as never, + messageManager, + llmProviderPresenter: {} as never, + configPresenter: {} as never, + searchManager: {} as SearchManager + } + + const generatingMessages = new Map() + generatingMessages.set(messageId, { + message: getCurrentMessage(), + conversationId, + startTime: Date.now(), + firstTokenTime: null, + promptTokens: 0, + reasoningStartTime: null, + reasoningEndTime: null, + lastReasoningTime: null + }) + + const handler = new PermissionHandler(ctx, { + generatingMessages, + llmProviderPresenter: {} as ILlmProviderPresenter, + getMcpPresenter: () => ({ grantPermission: vi.fn() }) as unknown as IMCPPresenter, + getToolPresenter: () => + ({ + getAllToolDefinitions: vi.fn(), + callTool: vi.fn(), + buildToolSystemPrompt: vi.fn() + }) as unknown as IToolPresenter, + streamGenerationHandler: {} as StreamGenerationHandler, + llmEventHandler: {} as LLMEventHandler, + commandPermissionHandler: new CommandPermissionService() + }) + + const resumeSpy = vi + .spyOn( + handler as unknown as { resumeToolExecutionAfterPermissions: (...args: any[]) => any }, + 'resumeToolExecutionAfterPermissions' + ) + .mockResolvedValue(undefined) + + await handler.handlePermissionResponse(messageId, 'tool-1', true, 'read', false) + expect(resumeSpy).not.toHaveBeenCalled() + + await handler.handlePermissionResponse(messageId, 'tool-2', true, 'read', false) + expect(resumeSpy).toHaveBeenCalledTimes(1) + expect(resumeSpy).toHaveBeenCalledWith(messageId, true) + expect(resumeSpy.mock.calls[0][2]).toBeUndefined() + }) +}) diff --git a/test/main/presenter/sessionPresenter/permissionHandler.test.ts b/test/main/presenter/sessionPresenter/permissionHandler.test.ts index 8b1e8603a..d8ed1b3d6 100644 --- a/test/main/presenter/sessionPresenter/permissionHandler.test.ts +++ b/test/main/presenter/sessionPresenter/permissionHandler.test.ts @@ -1,25 +1,111 @@ -import { describe, expect, it, vi } from 'vitest' +import { beforeEach, describe, expect, it, vi } from 'vitest' import type { AssistantMessage, AssistantMessageBlock } from '@shared/chat' import type { ILlmProviderPresenter, IMCPPresenter, IToolPresenter } from '@shared/presenter' import { PermissionHandler } from '@/presenter/agentPresenter/permission/permissionHandler' import { CommandPermissionService } from '@/presenter/permission' +import { presenter } from '@/presenter' import type { ThreadHandlerContext } from '@/presenter/searchPresenter/handlers/baseHandler' -import type { StreamGenerationHandler } from '@/presenter/sessionPresenter/streaming/streamGenerationHandler' -import type { LLMEventHandler } from '@/presenter/sessionPresenter/streaming/llmEventHandler' +import type { StreamGenerationHandler } from '@/presenter/agentPresenter/streaming/streamGenerationHandler' +import type { LLMEventHandler } from '@/presenter/agentPresenter/streaming/llmEventHandler' import type { MessageManager } from '@/presenter/sessionPresenter/managers/messageManager' import type { SearchManager } from '@/presenter/searchPresenter/managers/searchManager' -import type { GeneratingMessageState } from '@/presenter/sessionPresenter/streaming/types' +import type { GeneratingMessageState } from '@/presenter/agentPresenter/streaming/types' + +const sessionState = vi.hoisted(() => ({ + pendingPermissions: new Map< + string, + Array<{ + messageId: string + toolCallId: string + permissionType: 'read' | 'write' | 'all' | 'command' + payload: Record + }> + >(), + locks: new Map(), + status: new Map(), + sessions: new Map() +})) -vi.mock('@/presenter', () => ({ - presenter: { - sessionManager: { - clearPendingPermission: vi.fn(), - setStatus: vi.fn(), - startLoop: vi.fn() - } +const presenterMock = vi.hoisted(() => ({ + sessionManager: { + clearPendingPermission: vi.fn((agentId: string) => { + sessionState.pendingPermissions.delete(agentId) + }), + setStatus: vi.fn((agentId: string, status: string) => { + sessionState.status.set(agentId, status) + }), + getStatus: vi.fn((agentId: string) => { + return ( + (sessionState.status.get(agentId) as + | 'idle' + | 'generating' + | 'waiting_permission' + | 'waiting_question' + | null) ?? 'waiting_permission' + ) + }), + startLoop: vi.fn().mockResolvedValue(undefined), + removePendingPermission: vi.fn((agentId: string, messageId: string, toolCallId: string) => { + const pending = sessionState.pendingPermissions.get(agentId) ?? [] + sessionState.pendingPermissions.set( + agentId, + pending.filter((item) => !(item.messageId === messageId && item.toolCallId === toolCallId)) + ) + }), + addPendingPermission: vi.fn( + ( + agentId: string, + permission: { + messageId: string + toolCallId: string + permissionType: 'read' | 'write' | 'all' | 'command' + payload: Record + } + ) => { + const pending = sessionState.pendingPermissions.get(agentId) ?? [] + pending.push(permission) + sessionState.pendingPermissions.set(agentId, pending) + } + ), + getPendingPermissions: vi.fn((agentId: string) => { + return sessionState.pendingPermissions.get(agentId) ?? [] + }), + hasPendingPermissions: vi.fn((agentId: string, messageId?: string) => { + const pending = sessionState.pendingPermissions.get(agentId) ?? [] + if (!messageId) { + return pending.length > 0 + } + return pending.some((item) => item.messageId === messageId) + }), + acquirePermissionResumeLock: vi.fn((agentId: string, messageId: string) => { + if (sessionState.locks.get(agentId)?.messageId === messageId) { + return false + } + sessionState.locks.set(agentId, { messageId, startedAt: Date.now() }) + return true + }), + getPermissionResumeLock: vi.fn((agentId: string) => { + return sessionState.locks.get(agentId) + }), + releasePermissionResumeLock: vi.fn((agentId: string) => { + sessionState.locks.delete(agentId) + }), + getSessionSync: vi.fn((agentId: string) => { + return sessionState.sessions.get(agentId) ?? null + }) + }, + filePermissionService: { + approve: vi.fn() + }, + settingsPermissionService: { + approve: vi.fn() } })) +vi.mock('@/presenter', () => ({ + presenter: presenterMock +})) + const createAssistantMessage = ( blocks: AssistantMessageBlock[], conversationId: string, @@ -33,246 +119,272 @@ const createAssistantMessage = ( } as AssistantMessage } -describe('PermissionHandler - ACP permissions', () => { - const createHandler = (overrides?: { - generatingState?: Partial - block?: AssistantMessageBlock - resolveAgentPermission?: ReturnType - }) => { - const conversationId = 'conv-1' - const messageId = 'msg-1' - const toolCallId = 'tool-1' - const permissionRequest = { - providerId: 'acp', - requestId: 'req-123' - } - - const block: AssistantMessageBlock = - overrides?.block || +const createPermissionHandler = (options: { + message: AssistantMessage + llmProviderPresenter?: ILlmProviderPresenter +}) => { + let currentMessage = options.message + const messageManager = { + getMessage: vi.fn().mockImplementation(async () => currentMessage), + editMessage: vi.fn().mockImplementation(async (_id: string, rawContent: string) => { + const next = JSON.parse(rawContent) as AssistantMessageBlock[] + currentMessage = { + ...currentMessage, + content: next + } + return currentMessage + }), + handleMessageError: vi.fn() + } as unknown as MessageManager + + const ctx: ThreadHandlerContext = { + sqlitePresenter: {} as never, + messageManager, + llmProviderPresenter: options.llmProviderPresenter ?? ({} as ILlmProviderPresenter), + configPresenter: {} as never, + searchManager: {} as SearchManager + } + + const generatingMessages = new Map() + generatingMessages.set(currentMessage.id, { + message: currentMessage, + conversationId: currentMessage.conversationId, + startTime: Date.now(), + firstTokenTime: null, + promptTokens: 0, + reasoningStartTime: null, + reasoningEndTime: null, + lastReasoningTime: null + }) + + const handler = new PermissionHandler(ctx, { + generatingMessages, + llmProviderPresenter: options.llmProviderPresenter ?? ({} as ILlmProviderPresenter), + getMcpPresenter: () => ({ + grantPermission: vi.fn(), + isServerRunning: vi.fn().mockResolvedValue(true) + }) as unknown as IMCPPresenter, + getToolPresenter: () => + ({ + getAllToolDefinitions: vi.fn(), + callTool: vi.fn(), + buildToolSystemPrompt: vi.fn() + }) as unknown as IToolPresenter, + streamGenerationHandler: {} as StreamGenerationHandler, + llmEventHandler: {} as LLMEventHandler, + commandPermissionHandler: new CommandPermissionService() + }) + + return { handler, messageManager, getCurrentMessage: () => currentMessage } +} + +describe('PermissionHandler', () => { + beforeEach(() => { + vi.clearAllMocks() + sessionState.pendingPermissions.clear() + sessionState.locks.clear() + sessionState.status.clear() + sessionState.sessions.clear() + }) + + describe('ACP permissions', () => { + it('routes granted permissions through llmProviderPresenter for ACP blocks', async () => { + const conversationId = 'conv-1' + const messageId = 'msg-1' + const toolCallId = 'tool-1' + sessionState.sessions.set(conversationId, { id: conversationId }) + + const permissionBlock = { type: 'action', action_type: 'tool_call_permission', status: 'pending', timestamp: Date.now(), content: 'components.messageBlockPermissionRequest.description.write', - tool_call: { - id: toolCallId, - name: 'acp-tool' - }, + tool_call: { id: toolCallId, name: 'acp-tool' }, extra: { needsUserAction: true, providerId: 'acp', - permissionRequestId: permissionRequest.requestId, - permissionRequest: JSON.stringify(permissionRequest), + permissionRequestId: 'req-123', + permissionRequest: JSON.stringify({ providerId: 'acp', requestId: 'req-123' }), permissionType: 'write' } - } as AssistantMessageBlock) - - const assistantMessage = createAssistantMessage([block], conversationId, messageId) - - const messageManager = { - getMessage: vi.fn().mockResolvedValue(assistantMessage), - editMessage: vi.fn().mockResolvedValue(assistantMessage), - handleMessageError: vi.fn() - } as unknown as MessageManager - - const llmProviderPresenter = { - resolveAgentPermission: - overrides?.resolveAgentPermission || vi.fn().mockResolvedValue(undefined) - } as unknown as ILlmProviderPresenter - - const ctx: ThreadHandlerContext = { - sqlitePresenter: {} as never, - messageManager, - llmProviderPresenter, - configPresenter: {} as never, - searchManager: {} as SearchManager - } - - const generatingMessages = new Map() - generatingMessages.set(messageId, { - message: assistantMessage, - conversationId, - startTime: Date.now(), - firstTokenTime: null, - promptTokens: 0, - reasoningStartTime: null, - reasoningEndTime: null, - lastReasoningTime: null, - ...(overrides?.generatingState || {}) - }) + } as AssistantMessageBlock - const permissionHandler = new PermissionHandler(ctx, { - generatingMessages, - llmProviderPresenter, - getMcpPresenter: () => ({ grantPermission: vi.fn() }) as unknown as IMCPPresenter, - getToolPresenter: () => - ({ - getAllToolDefinitions: vi.fn(), - callTool: vi.fn(), - buildToolSystemPrompt: vi.fn() - }) as unknown as IToolPresenter, - streamGenerationHandler: {} as StreamGenerationHandler, - llmEventHandler: {} as LLMEventHandler, - commandPermissionHandler: new CommandPermissionService() - }) + const message = createAssistantMessage([permissionBlock], conversationId, messageId) + const llmProviderPresenter = { + resolveAgentPermission: vi.fn().mockResolvedValue(undefined) + } as unknown as ILlmProviderPresenter - return { - handler: permissionHandler, - block, - conversationId, - messageId, - toolCallId, - llmProviderPresenter, - messageManager, - generatingMessages - } - } + const { handler } = createPermissionHandler({ + message, + llmProviderPresenter + }) - it('routes granted permissions through llmProviderPresenter for ACP blocks', async () => { - const { handler, messageId, toolCallId, llmProviderPresenter } = createHandler() + await handler.handlePermissionResponse(messageId, toolCallId, true, 'write', false) - await handler.handlePermissionResponse(messageId, toolCallId, true, 'write', false) + expect(llmProviderPresenter.resolveAgentPermission).toHaveBeenCalledWith('req-123', true) + expect(presenter.sessionManager.clearPendingPermission).toHaveBeenCalledWith(conversationId) + expect(presenter.sessionManager.setStatus).toHaveBeenCalledWith(conversationId, 'generating') + }) - expect(llmProviderPresenter.resolveAgentPermission).toHaveBeenCalledWith('req-123', true) - }) + it('routes denied permissions through llmProviderPresenter for ACP blocks', async () => { + const conversationId = 'conv-1' + const messageId = 'msg-1' + const toolCallId = 'tool-1' + sessionState.sessions.set(conversationId, { id: conversationId }) - it('routes denied permissions through llmProviderPresenter for ACP blocks', async () => { - const { handler, messageId, toolCallId, llmProviderPresenter } = createHandler() + const permissionBlock = { + type: 'action', + action_type: 'tool_call_permission', + status: 'pending', + timestamp: Date.now(), + content: 'components.messageBlockPermissionRequest.description.write', + tool_call: { id: toolCallId, name: 'acp-tool' }, + extra: { + needsUserAction: true, + providerId: 'acp', + permissionRequestId: 'req-123', + permissionRequest: JSON.stringify({ providerId: 'acp', requestId: 'req-123' }), + permissionType: 'write' + } + } as AssistantMessageBlock + + const message = createAssistantMessage([permissionBlock], conversationId, messageId) + const llmProviderPresenter = { + resolveAgentPermission: vi.fn().mockResolvedValue(undefined) + } as unknown as ILlmProviderPresenter + + const { handler } = createPermissionHandler({ + message, + llmProviderPresenter + }) - await handler.handlePermissionResponse(messageId, toolCallId, false, 'write', false) + await handler.handlePermissionResponse(messageId, toolCallId, false, 'write', false) - expect(llmProviderPresenter.resolveAgentPermission).toHaveBeenCalledWith('req-123', false) + expect(llmProviderPresenter.resolveAgentPermission).toHaveBeenCalledWith('req-123', false) + }) }) -}) -describe('PermissionHandler - permission block removal', () => { - const createRemovalHandler = () => { - const conversationId = 'conv-2' - const messageId = 'msg-2' - const toolCallId = 'tool-2' - - const toolCallBlock: AssistantMessageBlock = { - type: 'tool_call', - status: 'loading', - timestamp: Date.now(), - tool_call: { - id: toolCallId, - name: 'writeFile', - params: '{"path":"/tmp/test.txt"}' + describe('Session manager helpers', () => { + it('stores and filters pending permissions by messageId', () => { + const sessionManager = presenter.sessionManager as unknown as { + addPendingPermission: (agentId: string, permission: any) => void + getPendingPermissions: (agentId: string) => Array<{ toolCallId: string }> + hasPendingPermissions: (agentId: string, messageId?: string) => boolean } - } - - const permissionBlock: AssistantMessageBlock = { - type: 'action', - action_type: 'tool_call_permission', - status: 'pending', - timestamp: Date.now(), - content: 'Permission required', - tool_call: { - id: toolCallId, - name: 'writeFile', - params: '{"path":"/tmp/test.txt"}', - server_name: 'filesystem', - server_description: 'Local file system access' - }, - extra: { - needsUserAction: true - } - } - - const assistantMessage = createAssistantMessage( - [toolCallBlock, permissionBlock], - conversationId, - messageId - ) - const generatingMessage = createAssistantMessage( - [toolCallBlock, permissionBlock], - conversationId, - messageId - ) - - const messageManager = { - getMessage: vi.fn().mockResolvedValue(assistantMessage), - editMessage: vi.fn().mockResolvedValue(assistantMessage), - handleMessageError: vi.fn() - } as unknown as MessageManager - - const ctx: ThreadHandlerContext = { - sqlitePresenter: {} as never, - messageManager, - llmProviderPresenter: {} as never, - configPresenter: {} as never, - searchManager: {} as SearchManager - } - - const generatingMessages = new Map() - generatingMessages.set(messageId, { - message: generatingMessage, - conversationId, - startTime: Date.now(), - firstTokenTime: null, - promptTokens: 0, - reasoningStartTime: null, - reasoningEndTime: null, - lastReasoningTime: null + + sessionManager.addPendingPermission('agent-1', { + messageId: 'msg-1', + toolCallId: 'tool-1', + permissionType: 'read', + payload: {} + }) + sessionManager.addPendingPermission('agent-1', { + messageId: 'msg-2', + toolCallId: 'tool-2', + permissionType: 'write', + payload: {} + }) + + const pending = sessionManager.getPendingPermissions('agent-1') + expect(pending).toHaveLength(2) + expect(sessionManager.hasPendingPermissions('agent-1', 'msg-1')).toBe(true) + expect(sessionManager.hasPendingPermissions('agent-1', 'msg-3')).toBe(false) }) - const permissionHandler = new PermissionHandler(ctx, { - generatingMessages, - llmProviderPresenter: {} as ILlmProviderPresenter, - getMcpPresenter: () => ({ grantPermission: vi.fn() }) as unknown as IMCPPresenter, - getToolPresenter: () => - ({ - getAllToolDefinitions: vi.fn(), - callTool: vi.fn(), - buildToolSystemPrompt: vi.fn() - }) as unknown as IToolPresenter, - streamGenerationHandler: {} as StreamGenerationHandler, - llmEventHandler: {} as LLMEventHandler, - commandPermissionHandler: new CommandPermissionService() + it('acquires and releases permission resume lock', () => { + const sessionManager = presenter.sessionManager as unknown as { + acquirePermissionResumeLock: (agentId: string, messageId: string) => boolean + getPermissionResumeLock: ( + agentId: string + ) => { messageId: string; startedAt: number } | undefined + releasePermissionResumeLock: (agentId: string) => void + } + + const firstAcquire = sessionManager.acquirePermissionResumeLock('agent-1', 'msg-1') + const secondAcquire = sessionManager.acquirePermissionResumeLock('agent-1', 'msg-1') + const lock = sessionManager.getPermissionResumeLock('agent-1') + + expect(firstAcquire).toBe(true) + expect(secondAcquire).toBe(false) + expect(lock?.messageId).toBe('msg-1') + expect(lock?.startedAt).toBeGreaterThan(0) + + sessionManager.releasePermissionResumeLock('agent-1') + expect(sessionManager.getPermissionResumeLock('agent-1')).toBeUndefined() }) + }) - return { - handler: permissionHandler, - messageManager, - generatingMessages, - messageId, - toolCallId - } - } + describe('Permission level hierarchy', () => { + it('updates same tool-call permission blocks with same or lower required levels', async () => { + const conversationId = 'conv-2' + const messageId = 'msg-2' + const toolCallId = 'tool-1' + sessionState.sessions.set(conversationId, { id: conversationId }) - it('removes permission blocks and updates tool_call blocks after resolution', async () => { - const { handler, messageManager, generatingMessages, messageId, toolCallId } = - createRemovalHandler() - vi.spyOn(handler, 'continueAfterPermissionDenied').mockResolvedValue() - - await handler.handlePermissionResponse(messageId, toolCallId, false, 'command', false) - - const updatedContent = JSON.parse( - (messageManager.editMessage as unknown as { mock: { calls: Array<[string, string]> } }).mock - .calls[0][1] - ) as AssistantMessageBlock[] - - const hasPermissionBlock = updatedContent.some( - (block) => block.type === 'action' && block.action_type === 'tool_call_permission' - ) - expect(hasPermissionBlock).toBe(false) - - const updatedToolCall = updatedContent.find( - (block) => block.type === 'tool_call' && block.tool_call?.id === toolCallId - ) - expect(updatedToolCall?.tool_call?.server_name).toBe('filesystem') - - const generatingContent = generatingMessages.get(messageId)?.message.content ?? [] - const hasGeneratingPermissionBlock = generatingContent.some( - (block) => block.type === 'action' && block.action_type === 'tool_call_permission' - ) - expect(hasGeneratingPermissionBlock).toBe(false) - - const generatingToolCall = generatingContent.find( - (block) => block.type === 'tool_call' && block.tool_call?.id === toolCallId - ) - expect(generatingToolCall?.tool_call?.server_name).toBe('filesystem') + const readPermissionBlock: AssistantMessageBlock = { + type: 'action', + action_type: 'tool_call_permission', + status: 'pending', + timestamp: Date.now(), + content: 'Permission required', + tool_call: { + id: toolCallId, + name: 'write_file', + params: '{"path":"a.txt"}' + }, + extra: { + needsUserAction: true, + serverName: 'mock-server', + permissionType: 'read' + } + } + + const writePermissionBlock: AssistantMessageBlock = { + type: 'action', + action_type: 'tool_call_permission', + status: 'pending', + timestamp: Date.now(), + content: 'Permission required', + tool_call: { + id: toolCallId, + name: 'write_file', + params: '{"path":"a.txt"}' + }, + extra: { + needsUserAction: true, + serverName: 'mock-server', + permissionType: 'write' + } + } + + const message = createAssistantMessage( + [readPermissionBlock, writePermissionBlock], + conversationId, + messageId + ) + const { handler, messageManager } = createPermissionHandler({ message }) + vi.spyOn( + handler as unknown as { resumeToolExecutionAfterPermissions: (...args: any[]) => any }, + 'resumeToolExecutionAfterPermissions' + ).mockResolvedValue(undefined) + + await handler.handlePermissionResponse(messageId, toolCallId, true, 'write', false) + + const updatedContent = JSON.parse( + (messageManager.editMessage as unknown as { mock: { calls: Array<[string, string]> } }).mock + .calls[0][1] + ) as AssistantMessageBlock[] + + const updatedPermissionBlocks = updatedContent.filter( + (block) => block.type === 'action' && block.action_type === 'tool_call_permission' + ) + expect(updatedPermissionBlocks).toHaveLength(2) + expect(updatedPermissionBlocks.every((block) => block.status === 'granted')).toBe(true) + expect(updatedPermissionBlocks.every((block) => block.extra?.needsUserAction === false)).toBe( + true + ) + }) }) })