From 1ccc1432fcad4aa0fd9f3ffcaa76c488d0dfd853 Mon Sep 17 00:00:00 2001 From: iliya Date: Wed, 11 Mar 2026 18:16:40 +0200 Subject: [PATCH] feat: enhance task change handling and improve plugin catalog integration - Added a 'source' field to PluginCatalogItem to distinguish between official and third-party plugins. - Refactored ChangeExtractorService to improve caching mechanisms and normalize file paths for better consistency. - Updated TaskBoundaryParser to support task IDs with underscores, enhancing task identification. - Enhanced TeamMcpConfigBuilder to merge user-defined MCP configurations with generated ones, improving configuration management. - Improved UI components to display source information for plugins and MCP servers, enhancing user experience and clarity. --- .../catalog/PluginCatalogService.ts | 1 + .../services/team/ChangeExtractorService.ts | 100 +++++++--- src/main/services/team/TaskBoundaryParser.ts | 23 +-- .../services/team/TeamMcpConfigBuilder.ts | 85 ++++++++- .../services/team/TeamProvisioningService.ts | 4 +- .../extensions/mcp/McpServerCard.tsx | 27 +-- .../extensions/mcp/McpServerDetailDialog.tsx | 2 +- .../extensions/plugins/PluginCard.tsx | 20 +- .../extensions/plugins/PluginDetailDialog.tsx | 24 ++- .../extensions/plugins/PluginsPanel.tsx | 9 +- .../components/team/TeamDetailView.tsx | 37 +++- .../team/dialogs/GlobalTaskDetailDialog.tsx | 11 +- .../team/dialogs/TaskDetailDialog.tsx | 93 ++++++---- .../team/kanban/KanbanGridLayout.tsx | 9 +- .../components/team/kanban/KanbanTaskCard.tsx | 22 ++- .../team/review/ChangeReviewDialog.tsx | 6 +- src/renderer/index.css | 6 + .../BrowserGridLayoutRepository.ts | 20 +- .../layout-system/gridLayoutSchema.ts | 47 ++--- .../store/slices/changeReviewSlice.ts | 78 ++++++-- src/renderer/store/slices/teamSlice.ts | 11 +- src/renderer/utils/taskChangeRequest.ts | 97 ++++++++++ src/shared/types/extensions/plugin.ts | 1 + .../team/ChangeExtractorService.test.ts | 148 +++++++++++++++ .../services/team/TaskBoundaryParser.test.ts | 47 +++++ .../team/TeamMcpConfigBuilder.test.ts | 175 +++++++++++++++++- test/renderer/store/changeReviewSlice.test.ts | 173 +++++++++++++++++ test/renderer/store/extensionsSlice.test.ts | 1 + test/renderer/utils/taskChangeRequest.test.ts | 67 +++++++ .../shared/utils/extensionNormalizers.test.ts | 1 + 30 files changed, 1163 insertions(+), 182 deletions(-) create mode 100644 src/renderer/utils/taskChangeRequest.ts create mode 100644 test/main/services/team/ChangeExtractorService.test.ts create mode 100644 test/renderer/store/changeReviewSlice.test.ts create mode 100644 test/renderer/utils/taskChangeRequest.test.ts diff --git a/src/main/services/extensions/catalog/PluginCatalogService.ts b/src/main/services/extensions/catalog/PluginCatalogService.ts index bc06d6c7..27f73c4f 100644 --- a/src/main/services/extensions/catalog/PluginCatalogService.ts +++ b/src/main/services/extensions/catalog/PluginCatalogService.ts @@ -289,6 +289,7 @@ export class PluginCatalogService { marketplaceId: qualifiedName, qualifiedName, name: raw.name, + source: 'official', description: raw.description ?? '', category: raw.category ?? 'other', author: raw.author, diff --git a/src/main/services/team/ChangeExtractorService.ts b/src/main/services/team/ChangeExtractorService.ts index 8bff0c22..e7d9d90a 100644 --- a/src/main/services/team/ChangeExtractorService.ts +++ b/src/main/services/team/ChangeExtractorService.ts @@ -132,19 +132,25 @@ export class ChangeExtractorService { } ): Promise { const includeDetails = options?.summaryOnly !== true; - const cacheKey = `task:${teamName}:${taskId}`; + const taskMeta = await this.readTaskMeta(teamName, taskId); + const effectiveOptions = { + owner: options?.owner ?? taskMeta?.owner, + status: options?.status ?? taskMeta?.status, + intervals: options?.intervals ?? taskMeta?.intervals, + since: options?.since, + }; + const cacheKey = this.buildTaskChangeCacheKey( + teamName, + taskId, + effectiveOptions, + includeDetails + ); const cached = includeDetails ? this.taskChangeCache.get(cacheKey) : undefined; if (cached && cached.expiresAt > Date.now()) { return cached.data; } - const taskMeta = await this.readTaskMeta(teamName, taskId); - const logs = await this.logsFinder.findLogsForTask(teamName, taskId, { - owner: options?.owner ?? taskMeta?.owner, - status: options?.status ?? taskMeta?.status, - intervals: options?.intervals ?? taskMeta?.intervals, - since: options?.since, - }); + const logs = await this.logsFinder.findLogsForTask(teamName, taskId, effectiveOptions); const logRefs = await this.resolveLogFileRefs(teamName, logs); if (logRefs.length === 0) { const empty = this.emptyTaskChangeSet(teamName, taskId); @@ -171,7 +177,7 @@ export class ChangeExtractorService { // Если scope не найден — try deterministic interval scoping, else fallback to whole file if (allScopes.length === 0) { - const intervals = options?.intervals ?? taskMeta?.intervals; + const intervals = effectiveOptions.intervals; if (Array.isArray(intervals) && intervals.length > 0) { const { files, toolUseIds, startTimestamp, endTimestamp } = await this.extractIntervalScopedChanges(logRefs, intervals, projectPath, includeDetails); @@ -345,7 +351,8 @@ export class ChangeExtractorService { status: typeof parsed.status === 'string' ? parsed.status : undefined, intervals: derivedIntervals, }; - } catch { + } catch (error) { + logger.debug(`Failed to read task meta for ${teamName}/${taskId}: ${String(error)}`); return null; } } @@ -532,7 +539,7 @@ export class ChangeExtractorService { const replaceAll = input.replace_all === true; if (targetPath) { - seenFiles.add(targetPath); + seenFiles.add(this.normalizeFilePathKey(targetPath)); snippets.push({ toolUseId, filePath: targetPath, @@ -551,8 +558,9 @@ export class ChangeExtractorService { const writeContent = typeof input.content === 'string' ? input.content : ''; if (targetPath) { - const isNew = !seenFiles.has(targetPath); - seenFiles.add(targetPath); + const normalizedTargetPath = this.normalizeFilePathKey(targetPath); + const isNew = !seenFiles.has(normalizedTargetPath); + seenFiles.add(normalizedTargetPath); snippets.push({ toolUseId, filePath: targetPath, @@ -571,7 +579,7 @@ export class ChangeExtractorService { const edits = Array.isArray(input.edits) ? input.edits : []; if (targetPath) { - seenFiles.add(targetPath); + seenFiles.add(this.normalizeFilePathKey(targetPath)); for (const edit of edits) { if (!edit || typeof edit !== 'object') continue; const editObj = edit as Record; @@ -668,24 +676,30 @@ export class ChangeExtractorService { projectPath?: string, includeDetails = true ): FileChangeSummary[] { - const fileMap = new Map(); + const fileMap = new Map< + string, + { filePath: string; snippets: SnippetDiff[]; isNewFile: boolean } + >(); for (const snippet of snippets) { // Пропускаем snippets с ошибками при агрегации if (snippet.isError) continue; - const existing = fileMap.get(snippet.filePath); + const normalizedFilePath = this.normalizeFilePathKey(snippet.filePath); + const existing = fileMap.get(normalizedFilePath); if (existing) { existing.snippets.push(snippet); } else { - fileMap.set(snippet.filePath, { + fileMap.set(normalizedFilePath, { + filePath: snippet.filePath, snippets: [snippet], isNewFile: snippet.type === 'write-new', }); } } - return [...fileMap.entries()].map(([fp, data]) => { + return [...fileMap.values()].map((data) => { + const fp = data.filePath; let totalAdded = 0; let totalRemoved = 0; for (const s of data.snippets) { @@ -854,16 +868,12 @@ export class ChangeExtractorService { projectPath?: string, includeDetails = true ): Promise { - const allFiles: FileChangeSummary[] = []; + const allSnippets: SnippetDiff[] = []; for (const ref of logRefs) { - const files = await this.extractAllChanges( - ref.filePath, - ref.memberName, - projectPath, - includeDetails - ); - allFiles.push(...files); + const snippets = await this.parseJSONLFile(ref.filePath); + allSnippets.push(...snippets); } + const allFiles = this.aggregateByFile(allSnippets, projectPath, includeDetails); const fallbackScope: TaskChangeScope = { taskId, @@ -916,4 +926,42 @@ export class ChangeExtractorService { warnings: ['No log files found for this task.'], }; } + + private buildTaskChangeCacheKey( + teamName: string, + taskId: string, + options: { + owner?: string; + status?: string; + intervals?: { startedAt: string; completedAt?: string }[]; + since?: string; + }, + includeDetails: boolean + ): string { + const owner = typeof options.owner === 'string' ? options.owner.trim() : ''; + const status = typeof options.status === 'string' ? options.status.trim() : ''; + const since = typeof options.since === 'string' ? options.since : ''; + const intervals = Array.isArray(options.intervals) + ? options.intervals.map((interval) => ({ + startedAt: interval.startedAt, + completedAt: interval.completedAt ?? '', + })) + : []; + + return JSON.stringify({ + type: 'task', + teamName, + taskId, + includeDetails, + owner, + status, + since, + intervals, + }); + } + + private normalizeFilePathKey(filePath: string): string { + const normalized = filePath.replace(/\\/g, '/'); + return normalized.replace(/^[A-Z]:/, (drive) => drive.toLowerCase()); + } } diff --git a/src/main/services/team/TaskBoundaryParser.ts b/src/main/services/team/TaskBoundaryParser.ts index 797b8608..d3b7512d 100644 --- a/src/main/services/team/TaskBoundaryParser.ts +++ b/src/main/services/team/TaskBoundaryParser.ts @@ -35,6 +35,13 @@ const MCP_TASK_BOUNDARY_TOOLS = new Set(['task_start', 'task_complete', 'task_se type DetectedMechanism = 'TaskUpdate' | 'mcp' | 'none'; +function extractTaskId(input: Record): string { + const rawTaskId = input.taskId ?? input.task_id; + if (typeof rawTaskId === 'string') return rawTaskId; + if (typeof rawTaskId === 'number') return String(rawTaskId); + return ''; +} + function pickDetectedMechanism( current: DetectedMechanism, next: Exclude @@ -191,13 +198,7 @@ export class TaskBoundaryParser { const input = b.input as Record | undefined; if (!input) continue; - const rawTaskId = input.taskId; - const taskId = - typeof rawTaskId === 'string' - ? rawTaskId - : typeof rawTaskId === 'number' - ? String(rawTaskId) - : ''; + const taskId = extractTaskId(input); if (!taskId) continue; const status = typeof input.status === 'string' ? input.status : ''; @@ -243,13 +244,7 @@ export class TaskBoundaryParser { const input = b.input as Record | undefined; if (!input) continue; - const rawTaskId = input.taskId; - const taskId = - typeof rawTaskId === 'string' - ? rawTaskId - : typeof rawTaskId === 'number' - ? String(rawTaskId) - : ''; + const taskId = extractTaskId(input); if (!taskId) continue; let event: TaskBoundaryEvent = null; diff --git a/src/main/services/team/TeamMcpConfigBuilder.ts b/src/main/services/team/TeamMcpConfigBuilder.ts index 0e70d756..038cbdde 100644 --- a/src/main/services/team/TeamMcpConfigBuilder.ts +++ b/src/main/services/team/TeamMcpConfigBuilder.ts @@ -4,6 +4,9 @@ import * as fs from 'fs'; import * as os from 'os'; import * as path from 'path'; +import { getHomeDir } from '@main/utils/pathDecoder'; +import { createLogger } from '@shared/utils/logger'; + import { atomicWriteAsync } from './atomicWrite'; interface McpLaunchSpec { @@ -12,6 +15,14 @@ interface McpLaunchSpec { } const MCP_SERVER_NAME = 'agent-teams'; +const logger = createLogger('Service:TeamMcpConfigBuilder'); +const USER_MCP_CONFIG_NAME = '.claude.json'; + +type McpServerConfig = Record; + +function isRecord(value: unknown): value is Record { + return !!value && typeof value === 'object' && !Array.isArray(value); +} function getWorkspaceRoot(): string { return process.cwd(); @@ -94,22 +105,25 @@ async function resolveMcpLaunchSpec(): Promise { } export class TeamMcpConfigBuilder { - async writeConfigFile(): Promise { + async writeConfigFile(_projectPath?: string): Promise { const launchSpec = await resolveMcpLaunchSpec(); const configDir = path.join(os.tmpdir(), 'claude-team-mcp'); const configPath = path.join(configDir, `agent-teams-mcp-${randomUUID()}.json`); + const userServers = await this.readUserMcpServers(); + const generatedServers: Record = { + [MCP_SERVER_NAME]: { + command: launchSpec.command, + args: launchSpec.args, + }, + }; + const mergedServers = this.mergeServers(userServers, generatedServers); await fs.promises.mkdir(configDir, { recursive: true }); await atomicWriteAsync( configPath, JSON.stringify( { - mcpServers: { - [MCP_SERVER_NAME]: { - command: launchSpec.command, - args: launchSpec.args, - }, - }, + mcpServers: mergedServers, }, null, 2 @@ -118,4 +132,61 @@ export class TeamMcpConfigBuilder { return configPath; } + + private async readUserMcpServers(): Promise> { + const configPath = path.join(getHomeDir(), USER_MCP_CONFIG_NAME); + return this.readMcpServersFromFile(configPath, 'user'); + } + + private async readMcpServersFromFile( + filePath: string, + scope: 'user' + ): Promise> { + try { + const raw = await fs.promises.readFile(filePath, 'utf8'); + const parsed = JSON.parse(raw) as Record; + const mcpServers = parsed.mcpServers; + if (!isRecord(mcpServers)) { + return {}; + } + + return Object.fromEntries( + Object.entries(mcpServers).filter(([, config]) => isRecord(config)) + ) as Record; + } catch (error) { + const err = error as NodeJS.ErrnoException; + if (err.code === 'ENOENT') { + return {}; + } + + logger.warn( + `Failed to read ${scope} MCP config from ${filePath}: ${ + error instanceof Error ? error.message : String(error) + }` + ); + return {}; + } + } + + private mergeServers( + userServers: Record, + generatedServers: Record + ): Record { + const duplicates = Object.keys(userServers).filter((name) => + Object.hasOwn(generatedServers, name) + ); + + if (duplicates.length > 0) { + logger.info(`Merging MCP configs with overrides for: ${duplicates.join(', ')}`); + } + + // We inline only top-level user MCP into --mcp-config. + // Project/local scopes are still loaded natively by Claude via + // --setting-sources user,project,local, which preserves documented precedence: + // local > project > user. Generated agent-teams must always win on name collision. + return { + ...userServers, + ...generatedServers, + }; + } } diff --git a/src/main/services/team/TeamProvisioningService.ts b/src/main/services/team/TeamProvisioningService.ts index 2211ed1c..9010dfb5 100644 --- a/src/main/services/team/TeamProvisioningService.ts +++ b/src/main/services/team/TeamProvisioningService.ts @@ -2387,7 +2387,7 @@ export class TeamProvisioningService { const { env: shellEnv } = await this.buildProvisioningEnv(); let mcpConfigPath: string; try { - mcpConfigPath = await this.mcpConfigBuilder.writeConfigFile(); + mcpConfigPath = await this.mcpConfigBuilder.writeConfigFile(request.cwd); } catch (error) { this.runs.delete(runId); this.activeByTeam.delete(request.teamName); @@ -2734,7 +2734,7 @@ export class TeamProvisioningService { const { env: shellEnv } = await this.buildProvisioningEnv(); let mcpConfigPath: string; try { - mcpConfigPath = await this.mcpConfigBuilder.writeConfigFile(); + mcpConfigPath = await this.mcpConfigBuilder.writeConfigFile(request.cwd); } catch (error) { this.runs.delete(runId); this.activeByTeam.delete(request.teamName); diff --git a/src/renderer/components/extensions/mcp/McpServerCard.tsx b/src/renderer/components/extensions/mcp/McpServerCard.tsx index cc3e2dfd..0b9d023d 100644 --- a/src/renderer/components/extensions/mcp/McpServerCard.tsx +++ b/src/renderer/components/extensions/mcp/McpServerCard.tsx @@ -17,16 +17,11 @@ import { Cloud, Clock, Globe, KeyRound, Lock, Monitor, Star, Tag, Wrench } from import { Github as GithubIcon } from 'lucide-react'; import { InstallButton } from '../common/InstallButton'; +import { SourceBadge } from '../common/SourceBadge'; import { sanitizeMcpServerName } from '@shared/utils/extensionNormalizers'; import type { McpCatalogItem, McpServerDiagnostic } from '@shared/types/extensions'; -/** Ribbon colors by source */ -const RIBBON_STYLES: Record = { - official: 'bg-blue-500/90 text-white', - glama: 'bg-zinc-600/90 text-zinc-200', -}; - interface McpServerCardProps { server: McpCatalogItem; isInstalled: boolean; @@ -81,17 +76,8 @@ export const McpServerCard = ({ isInstalled ? 'border-l-2 border-border border-l-emerald-500/30' : 'border-border' }`} > - {/* Source ribbon (top-left corner) */} -
-
- {server.source === 'official' ? 'Official' : 'Glama'} -
-
- {/* Header: icon + name */} -
+
{/* Server icon (only when available) */} {hasIcon && (
@@ -105,7 +91,14 @@ export const McpServerCard = ({ )}
-

{server.name}

+
+

{server.name}

+ {server.source !== 'official' && ( +
+ +
+ )} +
{isInstalled && ( )} - + {server.source !== 'official' && }
diff --git a/src/renderer/components/extensions/plugins/PluginCard.tsx b/src/renderer/components/extensions/plugins/PluginCard.tsx index 43f44e02..33517bc6 100644 --- a/src/renderer/components/extensions/plugins/PluginCard.tsx +++ b/src/renderer/components/extensions/plugins/PluginCard.tsx @@ -18,16 +18,20 @@ import type { EnrichedPlugin } from '@shared/types/extensions'; interface PluginCardProps { plugin: EnrichedPlugin; + index: number; onClick: (pluginId: string) => void; } -export const PluginCard = ({ plugin, onClick }: PluginCardProps): React.JSX.Element => { +export const PluginCard = ({ plugin, index, onClick }: PluginCardProps): React.JSX.Element => { const capabilities = inferCapabilities(plugin); const category = normalizeCategory(plugin.category); const installProgress = useStore((s) => s.pluginInstallProgress[plugin.pluginId] ?? 'idle'); const installPlugin = useStore((s) => s.installPlugin); const uninstallPlugin = useStore((s) => s.uninstallPlugin); const installError = useStore((s) => s.installErrors[plugin.pluginId]); + const baseStriped = index % 2 === 0; + const smStriped = Math.floor(index / 2) % 2 === 0; + const xlStriped = Math.floor(index / 3) % 2 === 0; return (
+ {plugin.source === 'official' && ( +
+
+ Official +
+
+ )} + {/* Header: name + status/meta */}
diff --git a/src/renderer/components/extensions/plugins/PluginDetailDialog.tsx b/src/renderer/components/extensions/plugins/PluginDetailDialog.tsx index 45d731b6..83cf3d00 100644 --- a/src/renderer/components/extensions/plugins/PluginDetailDialog.tsx +++ b/src/renderer/components/extensions/plugins/PluginDetailDialog.tsx @@ -32,6 +32,7 @@ import { ExternalLink, Loader2, Mail } from 'lucide-react'; import { InstallButton } from '../common/InstallButton'; import { InstallCountBadge } from '../common/InstallCountBadge'; +import { SourceBadge } from '../common/SourceBadge'; import { MarkdownViewer } from '@renderer/components/chat/viewers/MarkdownViewer'; @@ -87,14 +88,17 @@ export const PluginDetailDialog = ({ {plugin.name} {plugin.description}
- {plugin.isInstalled && ( - - Installed - - )} +
+ {plugin.isInstalled && ( + + Installed + + )} + +
@@ -108,6 +112,10 @@ export const PluginDetailDialog = ({ Category

{category}

+
+ Source +

{plugin.source}

+
{plugin.version && (
Version diff --git a/src/renderer/components/extensions/plugins/PluginsPanel.tsx b/src/renderer/components/extensions/plugins/PluginsPanel.tsx index 1ad168d8..5177538e 100644 --- a/src/renderer/components/extensions/plugins/PluginsPanel.tsx +++ b/src/renderer/components/extensions/plugins/PluginsPanel.tsx @@ -359,8 +359,13 @@ export const PluginsPanel = ({ {!loading && !error && filtered.length > 0 && (
- {filtered.map((plugin) => ( - + {filtered.map((plugin, index) => ( + ))}
)} diff --git a/src/renderer/components/team/TeamDetailView.tsx b/src/renderer/components/team/TeamDetailView.tsx index 50e4876f..c2d27741 100644 --- a/src/renderer/components/team/TeamDetailView.tsx +++ b/src/renderer/components/team/TeamDetailView.tsx @@ -26,6 +26,10 @@ import { formatProjectPath } from '@renderer/utils/pathDisplay'; import { buildTaskCountsByOwner, normalizePath } from '@renderer/utils/pathNormalize'; import { nameColorSet } from '@renderer/utils/projectColor'; import { resolveProjectIdByPath } from '@renderer/utils/projectLookup'; +import { + buildTaskChangeRequestOptions, + type TaskChangeRequestOptions, +} from '@renderer/utils/taskChangeRequest'; import { stripAgentBlocks } from '@shared/constants/agentBlocks'; import { deriveTaskDisplayId, formatTaskDisplayLabel } from '@shared/utils/taskIdentity'; import { @@ -175,6 +179,7 @@ export const TeamDetailView = ({ teamName }: TeamDetailViewProps): React.JSX.Ele memberName?: string; taskId?: string; initialFilePath?: string; + taskChangeRequestOptions?: TaskChangeRequestOptions; }>({ open: false, mode: 'task' }); // Active teams for conflict warning in LaunchTeamDialog @@ -723,6 +728,7 @@ export const TeamDetailView = ({ teamName }: TeamDetailViewProps): React.JSX.Ele mode: 'task', taskId: pendingReviewRequest.taskId, initialFilePath: pendingReviewRequest.filePath, + taskChangeRequestOptions: pendingReviewRequest.requestOptions, }); if (pendingReviewRequest.filePath) { selectReviewFile(pendingReviewRequest.filePath); @@ -763,18 +769,34 @@ export const TeamDetailView = ({ teamName }: TeamDetailViewProps): React.JSX.Ele [teamName, softDeleteTask] ); - const handleViewChanges = useCallback((taskId: string) => { - setReviewDialogState({ open: true, mode: 'task', taskId }); - }, []); + const handleViewChanges = useCallback( + (taskId: string) => { + const task = taskMap.get(taskId); + setReviewDialogState({ + open: true, + mode: 'task', + taskId, + taskChangeRequestOptions: task ? buildTaskChangeRequestOptions(task) : {}, + }); + }, + [taskMap] + ); const handleViewChangesForFile = useCallback( (taskId: string, filePath?: string) => { - setReviewDialogState({ open: true, mode: 'task', taskId }); + const task = taskMap.get(taskId); + setReviewDialogState({ + open: true, + mode: 'task', + taskId, + initialFilePath: filePath, + taskChangeRequestOptions: task ? buildTaskChangeRequestOptions(task) : {}, + }); if (filePath) { selectReviewFile(filePath); } }, - [selectReviewFile] + [selectReviewFile, taskMap] ); const handleDeleteTeam = useCallback((): void => { @@ -1873,7 +1895,9 @@ export const TeamDetailView = ({ teamName }: TeamDetailViewProps): React.JSX.Ele setReviewDialogState((prev) => ({ ...prev, open, - ...(open ? {} : { initialFilePath: undefined }), + ...(open + ? {} + : { initialFilePath: undefined, taskChangeRequestOptions: undefined }), })) } teamName={teamName} @@ -1881,6 +1905,7 @@ export const TeamDetailView = ({ teamName }: TeamDetailViewProps): React.JSX.Ele memberName={reviewDialogState.memberName} taskId={reviewDialogState.taskId} initialFilePath={reviewDialogState.initialFilePath} + taskChangeRequestOptions={reviewDialogState.taskChangeRequestOptions} projectPath={data.config.projectPath} onEditorAction={handleEditorAction} /> diff --git a/src/renderer/components/team/dialogs/GlobalTaskDetailDialog.tsx b/src/renderer/components/team/dialogs/GlobalTaskDetailDialog.tsx index 41b14484..0d0327d9 100644 --- a/src/renderer/components/team/dialogs/GlobalTaskDetailDialog.tsx +++ b/src/renderer/components/team/dialogs/GlobalTaskDetailDialog.tsx @@ -1,6 +1,7 @@ import { useCallback, useEffect, useMemo } from 'react'; import { useStore } from '@renderer/store'; +import { buildTaskChangeRequestOptions } from '@renderer/utils/taskChangeRequest'; import { ExternalLink } from 'lucide-react'; import { useShallow } from 'zustand/react/shallow'; @@ -99,11 +100,17 @@ export const GlobalTaskDetailDialog = (): React.JSX.Element | null => { const handleViewChanges = useCallback( (viewTaskId: string, filePath?: string) => { - setPendingReviewRequest({ taskId: viewTaskId, filePath }); + const targetTask = taskMap.get(viewTaskId); + if (!targetTask) return; + setPendingReviewRequest({ + taskId: viewTaskId, + filePath, + requestOptions: buildTaskChangeRequestOptions(targetTask), + }); closeGlobalTaskDetail(); openTeamTab(teamName); }, - [closeGlobalTaskDetail, openTeamTab, setPendingReviewRequest, teamName] + [closeGlobalTaskDetail, openTeamTab, setPendingReviewRequest, taskMap, teamName] ); if (!globalTaskDetail) return null; diff --git a/src/renderer/components/team/dialogs/TaskDetailDialog.tsx b/src/renderer/components/team/dialogs/TaskDetailDialog.tsx index e08df8ae..d9b015f4 100644 --- a/src/renderer/components/team/dialogs/TaskDetailDialog.tsx +++ b/src/renderer/components/team/dialogs/TaskDetailDialog.tsx @@ -34,6 +34,7 @@ import { TASK_STATUS_LABELS, TASK_STATUS_STYLES, } from '@renderer/utils/memberHelpers'; +import { buildTaskChangeRequestOptions, deriveTaskSince } from '@renderer/utils/taskChangeRequest'; import { getTaskKanbanColumn } from '@shared/utils/reviewState'; import { deriveTaskDisplayId, formatTaskDisplayLabel } from '@shared/utils/taskIdentity'; import { formatDistanceToNow } from 'date-fns'; @@ -74,29 +75,6 @@ import type { TeamTaskWithKanban, } from '@shared/types'; -const TASK_SINCE_GRACE_MS = 2 * 60 * 1000; - -function deriveTaskSince(task: TeamTaskWithKanban | null): string | undefined { - if (!task) return undefined; - const sources: string[] = []; - if (task.createdAt) sources.push(task.createdAt); - if (Array.isArray(task.workIntervals)) { - for (const i of task.workIntervals) { - if (i.startedAt) sources.push(i.startedAt); - } - } - if (Array.isArray(task.historyEvents)) { - for (const e of task.historyEvents) { - if (e.timestamp) sources.push(e.timestamp); - } - } - if (sources.length === 0) return undefined; - const earliest = sources.reduce((a, b) => (a < b ? a : b)); - const d = new Date(earliest); - d.setTime(d.getTime() - TASK_SINCE_GRACE_MS); - return d.toISOString(); -} - interface TaskDetailDialogProps { open: boolean; loading?: boolean; @@ -136,6 +114,7 @@ export const TaskDetailDialog = ({ const colorMap = useMemo(() => buildMemberColorMap(members), [members]); const currentTask = task ? (taskMap.get(task.id) ?? task) : null; const updateTaskFields = useStore((s) => s.updateTaskFields); + const recordTaskHasChanges = useStore((s) => s.recordTaskHasChanges); const [logsRefreshing, setLogsRefreshing] = useState(false); const [executionPreviewOnline, setExecutionPreviewOnline] = useState(false); @@ -289,19 +268,39 @@ export const TaskDetailDialog = ({ // Lazy-load task changes when dialog is open and task is completed const isTaskCompleted = currentTask?.status === 'completed'; const taskSince = useMemo(() => deriveTaskSince(currentTask), [currentTask]); + const taskChangeRequestOptions = useMemo( + () => (currentTask ? buildTaskChangeRequestOptions(currentTask) : null), + [currentTask] + ); + const taskChangeSummaryOptions = useMemo( + () => + currentTask + ? buildTaskChangeRequestOptions(currentTask, { + since: taskSince, + summaryOnly: true, + }) + : null, + [currentTask, taskSince] + ); const setTaskNeedsClarification = useStore((s) => s.setTaskNeedsClarification); const loadTaskChangeSummary = useCallback(async (): Promise => { - if (!currentTask || variant !== 'team' || !isTaskCompleted || !onViewChanges) return null; - const data = await api.review.getTaskChanges(teamName, currentTask.id, { - owner: currentTask.owner, - status: currentTask.status, - intervals: currentTask.workIntervals, - since: taskSince, - summaryOnly: true, - }); + if ( + !currentTask || + !taskChangeSummaryOptions || + variant !== 'team' || + !isTaskCompleted || + !onViewChanges + ) { + return null; + } + const data = await api.review.getTaskChanges( + teamName, + currentTask.id, + taskChangeSummaryOptions + ); return data.files; - }, [currentTask, isTaskCompleted, onViewChanges, teamName, taskSince, variant]); + }, [currentTask, isTaskCompleted, onViewChanges, taskChangeSummaryOptions, teamName, variant]); useEffect(() => { if (variant !== 'team') return; @@ -312,7 +311,17 @@ export const TaskDetailDialog = ({ setTaskChangesError(null); void loadTaskChangeSummary() .then((files) => { - if (!cancelled) setTaskChangesFiles(files ?? null); + if (!cancelled) { + setTaskChangesFiles(files ?? null); + if (currentTask && taskChangeRequestOptions) { + recordTaskHasChanges( + teamName, + currentTask.id, + taskChangeRequestOptions, + !!files?.length + ); + } + } }) .catch((error) => { if (!cancelled) { @@ -346,7 +355,12 @@ export const TaskDetailDialog = ({ setTaskChangesLoading(true); setTaskChangesError(null); void loadTaskChangeSummary() - .then((files) => setTaskChangesFiles(files ?? null)) + .then((files) => { + setTaskChangesFiles(files ?? null); + if (currentTask && taskChangeRequestOptions) { + recordTaskHasChanges(teamName, currentTask.id, taskChangeRequestOptions, !!files?.length); + } + }) .catch((error) => { setTaskChangesFiles(null); setTaskChangesError( @@ -354,7 +368,16 @@ export const TaskDetailDialog = ({ ); }) .finally(() => setTaskChangesLoading(false)); - }, [currentTask, isTaskCompleted, onViewChanges, loadTaskChangeSummary, variant]); + }, [ + currentTask, + isTaskCompleted, + onViewChanges, + loadTaskChangeSummary, + recordTaskHasChanges, + taskChangeRequestOptions, + teamName, + variant, + ]); const handleDependencyClick = (taskId: string): void => { handleClose(); diff --git a/src/renderer/components/team/kanban/KanbanGridLayout.tsx b/src/renderer/components/team/kanban/KanbanGridLayout.tsx index b8e54ca8..9e53c782 100644 --- a/src/renderer/components/team/kanban/KanbanGridLayout.tsx +++ b/src/renderer/components/team/kanban/KanbanGridLayout.tsx @@ -101,7 +101,7 @@ export const KanbanGridLayout = ({ }: KanbanGridLayoutProps): React.JSX.Element => { const columnMap = useMemo(() => new Map(columns.map((column) => [column.id, column])), [columns]); const visibleColumnIds = useMemo(() => columns.map((column) => column.id), [columns]); - const { visibleItems, applyVisibleItems } = usePersistedGridLayout({ + const { visibleItems, applyVisibleItems, isLoaded } = usePersistedGridLayout({ scopeKey: `${GRID_SCOPE_PREFIX}:${teamName}`, allItemIds: allColumnIds, visibleItemIds: visibleColumnIds, @@ -115,8 +115,9 @@ export const KanbanGridLayout = ({ ); useEffect(() => { + if (!isLoaded) return; setRenderLayout(visibleItems.map(toReactGridLayoutItem)); - }, [visibleItems]); + }, [isLoaded, visibleItems]); const applyReactGridLayout = useCallback( (layout: Layout, options?: { persist?: boolean }) => { @@ -128,6 +129,10 @@ export const KanbanGridLayout = ({ [applyVisibleItems] ); + if (!isLoaded) { + return
; + } + return (
buildTaskChangeRequestOptions(task), [task]); + const cacheKey = useMemo( + () => buildTaskChangePresenceKey(teamName, task.id, taskChangeRequestOptions), + [teamName, task.id, taskChangeRequestOptions] + ); const taskHasChanges = useStore((s) => s.taskHasChanges[cacheKey]); const checkTaskHasChanges = useStore((s) => s.checkTaskHasChanges); useEffect(() => { if (showChangesColumn && task.status === 'completed' && taskHasChanges !== true) { - void checkTaskHasChanges(teamName, task.id); + void checkTaskHasChanges(teamName, task.id, taskChangeRequestOptions); } - }, [showChangesColumn, task.status, task.id, teamName, taskHasChanges, checkTaskHasChanges]); + }, [ + showChangesColumn, + task.status, + task.id, + teamName, + taskHasChanges, + checkTaskHasChanges, + taskChangeRequestOptions, + ]); const isReviewManual = columnId === 'review' && !hasReviewers; const multiButton = diff --git a/src/renderer/components/team/review/ChangeReviewDialog.tsx b/src/renderer/components/team/review/ChangeReviewDialog.tsx index d8a4f1fd..ed3a0d12 100644 --- a/src/renderer/components/team/review/ChangeReviewDialog.tsx +++ b/src/renderer/components/team/review/ChangeReviewDialog.tsx @@ -10,6 +10,7 @@ import { useViewedFiles } from '@renderer/hooks/useViewedFiles'; import { cn } from '@renderer/lib/utils'; import { useStore } from '@renderer/store'; import { getFileHunkCount, REVIEW_INSTANT_APPLY } from '@renderer/store/slices/changeReviewSlice'; +import { type TaskChangeRequestOptions } from '@renderer/utils/taskChangeRequest'; import { buildSelectionAction } from '@renderer/utils/buildSelectionAction'; import { buildSelectionInfo, SELECTION_DEBOUNCE_MS } from '@renderer/utils/codemirrorSelectionInfo'; import { sortItemsAsTree } from '@renderer/utils/fileTreeBuilder'; @@ -47,6 +48,7 @@ interface ChangeReviewDialogProps { memberName?: string; taskId?: string; initialFilePath?: string; + taskChangeRequestOptions?: TaskChangeRequestOptions; projectPath?: string; onEditorAction?: (action: EditorSelectionAction) => void; } @@ -63,6 +65,7 @@ export const ChangeReviewDialog = ({ memberName, taskId, initialFilePath, + taskChangeRequestOptions, projectPath, onEditorAction, }: ChangeReviewDialogProps): React.ReactElement | null => { @@ -692,7 +695,7 @@ export const ChangeReviewDialog = ({ if (mode === 'agent' && memberName) { void fetchAgentChanges(teamName, memberName); } else if (mode === 'task' && taskId) { - void fetchTaskChanges(teamName, taskId); + void fetchTaskChanges(teamName, taskId, taskChangeRequestOptions ?? {}); } // On close — clear only volatile cache, keep decisions in store @@ -703,6 +706,7 @@ export const ChangeReviewDialog = ({ teamName, memberName, taskId, + taskChangeRequestOptions, decisionScopeKey, fetchAgentChanges, fetchTaskChanges, diff --git a/src/renderer/index.css b/src/renderer/index.css index 237f4d53..74fa0dba 100644 --- a/src/renderer/index.css +++ b/src/renderer/index.css @@ -318,6 +318,7 @@ width: 36px; height: 6px; transform: translateX(-50%); + cursor: ns-resize; } .kanban-grid-resize-handle-n { @@ -334,6 +335,7 @@ width: 6px; height: 36px; transform: translateY(-50%); + cursor: ew-resize; } .kanban-grid-resize-handle-e { @@ -355,21 +357,25 @@ .kanban-grid-resize-handle-ne { top: -4px; right: -4px; + cursor: nesw-resize; } .kanban-grid-resize-handle-nw { top: -4px; left: -4px; + cursor: nwse-resize; } .kanban-grid-resize-handle-se { right: -4px; bottom: -4px; + cursor: nwse-resize; } .kanban-grid-resize-handle-sw { left: -4px; bottom: -4px; + cursor: nesw-resize; } /* File icon glow — halo so dark icons stay visible on dark backgrounds */ diff --git a/src/renderer/services/layout-system/BrowserGridLayoutRepository.ts b/src/renderer/services/layout-system/BrowserGridLayoutRepository.ts index 3a39c968..27b07ae6 100644 --- a/src/renderer/services/layout-system/BrowserGridLayoutRepository.ts +++ b/src/renderer/services/layout-system/BrowserGridLayoutRepository.ts @@ -37,26 +37,36 @@ function removeLocalStorage(key: string): void { } } +function pickNewestState( + ...states: Array +): PersistedGridLayoutState | null { + return states.reduce((latest, current) => { + if (!current) return latest; + if (!latest) return current; + return current.updatedAt >= latest.updatedAt ? current : latest; + }, null); +} + export class BrowserGridLayoutRepository implements GridLayoutRepository { private idbUnavailable = false; private readonly fallbackStore = new Map(); async load(scopeKey: string): Promise { const key = storageKey(scopeKey); + const memoryState = this.fallbackStore.get(key) ?? null; + const localState = readLocalStorage(key); + let idbState: PersistedGridLayoutState | null = null; if (!this.idbUnavailable) { try { const stored = await get(key); - const sanitized = sanitizePersistedGridLayoutState(stored); - if (sanitized) { - return sanitized; - } + idbState = sanitizePersistedGridLayoutState(stored); } catch { this.idbUnavailable = true; } } - return this.fallbackStore.get(key) ?? readLocalStorage(key); + return pickNewestState(memoryState, localState, idbState); } async save(scopeKey: string, state: PersistedGridLayoutState): Promise { diff --git a/src/renderer/services/layout-system/gridLayoutSchema.ts b/src/renderer/services/layout-system/gridLayoutSchema.ts index 506ebb16..f4ee033d 100644 --- a/src/renderer/services/layout-system/gridLayoutSchema.ts +++ b/src/renderer/services/layout-system/gridLayoutSchema.ts @@ -120,41 +120,18 @@ export function projectVisibleGridLayoutItems( cols: number ): PersistedGridLayoutItem[] { const visibleIdSet = new Set(visibleIds); - const visibleItems = allItems + return allItems .filter((item) => visibleIdSet.has(item.id)) + .map((item) => { + const width = Math.min(Math.max(1, item.w), cols); + const maxX = Math.max(0, cols - width); + + return { + ...item, + w: width, + x: Math.min(Math.max(0, item.x), maxX), + y: Math.max(0, item.y), + }; + }) .sort((a, b) => (a.y === b.y ? a.x - b.x : a.y - b.y)); - - if (visibleItems.length === 0) { - return []; - } - - const fillWidth = - visibleItems.length <= 3 ? Math.max(1, Math.floor(cols / visibleItems.length)) : 0; - const projected: PersistedGridLayoutItem[] = []; - let currentX = 0; - let currentY = 0; - let rowHeight = 0; - - for (const item of visibleItems) { - const nextWidth = - visibleItems.length === 1 ? cols : Math.min(cols, Math.max(item.w, fillWidth || item.w)); - - if (currentX + nextWidth > cols) { - currentX = 0; - currentY += rowHeight; - rowHeight = 0; - } - - projected.push({ - ...item, - x: currentX, - y: currentY, - w: nextWidth, - }); - - currentX += nextWidth; - rowHeight = Math.max(rowHeight, item.h); - } - - return projected; } diff --git a/src/renderer/store/slices/changeReviewSlice.ts b/src/renderer/store/slices/changeReviewSlice.ts index 3e7b366a..3ba525ce 100644 --- a/src/renderer/store/slices/changeReviewSlice.ts +++ b/src/renderer/store/slices/changeReviewSlice.ts @@ -1,4 +1,8 @@ import { api } from '@renderer/api'; +import { + buildTaskChangePresenceKey, + type TaskChangeRequestOptions, +} from '@renderer/utils/taskChangeRequest'; import { computeDiffContextHash } from '@shared/utils/diffContextHash'; import { createLogger } from '@shared/utils/logger'; import { structuredPatch } from 'diff'; @@ -8,6 +12,7 @@ const taskChangesCheckInFlight = new Set(); /** Negative results cached with timestamp — recheck after 30s */ const taskChangesNegativeCache = new Map(); const NEGATIVE_CACHE_TTL = 30_000; +let latestTaskChangesRequestToken = 0; /** Debounce timer for persisting decisions to disk */ let persistDebounceTimer: ReturnType | null = null; @@ -78,12 +83,22 @@ export interface ChangeReviewSlice { // Editable diff state editedContents: Record; - /** Cache: "teamName:taskId" → true/false (has file changes) */ + /** Cache: "teamName:taskId:signature" → true/false (has file changes) */ taskHasChanges: Record; // Phase 1 actions fetchAgentChanges: (teamName: string, memberName: string) => Promise; - fetchTaskChanges: (teamName: string, taskId: string) => Promise; + fetchTaskChanges: ( + teamName: string, + taskId: string, + options: TaskChangeRequestOptions + ) => Promise; + recordTaskHasChanges: ( + teamName: string, + taskId: string, + options: TaskChangeRequestOptions, + hasChanges: boolean + ) => void; selectReviewFile: (filePath: string | null) => void; clearChangeReview: () => void; clearChangeReviewCache: () => void; @@ -145,7 +160,11 @@ export interface ChangeReviewSlice { saveEditedFile: (filePath: string, projectPath?: string) => Promise; // Task change availability - checkTaskHasChanges: (teamName: string, taskId: string) => Promise; + checkTaskHasChanges: ( + teamName: string, + taskId: string, + options: TaskChangeRequestOptions + ) => Promise; } /** @@ -278,18 +297,43 @@ export const createChangeReviewSlice: StateCreator { + recordTaskHasChanges: ( + teamName: string, + taskId: string, + options: TaskChangeRequestOptions, + hasChanges: boolean + ) => { + const cacheKey = buildTaskChangePresenceKey(teamName, taskId, options); + set((s) => ({ + taskHasChanges: { ...s.taskHasChanges, [cacheKey]: hasChanges }, + })); + if (hasChanges) { + taskChangesNegativeCache.delete(cacheKey); + } else { + taskChangesNegativeCache.set(cacheKey, Date.now()); + } + }, + + fetchTaskChanges: async (teamName: string, taskId: string, options: TaskChangeRequestOptions) => { + const requestToken = ++latestTaskChangesRequestToken; set({ changeSetLoading: true, changeSetError: null }); try { - const data = await api.review.getTaskChanges(teamName, taskId); - const cacheKey = `${teamName}:${taskId}`; + const data = await api.review.getTaskChanges(teamName, taskId, options); + if (requestToken !== latestTaskChangesRequestToken) return; + const cacheKey = buildTaskChangePresenceKey(teamName, taskId, options); set((s) => ({ activeChangeSet: data, changeSetLoading: false, selectedReviewFilePath: data.files[0]?.filePath ?? null, taskHasChanges: { ...s.taskHasChanges, [cacheKey]: data.files.length > 0 }, })); + if (data.files.length > 0) { + taskChangesNegativeCache.delete(cacheKey); + } else { + taskChangesNegativeCache.set(cacheKey, Date.now()); + } } catch (error) { + if (requestToken !== latestTaskChangesRequestToken) return; const message = error instanceof Error ? error.message : 'Failed to fetch task changes'; logger.error('fetchTaskChanges error:', message); set({ changeSetError: message, changeSetLoading: false }); @@ -301,6 +345,7 @@ export const createChangeReviewSlice: StateCreator { + latestTaskChangesRequestToken++; set({ activeChangeSet: null, changeSetLoading: false, @@ -320,6 +365,7 @@ export const createChangeReviewSlice: StateCreator { + latestTaskChangesRequestToken++; set({ activeChangeSet: null, changeSetLoading: false, @@ -337,6 +383,7 @@ export const createChangeReviewSlice: StateCreator { + latestTaskChangesRequestToken++; set({ activeChangeSet: null, changeSetLoading: false, @@ -978,8 +1025,12 @@ export const createChangeReviewSlice: StateCreator { - const cacheKey = `${teamName}:${taskId}`; + checkTaskHasChanges: async ( + teamName: string, + taskId: string, + options: TaskChangeRequestOptions + ) => { + const cacheKey = buildTaskChangePresenceKey(teamName, taskId, options); // Positive results are final — no need to recheck if (get().taskHasChanges[cacheKey] === true) return; // Prevent duplicate in-flight requests @@ -990,18 +1041,23 @@ export const createChangeReviewSlice: StateCreator 0) { set((s) => ({ taskHasChanges: { ...s.taskHasChanges, [cacheKey]: true }, })); taskChangesNegativeCache.delete(cacheKey); } else { + set((s) => ({ + taskHasChanges: { ...s.taskHasChanges, [cacheKey]: false }, + })); taskChangesNegativeCache.set(cacheKey, Date.now()); } } catch { - // Don't cache errors in store — allow retry when session data appears later - taskChangesNegativeCache.set(cacheKey, Date.now()); + // Allow immediate retry after transient failures (race, file lock, late logs). } finally { taskChangesCheckInFlight.delete(cacheKey); } diff --git a/src/renderer/store/slices/teamSlice.ts b/src/renderer/store/slices/teamSlice.ts index cb828c79..f38c5f7a 100644 --- a/src/renderer/store/slices/teamSlice.ts +++ b/src/renderer/store/slices/teamSlice.ts @@ -1,4 +1,5 @@ import { api } from '@renderer/api'; +import type { TaskChangeRequestOptions } from '@renderer/utils/taskChangeRequest'; import { normalizePath } from '@renderer/utils/pathNormalize'; import { IpcError, unwrapIpc } from '@renderer/utils/unwrapIpc'; import { createLogger } from '@shared/utils/logger'; @@ -277,8 +278,14 @@ export interface TeamSlice { openMemberProfile: (memberName: string) => void; closeMemberProfile: () => void; /** Set by GlobalTaskDetailDialog to signal TeamDetailView to open ChangeReviewDialog */ - pendingReviewRequest: { taskId: string; filePath?: string } | null; - setPendingReviewRequest: (req: { taskId: string; filePath?: string } | null) => void; + pendingReviewRequest: { + taskId: string; + filePath?: string; + requestOptions: TaskChangeRequestOptions; + } | null; + setPendingReviewRequest: ( + req: { taskId: string; filePath?: string; requestOptions: TaskChangeRequestOptions } | null + ) => void; selectedTeamName: string | null; selectedTeamData: TeamData | null; selectedTeamLoading: boolean; diff --git a/src/renderer/utils/taskChangeRequest.ts b/src/renderer/utils/taskChangeRequest.ts new file mode 100644 index 00000000..e41660b2 --- /dev/null +++ b/src/renderer/utils/taskChangeRequest.ts @@ -0,0 +1,97 @@ +import type { ReviewAPI } from '@shared/types/api'; +import type { TeamTaskWithKanban } from '@shared/types/team'; + +const TASK_SINCE_GRACE_MS = 2 * 60 * 1000; + +export type TaskChangeRequestOptions = NonNullable[2]>; + +export interface TaskChangeContext { + taskId: string; + requestOptions: TaskChangeRequestOptions; + initialFilePath?: string; +} + +type TaskChangeTaskLike = Pick< + TeamTaskWithKanban, + 'id' | 'owner' | 'status' | 'createdAt' | 'updatedAt' | 'workIntervals' | 'historyEvents' +>; + +export function deriveTaskSince(task: TaskChangeTaskLike | null): string | undefined { + if (!task) return undefined; + + const sources: string[] = []; + if (task.createdAt) sources.push(task.createdAt); + if (Array.isArray(task.workIntervals)) { + for (const interval of task.workIntervals) { + if (interval.startedAt) sources.push(interval.startedAt); + } + } + if (Array.isArray(task.historyEvents)) { + for (const event of task.historyEvents) { + if (event.timestamp) sources.push(event.timestamp); + } + } + if (sources.length === 0) return undefined; + + const earliest = sources.reduce((a, b) => (a < b ? a : b)); + const date = new Date(earliest); + date.setTime(date.getTime() - TASK_SINCE_GRACE_MS); + return date.toISOString(); +} + +export function buildTaskChangeRequestOptions( + task: TaskChangeTaskLike, + overrides?: Partial +): TaskChangeRequestOptions { + const options: TaskChangeRequestOptions = { + owner: task.owner, + status: task.status, + intervals: task.workIntervals, + since: deriveTaskSince(task), + }; + + return { + ...options, + ...overrides, + }; +} + +export function buildTaskChangeContext( + task: TaskChangeTaskLike, + input?: { initialFilePath?: string; summaryOnly?: boolean } +): TaskChangeContext { + return { + taskId: task.id, + requestOptions: buildTaskChangeRequestOptions(task, { + summaryOnly: input?.summaryOnly, + }), + initialFilePath: input?.initialFilePath, + }; +} + +export function buildTaskChangeSignature(options: TaskChangeRequestOptions): string { + const owner = typeof options.owner === 'string' ? options.owner.trim() : ''; + const status = typeof options.status === 'string' ? options.status.trim() : ''; + const since = typeof options.since === 'string' ? options.since : ''; + const intervals = Array.isArray(options.intervals) + ? options.intervals.map((interval) => ({ + startedAt: interval.startedAt, + completedAt: interval.completedAt ?? '', + })) + : []; + + return JSON.stringify({ + owner, + status, + since, + intervals, + }); +} + +export function buildTaskChangePresenceKey( + teamName: string, + taskId: string, + options: TaskChangeRequestOptions +): string { + return `${teamName}:${taskId}:${buildTaskChangeSignature(options)}`; +} diff --git a/src/shared/types/extensions/plugin.ts b/src/shared/types/extensions/plugin.ts index 9aa89dbc..698f800b 100644 --- a/src/shared/types/extensions/plugin.ts +++ b/src/shared/types/extensions/plugin.ts @@ -14,6 +14,7 @@ export interface PluginCatalogItem { name: string; // display name only // Metadata + source: 'official'; description: string; category: string; // open-ended string, derived from marketplace.json author?: { name: string; email?: string }; diff --git a/test/main/services/team/ChangeExtractorService.test.ts b/test/main/services/team/ChangeExtractorService.test.ts new file mode 100644 index 00000000..4162fd81 --- /dev/null +++ b/test/main/services/team/ChangeExtractorService.test.ts @@ -0,0 +1,148 @@ +import * as os from 'os'; +import * as path from 'path'; +import { afterEach, describe, expect, it, vi } from 'vitest'; + +import * as fs from 'fs/promises'; + +import { ChangeExtractorService } from '../../../../src/main/services/team/ChangeExtractorService'; +import { setClaudeBasePathOverride } from '../../../../src/main/utils/pathDecoder'; + +describe('ChangeExtractorService', () => { + let tmpDir: string | null = null; + + afterEach(async () => { + setClaudeBasePathOverride(null); + if (tmpDir) { + await fs.rm(tmpDir, { recursive: true, force: true }); + tmpDir = null; + } + }); + + it('does not reuse detailed task-change cache across different scope inputs', async () => { + tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), 'change-extractor-service-')); + setClaudeBasePathOverride(tmpDir); + + const aliceLogPath = path.join(tmpDir, 'alice.jsonl'); + await fs.writeFile( + aliceLogPath, + JSON.stringify({ + timestamp: '2026-03-01T10:00:00.000Z', + type: 'assistant', + message: { + role: 'assistant', + content: [ + { + type: 'tool_use', + id: 'tool-1', + name: 'Write', + input: { file_path: '/repo/src/file.ts', content: 'export const value = 1;\n' }, + }, + ], + }, + }) + '\n', + 'utf8' + ); + + const findLogsForTask = vi.fn(async (_teamName: string, _taskId: string, options?: any) => + options?.owner === 'alice' ? [{ filePath: aliceLogPath, memberName: 'alice' }] : [] + ); + const parseBoundaries = vi.fn(async () => ({ + boundaries: [], + scopes: [], + isSingleTaskSession: true, + detectedMechanism: 'none' as const, + })); + const service = new ChangeExtractorService( + { + findLogsForTask, + findMemberLogPaths: vi.fn(async () => []), + } as any, + { parseBoundaries } as any, + { getConfig: vi.fn(async () => ({ projectPath: '/repo' })) } as any + ); + + const empty = await service.getTaskChanges('team-a', '1', { owner: 'bob', status: 'completed' }); + const populated = await service.getTaskChanges('team-a', '1', { + owner: 'alice', + status: 'completed', + }); + + expect(empty.files).toHaveLength(0); + expect(populated.files).toHaveLength(1); + expect(findLogsForTask).toHaveBeenCalledTimes(2); + }); + + it('merges fallback changes for the same Windows file across slash variants', async () => { + tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), 'change-extractor-service-')); + setClaudeBasePathOverride(tmpDir); + + const firstLogPath = path.join(tmpDir, 'first.jsonl'); + const secondLogPath = path.join(tmpDir, 'second.jsonl'); + await fs.writeFile( + firstLogPath, + JSON.stringify({ + timestamp: '2026-03-01T10:00:00.000Z', + type: 'assistant', + message: { + role: 'assistant', + content: [ + { + type: 'tool_use', + id: 'tool-1', + name: 'Write', + input: { file_path: 'C:\\repo\\src\\same.ts', content: 'first\n' }, + }, + ], + }, + }) + '\n', + 'utf8' + ); + await fs.writeFile( + secondLogPath, + JSON.stringify({ + timestamp: '2026-03-01T10:01:00.000Z', + type: 'assistant', + message: { + role: 'assistant', + content: [ + { + type: 'tool_use', + id: 'tool-2', + name: 'Write', + input: { file_path: 'C:/repo/src/same.ts', content: 'second\n' }, + }, + ], + }, + }) + '\n', + 'utf8' + ); + + const service = new ChangeExtractorService( + { + findLogsForTask: vi.fn(async () => [ + { filePath: firstLogPath, memberName: 'alice' }, + { filePath: secondLogPath, memberName: 'alice' }, + ]), + findMemberLogPaths: vi.fn(async () => []), + } as any, + { + parseBoundaries: vi.fn(async () => ({ + boundaries: [], + scopes: [], + isSingleTaskSession: true, + detectedMechanism: 'none' as const, + })), + } as any, + { getConfig: vi.fn(async () => ({ projectPath: 'C:\\repo' })) } as any + ); + + const result = await service.getTaskChanges('team-a', '1', { + owner: 'alice', + status: 'completed', + }); + + expect(result.files).toHaveLength(1); + expect(result.files[0]?.relativePath).toBe('src/same.ts'); + expect(result.totalLinesAdded).toBe(2); + }); +}); diff --git a/test/main/services/team/TaskBoundaryParser.test.ts b/test/main/services/team/TaskBoundaryParser.test.ts index 22cad834..e1f592bb 100644 --- a/test/main/services/team/TaskBoundaryParser.test.ts +++ b/test/main/services/team/TaskBoundaryParser.test.ts @@ -110,4 +110,51 @@ describe('TaskBoundaryParser', () => { expect(result.boundaries).toHaveLength(1); expect(result.boundaries[0]?.mechanism).toBe('mcp'); }); + + it('accepts task_id for TaskUpdate and MCP task markers', async () => { + tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), 'task-boundary-parser-')); + const jsonlPath = path.join(tmpDir, 'task-id-underscore.jsonl'); + await fs.writeFile( + jsonlPath, + [ + JSON.stringify({ + timestamp: '2026-03-01T10:00:00.000Z', + type: 'assistant', + message: { + role: 'assistant', + content: [ + { + type: 'tool_use', + id: 'tool-1', + name: 'TaskUpdate', + input: { task_id: 'task-123', status: 'in_progress' }, + }, + ], + }, + }), + JSON.stringify({ + timestamp: '2026-03-01T10:10:00.000Z', + type: 'assistant', + message: { + role: 'assistant', + content: [ + { + type: 'tool_use', + id: 'tool-2', + name: 'task_complete', + input: { task_id: 'task-123' }, + }, + ], + }, + }), + ].join('\n') + '\n', + 'utf8' + ); + + const result = await new TaskBoundaryParser().parseBoundaries(jsonlPath); + + expect(result.boundaries).toHaveLength(2); + expect(result.boundaries.map((entry) => entry.taskId)).toEqual(['task-123', 'task-123']); + expect(result.boundaries.map((entry) => entry.event)).toEqual(['start', 'complete']); + }); }); diff --git a/test/main/services/team/TeamMcpConfigBuilder.test.ts b/test/main/services/team/TeamMcpConfigBuilder.test.ts index 454462be..9dd3cda1 100644 --- a/test/main/services/team/TeamMcpConfigBuilder.test.ts +++ b/test/main/services/team/TeamMcpConfigBuilder.test.ts @@ -1,10 +1,23 @@ -import { afterEach, describe, expect, it } from 'vitest'; +import { afterEach, describe, expect, it, vi } from 'vitest'; import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; + +let mockHomeDir = ''; + +vi.mock('@main/utils/pathDecoder', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + getHomeDir: () => mockHomeDir || actual.getHomeDir(), + }; +}); import { TeamMcpConfigBuilder } from '@main/services/team/TeamMcpConfigBuilder'; describe('TeamMcpConfigBuilder', () => { const createdPaths: string[] = []; + const createdDirs: string[] = []; afterEach(() => { for (const filePath of createdPaths.splice(0)) { @@ -14,6 +27,14 @@ describe('TeamMcpConfigBuilder', () => { // ignore cleanup issues in temp dir } } + for (const dirPath of createdDirs.splice(0)) { + try { + fs.rmSync(dirPath, { recursive: true, force: true }); + } catch { + // ignore cleanup issues in temp dir + } + } + mockHomeDir = ''; }); it('prefers the source MCP entry when workspace source is available', async () => { @@ -37,4 +58,156 @@ describe('TeamMcpConfigBuilder', () => { `${process.cwd()}/mcp-server/src/index.ts`, ]); }); + + it('merges top-level user MCP with generated agent-teams config', async () => { + const homeDir = fs.mkdtempSync(path.join(os.tmpdir(), 'team-mcp-home-')); + const projectDir = fs.mkdtempSync(path.join(os.tmpdir(), 'team-mcp-project-')); + createdDirs.push(homeDir, projectDir); + mockHomeDir = homeDir; + + fs.writeFileSync( + path.join(homeDir, '.claude.json'), + JSON.stringify( + { + mcpServers: { + globalOnly: { type: 'http', url: 'https://global.example.com/mcp' }, + duplicateServer: { type: 'http', url: 'https://global.example.com/duplicate' }, + }, + }, + null, + 2 + ) + ); + + fs.writeFileSync( + path.join(projectDir, '.mcp.json'), + JSON.stringify( + { + mcpServers: { + projectOnly: { command: 'node', args: ['project-server.js'] }, + duplicateServer: { command: 'node', args: ['project-override.js'] }, + }, + }, + null, + 2 + ) + ); + + const builder = new TeamMcpConfigBuilder(); + const configPath = await builder.writeConfigFile(projectDir); + createdPaths.push(configPath); + + const parsed = JSON.parse(fs.readFileSync(configPath, 'utf8')) as { + mcpServers: Record; + }; + + expect(Object.keys(parsed.mcpServers).sort()).toEqual([ + 'agent-teams', + 'duplicateServer', + 'globalOnly', + ]); + expect(parsed.mcpServers.globalOnly).toMatchObject({ + type: 'http', + url: 'https://global.example.com/mcp', + }); + expect(parsed.mcpServers.duplicateServer).toMatchObject({ + type: 'http', + url: 'https://global.example.com/duplicate', + }); + }); + + it('does not inline project MCP config to preserve native Claude precedence', async () => { + const homeDir = fs.mkdtempSync(path.join(os.tmpdir(), 'team-mcp-home-')); + const projectDir = fs.mkdtempSync(path.join(os.tmpdir(), 'team-mcp-project-')); + createdDirs.push(homeDir, projectDir); + mockHomeDir = homeDir; + + fs.writeFileSync(path.join(homeDir, '.claude.json'), JSON.stringify({ mcpServers: {} }, null, 2)); + fs.writeFileSync( + path.join(projectDir, '.mcp.json'), + JSON.stringify( + { + mcpServers: { + projectOnly: { command: 'node', args: ['project-server.js'] }, + }, + }, + null, + 2 + ) + ); + + const builder = new TeamMcpConfigBuilder(); + const configPath = await builder.writeConfigFile(projectDir); + createdPaths.push(configPath); + + const parsed = JSON.parse(fs.readFileSync(configPath, 'utf8')) as { + mcpServers: Record; + }; + + expect(parsed.mcpServers.projectOnly).toBeUndefined(); + expect(Object.keys(parsed.mcpServers)).toEqual(['agent-teams']); + }); + + it('generated agent-teams server overrides same-named user MCP entry', async () => { + const homeDir = fs.mkdtempSync(path.join(os.tmpdir(), 'team-mcp-home-')); + createdDirs.push(homeDir); + mockHomeDir = homeDir; + + fs.writeFileSync( + path.join(homeDir, '.claude.json'), + JSON.stringify( + { + mcpServers: { + 'agent-teams': { command: 'node', args: ['user-server.js'] }, + }, + }, + null, + 2 + ) + ); + + const builder = new TeamMcpConfigBuilder(); + const configPath = await builder.writeConfigFile(); + createdPaths.push(configPath); + + const parsed = JSON.parse(fs.readFileSync(configPath, 'utf8')) as { + mcpServers: Record; + }; + + expect(parsed.mcpServers['agent-teams']).toMatchObject({ + command: 'pnpm', + args: [ + '--dir', + `${process.cwd()}/mcp-server`, + 'exec', + 'tsx', + `${process.cwd()}/mcp-server/src/index.ts`, + ], + }); + }); + + it('ignores malformed user MCP file', async () => { + const homeDir = fs.mkdtempSync(path.join(os.tmpdir(), 'team-mcp-home-')); + const projectDir = fs.mkdtempSync(path.join(os.tmpdir(), 'team-mcp-project-')); + createdDirs.push(homeDir, projectDir); + mockHomeDir = homeDir; + + fs.writeFileSync(path.join(homeDir, '.claude.json'), '{ invalid json'); + + const builder = new TeamMcpConfigBuilder(); + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + let configPath = ''; + try { + configPath = await builder.writeConfigFile(projectDir); + } finally { + warnSpy.mockRestore(); + } + createdPaths.push(configPath); + + const parsed = JSON.parse(fs.readFileSync(configPath, 'utf8')) as { + mcpServers: Record; + }; + + expect(Object.keys(parsed.mcpServers)).toEqual(['agent-teams']); + }); }); diff --git a/test/renderer/store/changeReviewSlice.test.ts b/test/renderer/store/changeReviewSlice.test.ts new file mode 100644 index 00000000..0df5a8ae --- /dev/null +++ b/test/renderer/store/changeReviewSlice.test.ts @@ -0,0 +1,173 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { create } from 'zustand'; + +import { createChangeReviewSlice } from '../../../src/renderer/store/slices/changeReviewSlice'; + +const hoisted = vi.hoisted(() => ({ + getTaskChanges: vi.fn(), + getAgentChanges: vi.fn(), + getChangeStats: vi.fn(), + getFileContent: vi.fn(), + applyDecisions: vi.fn(), + saveEditedFile: vi.fn(), + checkConflict: vi.fn(), + rejectHunks: vi.fn(), + rejectFile: vi.fn(), + previewReject: vi.fn(), +})); + +vi.mock('@renderer/api', () => ({ + api: { + review: { + getTaskChanges: hoisted.getTaskChanges, + getAgentChanges: hoisted.getAgentChanges, + getChangeStats: hoisted.getChangeStats, + getFileContent: hoisted.getFileContent, + applyDecisions: hoisted.applyDecisions, + saveEditedFile: hoisted.saveEditedFile, + checkConflict: hoisted.checkConflict, + rejectHunks: hoisted.rejectHunks, + rejectFile: hoisted.rejectFile, + previewReject: hoisted.previewReject, + }, + }, +})); + +function createSliceStore() { + return create()((set, get, store) => ({ + ...createChangeReviewSlice(set as never, get as never, store as never), + })); +} + +function deferred() { + let resolve!: (value: T) => void; + let reject!: (error?: unknown) => void; + const promise = new Promise((res, rej) => { + resolve = res; + reject = rej; + }); + return { promise, resolve, reject }; +} + +const OPTIONS_A = { + owner: 'alice', + status: 'completed', + intervals: [{ startedAt: '2026-03-01T10:00:00.000Z' }], + since: '2026-03-01T09:58:00.000Z', +}; + +const OPTIONS_B = { + owner: 'bob', + status: 'completed', + intervals: [{ startedAt: '2026-03-01T11:00:00.000Z' }], + since: '2026-03-01T10:58:00.000Z', +}; + +describe('changeReviewSlice task changes', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('does not cache errors as negative task-change results', async () => { + const store = createSliceStore(); + hoisted.getTaskChanges.mockRejectedValue(new Error('transient')); + + await store.getState().checkTaskHasChanges('team-a', '1', OPTIONS_A); + await store.getState().checkTaskHasChanges('team-a', '1', OPTIONS_A); + + expect(hoisted.getTaskChanges).toHaveBeenCalledTimes(2); + }); + + it('negative-caches confirmed empty results per request signature', async () => { + const store = createSliceStore(); + hoisted.getTaskChanges.mockResolvedValue({ + files: [], + totalFiles: 0, + totalLinesAdded: 0, + totalLinesRemoved: 0, + teamName: 'team-a', + taskId: '1', + confidence: 'fallback', + computedAt: '2026-03-01T12:00:00.000Z', + scope: { + taskId: '1', + memberName: '', + startLine: 0, + endLine: 0, + startTimestamp: '', + endTimestamp: '', + toolUseIds: [], + filePaths: [], + confidence: { tier: 4, label: 'fallback', reason: 'No log files found for task' }, + }, + warnings: [], + }); + + await store.getState().checkTaskHasChanges('team-a', '1', OPTIONS_A); + await store.getState().checkTaskHasChanges('team-a', '1', OPTIONS_A); + await store.getState().checkTaskHasChanges('team-a', '1', OPTIONS_B); + + expect(hoisted.getTaskChanges).toHaveBeenCalledTimes(2); + }); + + it('ignores stale fetchTaskChanges responses when a newer task request wins', async () => { + const store = createSliceStore(); + const first = deferred(); + const second = deferred(); + hoisted.getTaskChanges.mockReturnValueOnce(first.promise).mockReturnValueOnce(second.promise); + + const firstFetch = store.getState().fetchTaskChanges('team-a', '1', OPTIONS_A); + const secondFetch = store.getState().fetchTaskChanges('team-a', '2', OPTIONS_B); + + second.resolve({ + teamName: 'team-a', + taskId: '2', + files: [{ filePath: '/repo/new.ts', relativePath: 'new.ts', snippets: [], linesAdded: 1, linesRemoved: 0, isNewFile: true }], + totalFiles: 1, + totalLinesAdded: 1, + totalLinesRemoved: 0, + confidence: 'fallback', + computedAt: '2026-03-01T12:00:00.000Z', + scope: { + taskId: '2', + memberName: 'bob', + startLine: 0, + endLine: 0, + startTimestamp: '', + endTimestamp: '', + toolUseIds: [], + filePaths: ['/repo/new.ts'], + confidence: { tier: 4, label: 'fallback', reason: 'No task boundaries found in JSONL' }, + }, + warnings: [], + }); + await secondFetch; + + first.resolve({ + teamName: 'team-a', + taskId: '1', + files: [{ filePath: '/repo/old.ts', relativePath: 'old.ts', snippets: [], linesAdded: 1, linesRemoved: 0, isNewFile: true }], + totalFiles: 1, + totalLinesAdded: 1, + totalLinesRemoved: 0, + confidence: 'fallback', + computedAt: '2026-03-01T12:00:00.000Z', + scope: { + taskId: '1', + memberName: 'alice', + startLine: 0, + endLine: 0, + startTimestamp: '', + endTimestamp: '', + toolUseIds: [], + filePaths: ['/repo/old.ts'], + confidence: { tier: 4, label: 'fallback', reason: 'No task boundaries found in JSONL' }, + }, + warnings: [], + }); + await firstFetch; + + expect(store.getState().activeChangeSet?.taskId).toBe('2'); + expect(store.getState().selectedReviewFilePath).toBe('/repo/new.ts'); + }); +}); diff --git a/test/renderer/store/extensionsSlice.test.ts b/test/renderer/store/extensionsSlice.test.ts index 80b175aa..0a2a696a 100644 --- a/test/renderer/store/extensionsSlice.test.ts +++ b/test/renderer/store/extensionsSlice.test.ts @@ -36,6 +36,7 @@ const makePlugin = (overrides: Partial): EnrichedPlugin => ({ marketplaceId: 'test@marketplace', qualifiedName: 'test@marketplace', name: 'Test Plugin', + source: 'official', description: 'A test plugin', category: 'testing', hasLspServers: false, diff --git a/test/renderer/utils/taskChangeRequest.test.ts b/test/renderer/utils/taskChangeRequest.test.ts new file mode 100644 index 00000000..041ac310 --- /dev/null +++ b/test/renderer/utils/taskChangeRequest.test.ts @@ -0,0 +1,67 @@ +import { describe, expect, it } from 'vitest'; + +import { + buildTaskChangePresenceKey, + buildTaskChangeRequestOptions, + deriveTaskSince, +} from '@renderer/utils/taskChangeRequest'; + +describe('taskChangeRequest', () => { + it('derives since from the earliest known task timestamp with grace window', () => { + const since = deriveTaskSince({ + id: 't1', + owner: 'alice', + status: 'completed', + createdAt: '2026-03-01T10:05:00.000Z', + updatedAt: '2026-03-01T12:00:00.000Z', + workIntervals: [{ startedAt: '2026-03-01T10:10:00.000Z' }], + historyEvents: [ + { + id: 'evt-1', + type: 'status_changed', + from: 'pending', + to: 'in_progress', + timestamp: '2026-03-01T10:00:00.000Z', + }, + ], + }); + + expect(since).toBe('2026-03-01T09:58:00.000Z'); + }); + + it('builds canonical task change request options', () => { + const options = buildTaskChangeRequestOptions( + { + id: 't1', + owner: 'alice', + status: 'completed', + createdAt: '2026-03-01T10:05:00.000Z', + updatedAt: '2026-03-01T12:00:00.000Z', + workIntervals: [{ startedAt: '2026-03-01T10:10:00.000Z' }], + historyEvents: [], + }, + { summaryOnly: true } + ); + + expect(options).toEqual({ + owner: 'alice', + status: 'completed', + intervals: [{ startedAt: '2026-03-01T10:10:00.000Z' }], + since: '2026-03-01T10:03:00.000Z', + summaryOnly: true, + }); + }); + + it('uses scope inputs for presence keys', () => { + const base = { + owner: 'alice', + status: 'completed', + intervals: [{ startedAt: '2026-03-01T10:10:00.000Z' }], + since: '2026-03-01T10:03:00.000Z', + }; + + expect(buildTaskChangePresenceKey('team-a', '1', base)).not.toBe( + buildTaskChangePresenceKey('team-a', '1', { ...base, owner: 'bob' }) + ); + }); +}); diff --git a/test/shared/utils/extensionNormalizers.test.ts b/test/shared/utils/extensionNormalizers.test.ts index a81eb3c4..ff28a938 100644 --- a/test/shared/utils/extensionNormalizers.test.ts +++ b/test/shared/utils/extensionNormalizers.test.ts @@ -39,6 +39,7 @@ describe('inferCapabilities', () => { marketplaceId: 'test@marketplace', qualifiedName: 'test@marketplace', name: 'test', + source: 'official', description: 'test', category: 'development', hasLspServers: false,