From 704b9cbfe53df195251e4d559c97cd6e786c3fce Mon Sep 17 00:00:00 2001 From: iliya Date: Sun, 22 Feb 2026 18:41:08 +0200 Subject: [PATCH] feat: enhance team configuration and member detection - Added validation for team configuration updates to ensure name, description, and color are strings. - Improved member detection logic to avoid false positives by ensuring only one known member name is matched. - Refactored post-launch configuration updates to combine session history and project path updates, preventing race conditions. - Updated UI components to streamline state management for collapsible sections. These changes aim to improve data integrity and user experience in team management functionalities. --- src/main/ipc/teams.ts | 9 +++ .../services/team/ClaudeBinaryResolver.ts | 3 +- .../services/team/TeamMemberLogsFinder.ts | 50 ++++-------- .../services/team/TeamProvisioningService.ts | 78 +++++++------------ .../team/CollapsibleTeamSection.tsx | 9 +-- .../team/dialogs/SendMessageDialog.tsx | 21 +++-- test/main/ipc/teams.test.ts | 7 +- 7 files changed, 77 insertions(+), 100 deletions(-) diff --git a/src/main/ipc/teams.ts b/src/main/ipc/teams.ts index c3fa25d3..574a46c0 100644 --- a/src/main/ipc/teams.ts +++ b/src/main/ipc/teams.ts @@ -202,6 +202,15 @@ async function handleUpdateConfig( return { success: false, error: 'Invalid updates object' }; } const { name, description, color } = updates as TeamUpdateConfigRequest; + if (name !== undefined && typeof name !== 'string') { + return { success: false, error: 'name must be a string' }; + } + if (description !== undefined && typeof description !== 'string') { + return { success: false, error: 'description must be a string' }; + } + if (color !== undefined && typeof color !== 'string') { + return { success: false, error: 'color must be a string' }; + } return wrapTeamHandler('updateConfig', async () => { const result = await getTeamDataService().updateConfig(validated.value!, { name, diff --git a/src/main/services/team/ClaudeBinaryResolver.ts b/src/main/services/team/ClaudeBinaryResolver.ts index 46921bf4..e05b766e 100644 --- a/src/main/services/team/ClaudeBinaryResolver.ts +++ b/src/main/services/team/ClaudeBinaryResolver.ts @@ -55,7 +55,8 @@ export class ClaudeBinaryResolver { const platformBinaryName = process.platform === 'win32' ? 'claude.cmd' : 'claude'; const fromPath = await resolveFromPathEnv(platformBinaryName); if (fromPath) { - return fromPath; + cachedPath = fromPath; + return cachedPath; } const candidates: string[] = [ diff --git a/src/main/services/team/TeamMemberLogsFinder.ts b/src/main/services/team/TeamMemberLogsFinder.ts index 144e4a57..a6a9bd11 100644 --- a/src/main/services/team/TeamMemberLogsFinder.ts +++ b/src/main/services/team/TeamMemberLogsFinder.ts @@ -427,52 +427,30 @@ export class TeamMemberLogsFinder { } /** - * Detects the member name from a parsed JSONL message using multiple signals. - * Returns a detection result with the name and a priority level: - * 3 = routing sender (highest, handled outside this method) - * 2 = "You are {name}" spawn prompt - * 1 = text-based fallback (single member match or task assignment context) + * Last-resort member detection from message text. + * Only called when all structured signals (teammate_id, process.team, routing) failed. + * Returns priority 1 (lowest) — only if exactly one known member name appears. */ private detectMemberFromMessage( msg: Record, knownMembers: Set ): { name: string; priority: number } | null { + if (this.extractRole(msg) !== 'user') return null; + const text = this.extractTextContent(msg); if (!text) return null; - // Signal 1 (priority 2): "You are {name}, a {role}" pattern (spawn prompt) - const youAreMatch = /\bYou are (\w[\w-]*),\s+a\s+/i.exec(text); - if (youAreMatch) { - const name = youAreMatch[1].toLowerCase(); - if (knownMembers.has(name)) { - return { name: youAreMatch[1], priority: 2 }; + // Only attribute if exactly one known member name appears (word-boundary match). + // Avoids false positives when multiple members are mentioned. + const matches: string[] = []; + for (const name of knownMembers) { + const regex = new RegExp(`\\b${escapeRegex(name)}\\b`, 'i'); + if (regex.test(text)) { + matches.push(name); } } - - // Signal 2 (priority 1): Task assignment — look for member name in the task content - if (text.includes('New task assigned to you') || text.includes('task assigned')) { - for (const name of knownMembers) { - const regex = new RegExp(`\\b${escapeRegex(name)}\\b`, 'i'); - if (regex.test(text)) { - return { name: findOriginalCase(text, name), priority: 1 }; - } - } - } - - // Signal 3 (priority 1): General fallback — check if exactly one known member - // name appears in the first user message content (word-boundary match) - if (msg.role === 'user') { - const matches: string[] = []; - for (const name of knownMembers) { - const regex = new RegExp(`\\b${escapeRegex(name)}\\b`, 'i'); - if (regex.test(text)) { - matches.push(name); - } - } - // Only attribute if exactly one member matches (avoid ambiguity) - if (matches.length === 1) { - return { name: findOriginalCase(text, matches[0]), priority: 1 }; - } + if (matches.length === 1) { + return { name: findOriginalCase(text, matches[0]), priority: 1 }; } return null; diff --git a/src/main/services/team/TeamProvisioningService.ts b/src/main/services/team/TeamProvisioningService.ts index ff403aef..011d4002 100644 --- a/src/main/services/team/TeamProvisioningService.ts +++ b/src/main/services/team/TeamProvisioningService.ts @@ -1091,8 +1091,7 @@ export class TeamProvisioningService { this.stopFilesystemMonitor(run); if (run.isLaunch) { - await this.ensureProjectPathInConfig(run.teamName, run.request.cwd); - await this.appendSessionToHistory(run.teamName); + await this.updateConfigPostLaunch(run.teamName, run.request.cwd); const readyMessage = 'Team launched — process alive and ready'; const progress = updateProgress(run, 'ready', readyMessage, { cliLogsTail: extractLogsTail(run.stdoutBuffer, run.stderrBuffer), @@ -1129,8 +1128,7 @@ export class TeamProvisioningService { // Persist teammates metadata separately from config.json. await this.persistMembersMeta(run.teamName, run.request); - await this.ensureProjectPathInConfig(run.teamName, run.request.cwd); - await this.appendSessionToHistory(run.teamName); + await this.updateConfigPostLaunch(run.teamName, run.request.cwd); const progress = updateProgress(run, 'ready', 'Team provisioned — process alive and ready', { cliLogsTail: extractLogsTail(run.stdoutBuffer, run.stderrBuffer), @@ -1669,60 +1667,44 @@ export class TeamProvisioningService { } /** - * Append current leadSessionId to sessionHistory array in config.json. - * Called after launch/create to track which sessions belong to this team. + * Single atomic read-mutate-write for post-launch config updates. + * Combines session history append and projectPath update to avoid + * race conditions with the CLI writing to the same file. */ - private async appendSessionToHistory(teamName: string): Promise { + private async updateConfigPostLaunch(teamName: string, projectPath: string): Promise { const configPath = path.join(getTeamsBasePath(), teamName, 'config.json'); try { const raw = await fs.promises.readFile(configPath, 'utf8'); const config = JSON.parse(raw) as Record; + + // Append session to history const leadSessionId = config.leadSessionId; - if (typeof leadSessionId !== 'string' || leadSessionId.trim().length === 0) { - return; + if (typeof leadSessionId === 'string' && leadSessionId.trim().length > 0) { + const sessionHistory = Array.isArray(config.sessionHistory) + ? (config.sessionHistory as string[]) + : []; + if (!sessionHistory.includes(leadSessionId)) { + sessionHistory.push(leadSessionId); + config.sessionHistory = sessionHistory; + } } - const history = Array.isArray(config.sessionHistory) - ? (config.sessionHistory as string[]) - : []; - if (history.includes(leadSessionId)) { - return; + + // Ensure projectPath + if (projectPath.trim()) { + config.projectPath = projectPath; + const pathHistory = Array.isArray(config.projectPathHistory) + ? (config.projectPathHistory as string[]).filter( + (p) => typeof p === 'string' && p !== projectPath + ) + : []; + pathHistory.push(projectPath); + config.projectPathHistory = pathHistory; } - history.push(leadSessionId); - config.sessionHistory = history; - await fs.promises.writeFile(configPath, JSON.stringify(config, null, 2), 'utf8'); - logger.info(`[${teamName}] Appended session ${leadSessionId} to sessionHistory`); + + await atomicWriteAsync(configPath, JSON.stringify(config, null, 2)); } catch (error) { logger.warn( - `[${teamName}] Failed to append session to history: ${error instanceof Error ? error.message : String(error)}` - ); - } - } - - private async ensureProjectPathInConfig(teamName: string, projectPath: string): Promise { - if (!projectPath.trim()) { - return; - } - const configPath = path.join(getTeamsBasePath(), teamName, 'config.json'); - try { - const raw = await fs.promises.readFile(configPath, 'utf8'); - const config = JSON.parse(raw) as Record; - - // Always update projectPath to current cwd - config.projectPath = projectPath; - - // Maintain ordered history (no duplicates, most recent last) - const history = Array.isArray(config.projectPathHistory) - ? (config.projectPathHistory as string[]).filter( - (p) => typeof p === 'string' && p !== projectPath - ) - : []; - history.push(projectPath); - config.projectPathHistory = history; - - await fs.promises.writeFile(configPath, JSON.stringify(config, null, 2), 'utf8'); - } catch (error) { - logger.warn( - `[${teamName}] Failed to ensure projectPath in config.json: ${ + `[${teamName}] Failed to update config post-launch: ${ error instanceof Error ? error.message : String(error) }` ); diff --git a/src/renderer/components/team/CollapsibleTeamSection.tsx b/src/renderer/components/team/CollapsibleTeamSection.tsx index 4c1ba78c..2b1f1d2b 100644 --- a/src/renderer/components/team/CollapsibleTeamSection.tsx +++ b/src/renderer/components/team/CollapsibleTeamSection.tsx @@ -21,10 +21,7 @@ export const CollapsibleTeamSection = ({ children, }: CollapsibleTeamSectionProps): React.JSX.Element => { const [open, setOpen] = useState(defaultOpen); - - if (forceOpen && !open) { - setOpen(true); - } + const isOpen = forceOpen ? true : open; return (
@@ -36,7 +33,7 @@ export const CollapsibleTeamSection = ({ > {title} {badge != null && ( @@ -50,7 +47,7 @@ export const CollapsibleTeamSection = ({ {action &&
{action}
} - {open &&
{children}
} + {isOpen &&
{children}
}
); }; diff --git a/src/renderer/components/team/dialogs/SendMessageDialog.tsx b/src/renderer/components/team/dialogs/SendMessageDialog.tsx index a50ae36c..034ff50b 100644 --- a/src/renderer/components/team/dialogs/SendMessageDialog.tsx +++ b/src/renderer/components/team/dialogs/SendMessageDialog.tsx @@ -1,4 +1,4 @@ -import { useMemo, useState } from 'react'; +import { useEffect, useMemo, useState } from 'react'; import { Button } from '@renderer/components/ui/button'; import { @@ -65,15 +65,24 @@ export const SendMessageDialog = ({ setPrevOpen(open); } - // Auto-close on successful send (lastResult changed while dialog is open) + // Track whether auto-close is needed (setState in render phase is fine) + const [pendingAutoClose, setPendingAutoClose] = useState(false); if (open && lastResult && lastResult !== prevResult) { - setMember(''); - textDraft.clearDraft(); - setSummary(''); setPrevResult(lastResult); - onClose(); + setPendingAutoClose(true); } + // Side effects (onClose mutates parent state) must run in useEffect, not render phase + useEffect(() => { + if (pendingAutoClose) { + setMember(''); + textDraft.clearDraft(); + setSummary(''); + setPendingAutoClose(false); + onClose(); + } + }, [pendingAutoClose]); // eslint-disable-line react-hooks/exhaustive-deps + const mentionSuggestions = useMemo( () => members.map((m) => ({ diff --git a/test/main/ipc/teams.test.ts b/test/main/ipc/teams.test.ts index 4d5b412d..5a263fd7 100644 --- a/test/main/ipc/teams.test.ts +++ b/test/main/ipc/teams.test.ts @@ -1,3 +1,4 @@ +import * as os from 'os'; import { beforeEach, describe, expect, it, vi } from 'vitest'; vi.mock('@preload/constants/ipcChannels', () => ({ @@ -148,7 +149,7 @@ describe('ipc teams handlers', () => { const createResult = (await handlers.get(TEAM_CREATE)!({ sender: { send: vi.fn() } } as never, { teamName: 'my-team', members: [{ name: 'alice' }], - cwd: '/', + cwd: os.tmpdir(), })) as { success: boolean }; expect(createResult.success).toBe(true); expect(provisioningService.createTeam).toHaveBeenCalledTimes(1); @@ -247,7 +248,7 @@ describe('ipc teams handlers', () => { const result = (await handler({ sender: { send: vi.fn() } } as never, { teamName: 'test-team', members: [{ name: 'alice' }], - cwd: '/', + cwd: os.tmpdir(), prompt: 'Build a web app', })) as { success: boolean }; expect(result.success).toBe(true); @@ -260,7 +261,7 @@ describe('ipc teams handlers', () => { const result = (await handler({ sender: { send: vi.fn() } } as never, { teamName: 'test-team', members: [{ name: 'alice' }], - cwd: '/', + cwd: os.tmpdir(), prompt: 123, })) as { success: boolean; error: string }; expect(result.success).toBe(false);