diff --git a/src/main/ipc/teams.ts b/src/main/ipc/teams.ts index 2eee6d42..61c347ce 100644 --- a/src/main/ipc/teams.ts +++ b/src/main/ipc/teams.ts @@ -1,6 +1,6 @@ import { setCurrentMainOp } from '@main/services/infrastructure/EventLoopLagMonitor'; import { getAppIconPath } from '@main/utils/appIcon'; -import { getAppDataPath } from '@main/utils/pathDecoder'; +import { getAppDataPath, getTeamsBasePath } from '@main/utils/pathDecoder'; import { stripMarkdown } from '@main/utils/textFormatting'; import { TEAM_ADD_MEMBER, @@ -48,8 +48,11 @@ import { TEAM_SOFT_DELETE_TASK, TEAM_START_TASK, TEAM_STOP, + TEAM_TOOL_APPROVAL_READ_FILE, TEAM_TOOL_APPROVAL_RESPOND, TEAM_TOOL_APPROVAL_SETTINGS, + TEAM_GET_SAVED_REQUEST, + TEAM_DELETE_DRAFT, TEAM_UPDATE_CONFIG, TEAM_UPDATE_KANBAN, TEAM_UPDATE_KANBAN_COLUMN_ORDER, @@ -84,6 +87,8 @@ import { isAgentActionMode, } from '../services/team/actionModeInstructions'; import { TeamAttachmentStore } from '../services/team/TeamAttachmentStore'; +import { TeamMembersMetaStore } from '../services/team/TeamMembersMetaStore'; +import { TeamMetaStore } from '../services/team/TeamMetaStore'; import { buildAddMemberSpawnMessage } from '../services/team/TeamProvisioningService'; import { TeamTaskAttachmentStore } from '../services/team/TeamTaskAttachmentStore'; @@ -141,6 +146,7 @@ import type { TeamTask, TeamTaskStatus, TeamUpdateConfigRequest, + ToolApprovalFileContent, ToolApprovalSettings, UpdateKanbanPatch, } from '@shared/types'; @@ -261,6 +267,7 @@ let teamBackupService: TeamBackupService | null = null; const attachmentStore = new TeamAttachmentStore(); const taskAttachmentStore = new TeamTaskAttachmentStore(); +const teamMetaStore = new TeamMetaStore(); const ALLOWED_ATTACHMENT_TYPES = new Set(['image/png', 'image/jpeg', 'image/gif', 'image/webp']); const MAX_ATTACHMENT_SIZE = 10 * 1024 * 1024; // 10MB per file @@ -340,8 +347,11 @@ export function registerTeamHandlers(ipcMain: IpcMain): void { ipcMain.handle(TEAM_GET_TASK_ATTACHMENT, handleGetTaskAttachment); ipcMain.handle(TEAM_DELETE_TASK_ATTACHMENT, handleDeleteTaskAttachment); ipcMain.handle(TEAM_TOOL_APPROVAL_RESPOND, handleToolApprovalRespond); + ipcMain.handle(TEAM_TOOL_APPROVAL_READ_FILE, handleToolApprovalReadFile); ipcMain.handle(TEAM_VALIDATE_CLI_ARGS, handleValidateCliArgs); ipcMain.handle(TEAM_TOOL_APPROVAL_SETTINGS, handleToolApprovalSettings); + ipcMain.handle(TEAM_GET_SAVED_REQUEST, handleGetSavedRequest); + ipcMain.handle(TEAM_DELETE_DRAFT, handleDeleteDraft); logger.info('Team handlers registered'); } @@ -398,8 +408,11 @@ export function removeTeamHandlers(ipcMain: IpcMain): void { ipcMain.removeHandler(TEAM_GET_TASK_ATTACHMENT); ipcMain.removeHandler(TEAM_DELETE_TASK_ATTACHMENT); ipcMain.removeHandler(TEAM_TOOL_APPROVAL_RESPOND); + ipcMain.removeHandler(TEAM_TOOL_APPROVAL_READ_FILE); ipcMain.removeHandler(TEAM_VALIDATE_CLI_ARGS); ipcMain.removeHandler(TEAM_TOOL_APPROVAL_SETTINGS); + ipcMain.removeHandler(TEAM_GET_SAVED_REQUEST); + ipcMain.removeHandler(TEAM_DELETE_DRAFT); } function getTeamDataService(): TeamDataService { @@ -482,6 +495,13 @@ async function handleGetData( ) { return { success: false, error: 'TEAM_PROVISIONING' }; } + // Draft team: team.meta.json exists but config.json doesn't (provisioning failed before TeamCreate) + if (message === `Team not found: ${tn}`) { + const meta = await teamMetaStore.getMeta(tn); + if (meta) { + return { success: false, error: 'TEAM_DRAFT' }; + } + } logger.error(`[teams:getData] ${message}`); return { success: false, error: message }; } @@ -907,6 +927,58 @@ async function handleLaunchTeam( return { success: false, error: 'model must be a string' }; } + // Detect draft team: team.meta.json exists but config.json doesn't. + // This happens when user created team config without launching (launchTeam=false), + // or when provisioning failed before TeamCreate could run. + // Redirect to createTeam so TeamCreate runs properly. + const tn = validatedTeamName.value!; + const configPath = path.join(getTeamsBasePath(), tn, 'config.json'); + let isDraft = false; + try { + await fs.promises.access(configPath, fs.constants.F_OK); + } catch { + const meta = await teamMetaStore.getMeta(tn); + if (meta) isDraft = true; + } + + if (isDraft) { + const meta = await teamMetaStore.getMeta(tn); + const membersStore = new TeamMembersMetaStore(); + const members = await membersStore.getMembers(tn); + + const createRequest: TeamCreateRequest = { + teamName: tn, + displayName: meta?.displayName, + description: meta?.description, + color: meta?.color, + cwd, + prompt: typeof payload.prompt === 'string' ? payload.prompt.trim() || undefined : undefined, + model: typeof payload.model === 'string' ? payload.model.trim() || undefined : undefined, + effort: isValidEffort(payload.effort) ? payload.effort : undefined, + limitContext: typeof payload.limitContext === 'boolean' ? payload.limitContext : undefined, + skipPermissions: + typeof payload.skipPermissions === 'boolean' ? payload.skipPermissions : undefined, + worktree: + typeof payload.worktree === 'string' ? payload.worktree.trim() || undefined : undefined, + extraCliArgs: + typeof payload.extraCliArgs === 'string' + ? payload.extraCliArgs.trim() || undefined + : undefined, + members: members.map((m) => ({ name: m.name, role: m.role, workflow: m.workflow })), + }; + + return wrapTeamHandler('create', () => + getTeamProvisioningService().createTeam(createRequest, (progress) => { + try { + event.sender.send(TEAM_PROVISIONING_PROGRESS, progress); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + logger.warn(`Failed to emit draft launch provisioning progress: ${message}`); + } + }) + ); + } + return wrapTeamHandler('launch', () => getTeamProvisioningService().launchTeam( { @@ -2723,3 +2795,137 @@ async function handleToolApprovalSettings( } return { success: true, data: undefined }; } + +/** Max file size for tool approval diff preview (2MB). */ +const TOOL_APPROVAL_MAX_FILE_SIZE = 2 * 1024 * 1024; + +async function handleToolApprovalReadFile( + _event: IpcMainInvokeEvent, + filePath: unknown +): Promise> { + if (typeof filePath !== 'string' || filePath.trim().length === 0) { + return { success: false, error: 'filePath must be a non-empty string' }; + } + if (!path.isAbsolute(filePath)) { + return { success: false, error: 'filePath must be an absolute path' }; + } + + try { + let stats: fs.Stats; + try { + stats = await fs.promises.stat(filePath); + } catch (err) { + if ((err as NodeJS.ErrnoException).code === 'ENOENT') { + return { + success: true, + data: { content: '', exists: false, truncated: false, isBinary: false }, + }; + } + throw err; + } + + if (!stats.isFile()) { + return { + success: true, + data: { content: '', exists: true, truncated: false, isBinary: false, error: 'Not a file' }, + }; + } + + const truncated = stats.size > TOOL_APPROVAL_MAX_FILE_SIZE; + const readSize = truncated ? TOOL_APPROVAL_MAX_FILE_SIZE : stats.size; + + // Read file (potentially truncated) + const fd = await fs.promises.open(filePath, 'r'); + try { + const buffer = Buffer.alloc(readSize); + await fd.read(buffer, 0, readSize, 0); + + // Binary detection: check first 8KB for null bytes + const checkSize = Math.min(readSize, 8192); + for (let i = 0; i < checkSize; i++) { + if (buffer[i] === 0) { + return { + success: true, + data: { content: '', exists: true, truncated: false, isBinary: true }, + }; + } + } + + return { + success: true, + data: { content: buffer.toString('utf-8'), exists: true, truncated, isBinary: false }, + }; + } finally { + await fd.close(); + } + } catch (err) { + const msg = err instanceof Error ? err.message : String(err); + return { + success: true, + data: { content: '', exists: true, truncated: false, isBinary: false, error: msg }, + }; + } +} + +async function handleGetSavedRequest( + _event: IpcMainInvokeEvent, + teamName: unknown +): Promise> { + const validated = validateTeamName(teamName); + if (!validated.valid) { + return { success: false, error: validated.error ?? 'Invalid teamName' }; + } + const tn = validated.value!; + + const meta = await teamMetaStore.getMeta(tn); + if (!meta) { + return { success: true, data: null }; + } + + const membersStore = new TeamMembersMetaStore(); + const members = await membersStore.getMembers(tn); + + return { + success: true, + data: { + teamName: tn, + displayName: meta.displayName, + description: meta.description, + color: meta.color, + cwd: meta.cwd, + prompt: meta.prompt, + model: meta.model, + effort: meta.effort as TeamCreateRequest['effort'], + skipPermissions: meta.skipPermissions, + worktree: meta.worktree, + extraCliArgs: meta.extraCliArgs, + limitContext: meta.limitContext, + members: members.map((m) => ({ + name: m.name, + role: m.role, + workflow: m.workflow, + })), + }, + }; +} + +async function handleDeleteDraft( + _event: IpcMainInvokeEvent, + teamName: unknown +): Promise> { + const validated = validateTeamName(teamName); + if (!validated.valid) { + return { success: false, error: validated.error ?? 'Invalid teamName' }; + } + return wrapTeamHandler('deleteDraft', async () => { + // Only allow deleting draft teams (no config.json) + const configPath = path.join(getTeamsBasePath(), validated.value!, 'config.json'); + try { + await fs.promises.access(configPath, fs.constants.F_OK); + throw new Error('Cannot delete draft: team has config.json (use deleteTeam instead)'); + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== 'ENOENT') throw error; + } + await getTeamDataService().permanentlyDeleteTeam(validated.value!); + }); +} diff --git a/src/main/services/team/TeamProvisioningService.ts b/src/main/services/team/TeamProvisioningService.ts index 37871d1c..14d9c55d 100644 --- a/src/main/services/team/TeamProvisioningService.ts +++ b/src/main/services/team/TeamProvisioningService.ts @@ -54,6 +54,7 @@ import { withFileLock } from './fileLock'; import { withInboxLock } from './inboxLock'; import { TeamConfigReader } from './TeamConfigReader'; import { TeamInboxReader } from './TeamInboxReader'; +import { TeamMetaStore } from './TeamMetaStore'; import { TeamMcpConfigBuilder } from './TeamMcpConfigBuilder'; import { TeamMembersMetaStore } from './TeamMembersMetaStore'; import { TeamSentMessagesStore } from './TeamSentMessagesStore'; @@ -908,21 +909,21 @@ ${buildMemberSpawnPrompt(m, displayName, request.teamName, leadName) members: request.members, }); - return `agent_teams_ui [Agent Team: “${request.teamName}” | Project: “${projectName}” | Lead: “${leadName}”] — team config has been pre-created. Proceed with provisioning. + return `agent_teams_ui [Agent Team: “${request.teamName}” | Project: “${projectName}” | Lead: “${leadName}”] — team does NOT exist yet. You must create it. You are running in a non-interactive CLI session. Do not ask questions. Do everything in a single turn. CRITICAL: Execute ALL steps directly yourself in sequence. Do NOT delegate any step to a sub-agent via the Agent tool. The ONLY valid use of the Agent tool is spawning individual teammates in step 2. -Do NOT call mcp__agent-teams__team_launch, mcp__agent-teams__team_stop, or any other mcp__agent-teams__ runtime tool during provisioning. MCP board tools (task_create, task_set_status, etc.) are allowed only in step 3. +CRITICAL: For step 1, use the BUILT-IN TeamCreate tool — NOT any mcp__agent-teams__* MCP tool. Do NOT call mcp__agent-teams__team_launch, mcp__agent-teams__team_stop, or any other mcp__agent-teams__ runtime tool during provisioning. MCP board tools (task_create, task_set_status, etc.) are allowed only in step 3. You are “${leadName}”, the team lead. -Goal: Provision Claude Code agent team${request.members.length === 0 ? ' (solo — lead only)' : ' with live teammates'}. -The team config has been pre-created (config.json, members metadata). Proceed with member provisioning. +Goal: Create and provision a NEW Claude Code agent team${request.members.length === 0 ? ' (solo — lead only)' : ' with live teammates'}. +The team does NOT exist yet — no config, no state, nothing. Step 1 is MANDATORY. ${userPromptBlock} ${persistentContext} Steps (execute in this exact order — do NOT skip any step): -1) The team config has been pre-created. Proceed directly to step 2. +1) MANDATORY FIRST STEP: Call the BUILT-IN TeamCreate tool (not any MCP tool) with team_name=”${request.teamName}”. This creates the team config and in-memory state. Without this step, teammate spawns will FAIL. Do NOT assume the team already exists. ${step2Block} @@ -1263,7 +1264,8 @@ export class TeamProvisioningService { private readonly inboxReader: TeamInboxReader = new TeamInboxReader(), private readonly membersMetaStore: TeamMembersMetaStore = new TeamMembersMetaStore(), _sentMessagesStore: TeamSentMessagesStore = new TeamSentMessagesStore(), - private readonly mcpConfigBuilder: TeamMcpConfigBuilder = new TeamMcpConfigBuilder() + private readonly mcpConfigBuilder: TeamMcpConfigBuilder = new TeamMcpConfigBuilder(), + private readonly teamMetaStore: TeamMetaStore = new TeamMetaStore() ) {} setCrossTeamSender( @@ -2438,6 +2440,10 @@ export class TeamProvisioningService { if (silenceMs < STALL_WARNING_THRESHOLD_MS) return; if (lastWarningAt > 0 && now - lastWarningAt < STALL_WARNING_REPEAT_MS) return; + // Don't show stall warnings if CLI has already produced output — + // silence between tool calls is normal (e.g. waiting for teammate spawn). + if (run.claudeLogLines.length > 0) return; + lastWarningAt = now; const silenceSec = Math.round(silenceMs / 1000); @@ -2846,7 +2852,7 @@ export class TeamProvisioningService { waitingTasksSince: null, provisioningComplete: false, isLaunch: false, - fsPhase: 'waiting_members', + fsPhase: 'waiting_config', leadRelayCapture: null, activeCrossTeamReplyHints: [], leadMsgSeq: 0, @@ -2905,7 +2911,7 @@ export class TeamProvisioningService { '--mcp-config', mcpConfigPath, '--disallowedTools', - 'TeamDelete,TodoWrite,TeamCreate', + 'TeamDelete,TodoWrite', // Explicit --permission-mode overrides user's defaultMode in ~/.claude/settings.json // (e.g. "acceptEdits") which otherwise takes precedence over CLI flags ...(request.skipPermissions !== false @@ -2917,14 +2923,46 @@ export class TeamProvisioningService { ...parseCliArgs(request.extraCliArgs), ]; try { - await this.preCreateConfig(request); + // Pre-save our meta files before spawn — CLI doesn't touch these. + // If provisioning fails before TeamCreate, user can retry without re-entering config. + const teamDir = path.join(getTeamsBasePath(), request.teamName); + const tasksDir = path.join(getTasksBasePath(), request.teamName); + await fs.promises.mkdir(teamDir, { recursive: true }); + await fs.promises.mkdir(tasksDir, { recursive: true }); + await this.teamMetaStore.writeMeta(request.teamName, { + displayName: request.displayName, + description: request.description, + color: request.color, + cwd: request.cwd, + prompt: request.prompt, + model: request.model, + effort: request.effort, + skipPermissions: request.skipPermissions, + worktree: request.worktree, + extraCliArgs: request.extraCliArgs, + limitContext: request.limitContext, + createdAt: Date.now(), + }); + await this.membersMetaStore.writeMembers( + request.teamName, + request.members.map((m) => ({ + name: m.name.trim(), + role: m.role?.trim() || undefined, + workflow: m.workflow?.trim() || undefined, + agentType: 'general-purpose' as const, + color: getMemberColorByName(m.name.trim()), + joinedAt: Date.now(), + })) + ); + child = spawnCli(claudePath, spawnArgs, { cwd: request.cwd, env: { ...shellEnv }, stdio: ['pipe', 'pipe', 'pipe'], }); } catch (error) { - // Clean up pre-saved files (may not exist if preCreateConfig failed partway) + // Clean up pre-saved meta files if spawn failed (instant failure, not transient) + await this.teamMetaStore.deleteMeta(request.teamName).catch(() => {}); const teamDir = path.join(getTeamsBasePath(), request.teamName); const tasksDir = path.join(getTasksBasePath(), request.teamName); await fs.promises.rm(teamDir, { recursive: true, force: true }).catch(() => {}); @@ -2936,7 +2974,6 @@ export class TeamProvisioningService { updateProgress(run, 'spawning', 'Starting Claude CLI process', { pid: child.pid ?? undefined, - configReady: true, }); run.onProgress(run.progress); run.child = child; @@ -5124,12 +5161,17 @@ export class TeamProvisioningService { const configLead = config.members.find((m) => isLeadAgentType(m?.agentType)); leadName = configLead?.name?.trim() || 'team-lead'; // Convert config members (excluding lead) to TeamCreateRequest member format. - currentMembers = config.members + const configTeammates = config.members .filter((m) => !isLeadAgentType(m?.agentType) && m?.name) .map((m) => ({ name: m.name, role: m.role ?? undefined, })); + // When config.members only has the lead (pre-created config without + // TeamCreate), fall back to run.request.members for the teammate list. + if (configTeammates.length > 0) { + currentMembers = configTeammates; + } } else { leadName = run.request.members.find((m) => m.role?.toLowerCase().includes('lead'))?.name || @@ -5266,6 +5308,8 @@ export class TeamProvisioningService { toolName, toolInput, receivedAt: new Date().toISOString(), + teamColor: run.request.color, + teamDisplayName: run.request.displayName, }; // Check auto-allow rules before prompting user @@ -5445,7 +5489,8 @@ export class TeamProvisioningService { allow: boolean, message?: string ): Promise { - const currentRunId = this.getAliveRunId(teamName); + // Look in both provisioning and alive runs — control_requests arrive during provisioning too + const currentRunId = this.getTrackedRunId(teamName); if (!currentRunId) throw new Error(`No active process for team "${teamName}"`); const run = this.runs.get(currentRunId); if (!run) throw new Error(`Run not found for team "${teamName}"`); @@ -5494,13 +5539,25 @@ export class TeamProvisioningService { }; const stdin = run.child.stdin; + const responseJson = JSON.stringify(response) + '\n'; + logger.info( + `[${teamName}] Writing control_response for ${requestId}: ${allow ? 'allow' : 'deny'}` + ); try { await new Promise((resolve, reject) => { - stdin.write(JSON.stringify(response) + '\n', (err) => { + // Safety timeout — if stdin.write callback is never called (e.g. process died + // between the writable check and the write), reject instead of hanging forever. + const writeTimeout = setTimeout(() => { + reject(new Error(`Timeout writing control_response to stdin (process may have exited)`)); + }, 5000); + + stdin.write(responseJson, (err) => { + clearTimeout(writeTimeout); if (err) { logger.error(`[${teamName}] Failed to write control_response: ${err.message}`); reject(err); } else { + logger.info(`[${teamName}] control_response written successfully for ${requestId}`); resolve(); } }); @@ -5692,6 +5749,9 @@ export class TeamProvisioningService { run.request.color ); + // Clean up team.meta.json — provisioning succeeded, config.json is now authoritative. + await this.teamMetaStore.deleteMeta(run.teamName).catch(() => {}); + // Audit: flag any expected member not registered in config.json after provisioning. await this.auditMemberSpawnStatuses(run); @@ -7199,41 +7259,6 @@ export class TeamProvisioningService { } } - /** - * Pre-saves team config.json + members-meta.json + tasks/ dir to disk - * BEFORE spawning CLI. This guarantees the team persists even if CLI - * crashes (e.g. 429 rate limit) before it can call TeamCreate. - */ - private async preCreateConfig(request: TeamCreateRequest): Promise { - const teamDir = path.join(getTeamsBasePath(), request.teamName); - const tasksDir = path.join(getTasksBasePath(), request.teamName); - await fs.promises.mkdir(teamDir, { recursive: true }); - await fs.promises.mkdir(tasksDir, { recursive: true }); - - const config: Record = { - name: request.displayName?.trim() || request.teamName, - description: request.description?.trim() || undefined, - color: request.color?.trim() || undefined, - }; - if (request.cwd?.trim()) { - config.projectPath = request.cwd.trim(); - config.projectPathHistory = [request.cwd.trim()]; - } - await atomicWriteAsync(path.join(teamDir, 'config.json'), JSON.stringify(config, null, 2)); - - const joinedAt = Date.now(); - await this.membersMetaStore.writeMembers( - request.teamName, - request.members.map((m) => ({ - name: m.name.trim(), - role: m.role?.trim() || undefined, - agentType: 'general-purpose' as const, - color: getMemberColorByName(m.name.trim()), - joinedAt, - })) - ); - } - private async persistMembersMeta(teamName: string, request: TeamCreateRequest): Promise { const teammateMembers = request.members.filter((member) => { const trimmed = member.name.trim(); diff --git a/src/preload/constants/ipcChannels.ts b/src/preload/constants/ipcChannels.ts index 08e23fac..19131baf 100644 --- a/src/preload/constants/ipcChannels.ts +++ b/src/preload/constants/ipcChannels.ts @@ -373,6 +373,12 @@ export const TEAM_VALIDATE_CLI_ARGS = 'team:validateCliArgs'; /** Invoke: update tool approval settings (renderer → main) */ export const TEAM_TOOL_APPROVAL_SETTINGS = 'team:toolApprovalSettings'; +/** Invoke: read file content for tool approval diff preview (renderer → main) */ +export const TEAM_TOOL_APPROVAL_READ_FILE = 'team:toolApprovalReadFile'; + +export const TEAM_GET_SAVED_REQUEST = 'team:getSavedRequest'; +export const TEAM_DELETE_DRAFT = 'team:deleteDraft'; + // ============================================================================= // Cross-Team Communication Channels // ============================================================================= diff --git a/src/preload/index.ts b/src/preload/index.ts index 108e5537..25cc3189 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -132,6 +132,8 @@ import { TEAM_LIST, TEAM_MEMBER_SPAWN_STATUSES, TEAM_PERMANENTLY_DELETE, + TEAM_GET_SAVED_REQUEST, + TEAM_DELETE_DRAFT, TEAM_PREPARE_PROVISIONING, TEAM_PROCESS_ALIVE, TEAM_PROCESS_SEND, @@ -151,6 +153,7 @@ import { TEAM_START_TASK, TEAM_STOP, TEAM_TOOL_APPROVAL_EVENT, + TEAM_TOOL_APPROVAL_READ_FILE, TEAM_TOOL_APPROVAL_RESPOND, TEAM_TOOL_APPROVAL_SETTINGS, TEAM_UPDATE_CONFIG, @@ -275,6 +278,7 @@ import type { TeamTaskStatus, TeamUpdateConfigRequest, ToolApprovalEvent, + ToolApprovalFileContent, ToolApprovalSettings, TriggerTestResult, UpdateKanbanPatch, @@ -806,6 +810,12 @@ const electronAPI: ElectronAPI = { permanentlyDeleteTeam: async (teamName: string) => { return invokeIpcWithResult(TEAM_PERMANENTLY_DELETE, teamName); }, + getSavedRequest: async (teamName: string) => { + return invokeIpcWithResult(TEAM_GET_SAVED_REQUEST, teamName); + }, + deleteDraft: async (teamName: string) => { + return invokeIpcWithResult(TEAM_DELETE_DRAFT, teamName); + }, prepareProvisioning: async (cwd?: string) => { return invokeIpcWithResult(TEAM_PREPARE_PROVISIONING, cwd); }, @@ -1093,6 +1103,9 @@ const electronAPI: ElectronAPI = { updateToolApprovalSettings: async (settings: ToolApprovalSettings) => { return invokeIpcWithResult(TEAM_TOOL_APPROVAL_SETTINGS, settings); }, + readFileForToolApproval: async (filePath: string) => { + return invokeIpcWithResult(TEAM_TOOL_APPROVAL_READ_FILE, filePath); + }, }, crossTeam: { send: async (request: CrossTeamSendRequest) => { diff --git a/src/renderer/api/httpClient.ts b/src/renderer/api/httpClient.ts index 6c4ed967..b515456d 100644 --- a/src/renderer/api/httpClient.ts +++ b/src/renderer/api/httpClient.ts @@ -684,6 +684,13 @@ export class HttpAPIClient implements ElectronAPI { permanentlyDeleteTeam: async (_teamName: string): Promise => { throw new Error('Permanent team deletion is not available in browser mode'); }, + getSavedRequest: async (_teamName: string): Promise => { + console.warn('[HttpAPIClient] getSavedRequest is not available in browser mode'); + return null; + }, + deleteDraft: async (_teamName: string): Promise => { + throw new Error('Draft team deletion is not available in browser mode'); + }, prepareProvisioning: async (_cwd?: string): Promise => { throw new Error('Team provisioning is not available in browser mode'); }, @@ -916,6 +923,9 @@ export class HttpAPIClient implements ElectronAPI { updateToolApprovalSettings: async (): Promise => { console.warn('[HttpAPIClient] updateToolApprovalSettings is not available in browser mode'); }, + readFileForToolApproval: async () => { + throw new Error('Tool approval file read not available in browser mode'); + }, }; // Cross-team communication API stubs diff --git a/src/renderer/components/team/ToolApprovalDiffPreview.tsx b/src/renderer/components/team/ToolApprovalDiffPreview.tsx new file mode 100644 index 00000000..7912503c --- /dev/null +++ b/src/renderer/components/team/ToolApprovalDiffPreview.tsx @@ -0,0 +1,154 @@ +import React, { useEffect, useState } from 'react'; + +import { DiffViewer } from '@renderer/components/chat/viewers/DiffViewer'; +import { useToolApprovalDiff } from '@renderer/hooks/useToolApprovalDiff'; +import { AlertTriangle, ChevronDown, ChevronRight, FileDiff, Loader2 } from 'lucide-react'; + +// ============================================================================= +// Types +// ============================================================================= + +interface ToolApprovalDiffPreviewProps { + toolName: string; + toolInput: Record; + requestId: string; + onExpandedChange?: (expanded: boolean) => void; +} + +const DIFF_TOOLS = new Set(['Write', 'Edit', 'NotebookEdit']); + +// ============================================================================= +// Component +// ============================================================================= + +export const ToolApprovalDiffPreview: React.FC = ({ + toolName, + toolInput, + requestId, + onExpandedChange, +}) => { + const [expanded, setExpanded] = useState(false); + const diff = useToolApprovalDiff(toolName, toolInput, requestId, expanded); + + // Collapse when approval changes + useEffect(() => { + setExpanded(false); + onExpandedChange?.(false); + // eslint-disable-next-line react-hooks/exhaustive-deps -- onExpandedChange is stable setter, only reset on requestId change + }, [requestId]); + + if (!DIFF_TOOLS.has(toolName)) return null; + + const toggleExpanded = (): void => { + const next = !expanded; + setExpanded(next); + onExpandedChange?.(next); + }; + + return ( +
+ {/* Toggle button */} + + + {/* Collapsible content */} + {expanded && ( +
+ {diff.loading && ( +
+ + Reading file... +
+ )} + + {diff.isBinary && ( +
+ + Binary file — cannot preview +
+ )} + + {diff.error && !diff.loading && ( +
+ + {diff.error} +
+ )} + + {diff.truncated && !diff.loading && ( +
+ + File truncated at 2MB — diff may be incomplete +
+ )} + + {!diff.loading && !diff.isBinary && !diff.error && (diff.oldString || diff.newString) && ( +
+ {diff.isNewFile && ( + + New file + + )} + +
+ )} +
+ )} +
+ ); +}; diff --git a/src/renderer/components/team/ToolApprovalSheet.tsx b/src/renderer/components/team/ToolApprovalSheet.tsx index 3368b43d..18efa0df 100644 --- a/src/renderer/components/team/ToolApprovalSheet.tsx +++ b/src/renderer/components/team/ToolApprovalSheet.tsx @@ -1,10 +1,12 @@ -import React, { useCallback, useEffect, useRef, useState } from 'react'; +import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react'; import { getTeamColorSet, getThemedBadge } from '@renderer/constants/teamColors'; import { useTheme } from '@renderer/hooks/useTheme'; import { useStore } from '@renderer/store'; -import { FileText, Search, Terminal } from 'lucide-react'; +import { highlightLines } from '@renderer/utils/syntaxHighlighter'; +import { AlertTriangle, FileText, Search, Terminal } from 'lucide-react'; +import { ToolApprovalDiffPreview } from './ToolApprovalDiffPreview'; import { ToolApprovalSettingsPanel } from './dialogs/ToolApprovalSettingsPanel'; import type { ToolApprovalRequest } from '@shared/types'; @@ -52,6 +54,24 @@ function renderToolInput(toolName: string, input: Record): stri } } +/** Map tool name to a virtual filename for syntax highlighting. */ +function getToolInputFileName(toolName: string, input: Record): string { + switch (toolName) { + case 'Bash': + return 'command.sh'; + case 'Edit': + case 'Read': + case 'Write': + case 'NotebookEdit': + return typeof input.file_path === 'string' ? input.file_path : 'input.json'; + case 'Grep': + case 'Glob': + return 'pattern.txt'; + default: + return 'input.json'; + } +} + // --------------------------------------------------------------------------- // Elapsed timer hook // --------------------------------------------------------------------------- @@ -78,25 +98,53 @@ function useElapsed(receivedAt: string): number { // Main component // --------------------------------------------------------------------------- +/** Max time (ms) to wait for the IPC before considering it stuck */ +const RESPOND_TIMEOUT_MS = 10_000; + export const ToolApprovalSheet: React.FC = () => { const pendingApprovals = useStore((s) => s.pendingApprovals); const respondToToolApproval = useStore((s) => s.respondToToolApproval); const teams = useStore((s) => s.teams); + const selectedTeamName = useStore((s) => s.selectedTeamName); const { isLight } = useTheme(); const current: ToolApprovalRequest | undefined = pendingApprovals[0]; const containerRef = useRef(null); const [disabled, setDisabled] = useState(false); + const [error, setError] = useState(null); + const [diffExpanded, setDiffExpanded] = useState(false); + + // Clear error and collapse diff when current approval changes + useEffect(() => { + setError(null); + setDiffExpanded(false); + }, [current?.requestId]); const handleRespond = useCallback( (allow: boolean) => { if (!current || disabled) return; setDisabled(true); - void respondToToolApproval(current.teamName, current.runId, current.requestId, allow).finally( - () => { + setError(null); + + // Safety timeout — if IPC hangs (e.g. stdin.write callback never fires), + // re-enable the button so the user isn't stuck forever. + const safetyTimer = setTimeout(() => { + setDisabled(false); + setError('Response timed out — process may be unresponsive. Try again or stop the team.'); + }, RESPOND_TIMEOUT_MS); + + respondToToolApproval(current.teamName, current.runId, current.requestId, allow) + .then(() => { + clearTimeout(safetyTimer); + // Small delay before re-enabling to prevent accidental double-clicks setTimeout(() => setDisabled(false), 200); - } - ); + }) + .catch((err: unknown) => { + clearTimeout(safetyTimer); + const msg = err instanceof Error ? err.message : String(err); + setError(msg); + setDisabled(false); + }); }, [current, disabled, respondToToolApproval] ); @@ -120,13 +168,17 @@ export const ToolApprovalSheet: React.FC = () => { if (!current) return null; + // Prefer color from the approval itself (always available, even during provisioning), + // fall back to teams list for older approvals without the field. const teamSummary = teams.find((t) => t.teamName === current.teamName); - const teamColor = teamSummary?.color ? getTeamColorSet(teamSummary.color) : null; + const colorName = current.teamColor ?? teamSummary?.color; + const teamColor = colorName ? getTeamColorSet(colorName) : null; + const displayName = current.teamDisplayName ?? teamSummary?.displayName ?? current.teamName; return (
{
- {teamColor ? ( - - {teamSummary?.displayName ?? current.teamName} - - ) : ( - {current.teamName} - )} + {selectedTeamName !== current.teamName && + (teamColor ? ( + + {displayName} + + ) : ( + + {displayName} + + ))}
- {/* Tool input preview */} -
- 
+
+      {/* Diff preview (Write/Edit/NotebookEdit only) */}
+      
+
+      {/* Error feedback */}
+      {error && (
+        

-          {renderToolInput(current.toolName, current.toolInput)}
-
-
+ + {error} + + )} {/* Actions */}
{ ); }; +// --------------------------------------------------------------------------- +// Syntax-highlighted tool input preview +// --------------------------------------------------------------------------- + +const ToolInputPreview = ({ + toolName, + toolInput, +}: { + toolName: string; + toolInput: Record; +}): React.JSX.Element => { + const text = renderToolInput(toolName, toolInput); + const fileName = getToolInputFileName(toolName, toolInput); + const lines = useMemo(() => highlightLines(text, fileName), [text, fileName]); + + return ( +
+
+ {/* highlightLines uses hljs which HTML-escapes all input text, producing only tags. + This is safe: the source is our own renderToolInput() output, not arbitrary user HTML. + Same pattern used in ReviewDiffContent.tsx and DiffViewer for syntax highlighting. */} + {lines.map((html, i) => ( +
+ ))} +
+
+ ); +}; + // --------------------------------------------------------------------------- // Timeout progress bar sub-component // --------------------------------------------------------------------------- @@ -268,7 +382,7 @@ const TimeoutProgress = ({ receivedAt }: { receivedAt: string }): React.JSX.Elem />
- Auto-{settings.timeoutAction} in {remaining}s + Auto-{settings.timeoutAction} in {formatElapsed(remaining)}
); @@ -278,9 +392,18 @@ const TimeoutProgress = ({ receivedAt }: { receivedAt: string }): React.JSX.Elem // Elapsed display sub-component (uses hook) // --------------------------------------------------------------------------- +function formatElapsed(seconds: number): string { + if (seconds < 60) return `${seconds}s`; + const m = Math.floor(seconds / 60); + const s = seconds % 60; + return s > 0 ? `${m}m ${s}s` : `${m}m`; +} + const ElapsedDisplay = ({ receivedAt }: { receivedAt: string }): React.JSX.Element => { const elapsed = useElapsed(receivedAt); return ( - {elapsed}s + + {formatElapsed(elapsed)} + ); }; diff --git a/src/renderer/components/team/dialogs/ToolApprovalSettingsPanel.tsx b/src/renderer/components/team/dialogs/ToolApprovalSettingsPanel.tsx index 211790a1..ad5f7218 100644 --- a/src/renderer/components/team/dialogs/ToolApprovalSettingsPanel.tsx +++ b/src/renderer/components/team/dialogs/ToolApprovalSettingsPanel.tsx @@ -1,6 +1,13 @@ import React, { useState } from 'react'; import { Checkbox } from '@renderer/components/ui/checkbox'; +import { + Select, + SelectContent, + SelectItem, + SelectTrigger, + SelectValue, +} from '@renderer/components/ui/select'; import { useStore } from '@renderer/store'; import { ChevronDown, ChevronRight, Settings } from 'lucide-react'; @@ -80,22 +87,21 @@ export const ToolApprovalSettingsPanel: React.FC = () => { style={{ color: 'var(--color-text-secondary)' }} > On timeout: - + + + + + Wait forever + Allow + Deny + + {settings.timeoutAction !== 'wait' && ( <> diff --git a/src/renderer/hooks/useToolApprovalDiff.ts b/src/renderer/hooks/useToolApprovalDiff.ts new file mode 100644 index 00000000..787e7aab --- /dev/null +++ b/src/renderer/hooks/useToolApprovalDiff.ts @@ -0,0 +1,212 @@ +import { useEffect, useRef, useState } from 'react'; + +import { api } from '@renderer/api'; + +import type { ToolApprovalFileContent } from '@shared/types'; + +// ============================================================================= +// Types +// ============================================================================= + +export interface ToolApprovalDiffData { + /** Whether this tool type supports diff preview */ + hasDiff: boolean; + /** Loading state (file read in progress) */ + loading: boolean; + /** Error message */ + error: string | null; + /** File name for syntax highlighting */ + fileName: string; + /** Original file content */ + oldString: string; + /** New file content after tool execution */ + newString: string; + /** File doesn't exist yet */ + isNewFile: boolean; + /** File content was truncated at size limit */ + truncated: boolean; + /** File is binary */ + isBinary: boolean; +} + +const DIFF_TOOLS = new Set(['Write', 'Edit', 'NotebookEdit']); + +const INITIAL_STATE: ToolApprovalDiffData = { + hasDiff: false, + loading: false, + error: null, + fileName: '', + oldString: '', + newString: '', + isNewFile: false, + truncated: false, + isBinary: false, +}; + +// ============================================================================= +// Helpers +// ============================================================================= + +function getFilePath(toolInput: Record): string { + const fp = toolInput.file_path ?? toolInput.notebook_path; + return typeof fp === 'string' ? fp : ''; +} + +function computeEditResult( + currentContent: string, + toolInput: Record +): { newString: string; error: string | null } { + const oldStr = typeof toolInput.old_string === 'string' ? toolInput.old_string : ''; + const newStr = typeof toolInput.new_string === 'string' ? toolInput.new_string : ''; + const replaceAll = toolInput.replace_all === true; + + if (!oldStr) { + return { newString: currentContent, error: 'Edit: old_string is empty' }; + } + + if (!currentContent.includes(oldStr)) { + return { newString: currentContent, error: 'Fragment not found in current file' }; + } + + const result = replaceAll + ? currentContent.replaceAll(oldStr, newStr) + : currentContent.replace(oldStr, newStr); + + return { newString: result, error: null }; +} + +// ============================================================================= +// Hook +// ============================================================================= + +/** + * Lazy-loading hook that reads file content from disk and computes diff data + * for Write/Edit/NotebookEdit tool approvals. + * + * @param toolName - The tool requesting approval + * @param toolInput - The tool's input parameters + * @param requestId - Unique approval request ID (used for cancellation) + * @param enabled - Only fetch when true (lazy — user expanded the diff section) + */ +export function useToolApprovalDiff( + toolName: string, + toolInput: Record, + requestId: string, + enabled: boolean +): ToolApprovalDiffData { + const [state, setState] = useState(INITIAL_STATE); + const activeRef = useRef(null); + + const hasDiff = DIFF_TOOLS.has(toolName); + const filePath = getFilePath(toolInput); + + useEffect(() => { + // Reset when approval changes + setState(INITIAL_STATE); + activeRef.current = null; + }, [requestId]); + + useEffect(() => { + if (!hasDiff || !enabled || !filePath) return; + + // NotebookEdit: no file read needed, just show new_source + if (toolName === 'NotebookEdit') { + const newSource = typeof toolInput.new_source === 'string' ? toolInput.new_source : ''; + setState({ + hasDiff: true, + loading: false, + error: null, + fileName: filePath, + oldString: '', + newString: newSource, + isNewFile: false, + truncated: false, + isBinary: false, + }); + return; + } + + // Write / Edit: need to read current file from disk + const currentRequestId = requestId; + activeRef.current = currentRequestId; + + setState((prev) => ({ ...prev, hasDiff: true, loading: true, error: null })); + + void (async () => { + let result: ToolApprovalFileContent; + try { + result = await api.teams.readFileForToolApproval(filePath); + } catch (err) { + if (activeRef.current !== currentRequestId) return; + setState((prev) => ({ + ...prev, + loading: false, + error: err instanceof Error ? err.message : String(err), + })); + return; + } + + if (activeRef.current !== currentRequestId) return; + + if (result.error) { + setState((prev) => ({ + ...prev, + loading: false, + error: result.error ?? 'Unknown read error', + fileName: filePath, + })); + return; + } + + if (result.isBinary) { + setState((prev) => ({ + ...prev, + loading: false, + isBinary: true, + fileName: filePath, + })); + return; + } + + const isNewFile = !result.exists; + const currentContent = result.content; + + if (toolName === 'Write') { + const newContent = typeof toolInput.content === 'string' ? toolInput.content : ''; + setState({ + hasDiff: true, + loading: false, + error: null, + fileName: filePath, + oldString: currentContent, + newString: newContent, + isNewFile, + truncated: result.truncated, + isBinary: false, + }); + } else if (toolName === 'Edit') { + const { newString, error } = computeEditResult(currentContent, toolInput); + setState({ + hasDiff: true, + loading: false, + error, + fileName: filePath, + oldString: currentContent, + newString, + isNewFile: false, + truncated: result.truncated, + isBinary: false, + }); + } + })(); + + return () => { + activeRef.current = null; + }; + // eslint-disable-next-line react-hooks/exhaustive-deps -- toolInput is a fresh object each render, use requestId for identity + }, [hasDiff, enabled, filePath, requestId, toolName]); + + if (!hasDiff) return INITIAL_STATE; + + return { ...state, hasDiff: true }; +} diff --git a/src/renderer/store/slices/teamSlice.ts b/src/renderer/store/slices/teamSlice.ts index af1995ea..c7c8bf6c 100644 --- a/src/renderer/store/slices/teamSlice.ts +++ b/src/renderer/store/slices/teamSlice.ts @@ -1231,6 +1231,16 @@ export const createTeamSlice: StateCreator = (set, return; } + // Draft team: team.meta.json exists but config.json doesn't (provisioning failed) + if (msg === 'TEAM_DRAFT' || msg.includes('TEAM_DRAFT')) { + set({ + selectedTeamLoading: false, + selectedTeamData: null, + selectedTeamError: 'TEAM_DRAFT', + }); + return; + } + const message = error instanceof IpcError ? error.message @@ -1299,6 +1309,15 @@ export const createTeamSlice: StateCreator = (set, return; } + if (msg === 'TEAM_DRAFT' || msg.includes('TEAM_DRAFT')) { + set({ + selectedTeamLoading: false, + selectedTeamData: null, + selectedTeamError: 'TEAM_DRAFT', + }); + return; + } + logger.warn(`refreshTeamData(${teamName}) failed: ${msg}`); // Non-destructive: if we already have data, keep it visible. @@ -2061,9 +2080,11 @@ export const createTeamSlice: StateCreator = (set, (a) => !(a.runId === runId && a.requestId === requestId) ), })); - } catch { - // IPC failed — approval stays in UI, user can retry - // Do NOT modify pendingApprovals — nothing was removed, nothing to rollback + } catch (err) { + const msg = err instanceof Error ? err.message : String(err); + logger.error(`respondToToolApproval failed for ${teamName}/${requestId}: ${msg}`); + // Surface the error so ToolApprovalSheet can show feedback + throw err; } }, diff --git a/src/shared/types/api.ts b/src/shared/types/api.ts index f19ba5f7..14674d56 100644 --- a/src/shared/types/api.ts +++ b/src/shared/types/api.ts @@ -74,6 +74,7 @@ import type { TeamTaskStatus, TeamUpdateConfigRequest, ToolApprovalEvent, + ToolApprovalFileContent, ToolApprovalSettings, UpdateKanbanPatch, } from './team'; @@ -419,6 +420,8 @@ export interface TeamsAPI { deleteTeam: (teamName: string) => Promise; restoreTeam: (teamName: string) => Promise; permanentlyDeleteTeam: (teamName: string) => Promise; + getSavedRequest: (teamName: string) => Promise; + deleteDraft: (teamName: string) => Promise; prepareProvisioning: (cwd?: string) => Promise; createTeam: (request: TeamCreateRequest) => Promise; getProvisioningStatus: (runId: string) => Promise; @@ -536,6 +539,7 @@ export interface TeamsAPI { validateCliArgs: (rawArgs: string) => Promise; onToolApprovalEvent: (callback: (event: unknown, data: ToolApprovalEvent) => void) => () => void; updateToolApprovalSettings: (settings: ToolApprovalSettings) => Promise; + readFileForToolApproval: (filePath: string) => Promise; } // ============================================================================= diff --git a/src/shared/types/team.ts b/src/shared/types/team.ts index 1d016592..82a525d0 100644 --- a/src/shared/types/team.ts +++ b/src/shared/types/team.ts @@ -53,6 +53,8 @@ export interface TeamSummary { sessionHistory?: string[]; /** Propagated from config.deletedAt — set when the team has been soft-deleted. */ deletedAt?: string; + /** True when team.meta.json exists but config.json doesn't — provisioning failed before TeamCreate. */ + pendingCreate?: boolean; } export type TeamTaskStatus = 'pending' | 'in_progress' | 'completed' | 'deleted'; @@ -803,6 +805,10 @@ export interface ToolApprovalRequest { toolInput: Record; /** ISO timestamp when the request was received. */ receivedAt: string; + /** Team color name (from config or create request) for badge rendering. */ + teamColor?: string; + /** Team display name (from config or create request). */ + teamDisplayName?: string; } /** Dismissal event — process died, all pending approvals for this team+run should be removed. */ @@ -853,3 +859,12 @@ export type ToolApprovalEvent = | ToolApprovalRequest | ToolApprovalDismiss | ToolApprovalAutoResolved; + +/** Result of reading a file for tool approval diff preview. */ +export interface ToolApprovalFileContent { + content: string; + exists: boolean; + truncated: boolean; + isBinary: boolean; + error?: string; +}