From d398f3846b9773215f9172df47d702dc22e52757 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Titsworth-Morin?= Date: Thu, 25 Jun 2026 10:38:50 +0000 Subject: [PATCH 1/4] =?UTF-8?q?refactor:=20useEffect=20audit=20=E2=80=94?= =?UTF-8?q?=20HIGH=20priority=20wave=20(30=20items)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Shared hooks: - Add useMediaQuery (useSyncExternalStore) to packages/ui - Refactor useEscapeKey, useClickOutside with latest-ref pattern - Refactor usePrefersReducedMotion to useSyncExternalStore - Simplify useIsMobile, useIsStandalone, ThemeContext via useMediaQuery Forms/prop-sync (remove prop-to-state mirroring): - ProjectAgentCard, ProjectAgentsSection, ScalingSettings - ProfileFormDialog, TriggerForm Render derivation (replace effects with useMemo/derived state): - GlobalCommandPalette, RepoSelector, AccountMap, useMapFilters - useProjectChatState, useWorkspaceCore Network/query (add abort controllers, remove mountedRef): - useAgentChat, FilePreviewModal, useSessionLifecycle - ChatFilePanel, GitDiffView, useAdminAnalytics, useAdminErrors - useAdminHealth, useAdminLogQuery, useNodeLogs, useNodeSystemInfo - useWorkspacePorts, useBootLogStream, AppShell Lifecycle/timer (clean up intervals, fix race conditions): - ChatSession, OrphanedSessionsBanner, project-message-view - useTrialDraft, workspace/index, useProjectChatState Terminal/acp-client: - MultiTerminal, Terminal, useTerminalSessions, AgentPanel Co-Authored-By: Claude Opus 4.6 --- apps/web/src/components/AppShell.tsx | 12 +- apps/web/src/components/ChatSession.tsx | 12 +- apps/web/src/components/GitDiffView.tsx | 25 +- .../src/components/GlobalCommandPalette.tsx | 22 +- .../src/components/OrphanedSessionsBanner.tsx | 13 +- apps/web/src/components/ProjectAgentCard.tsx | 9 +- .../src/components/ProjectAgentsSection.tsx | 14 +- apps/web/src/components/RepoSelector.tsx | 15 +- apps/web/src/components/ScalingSettings.tsx | 73 ++- .../account-map/hooks/useMapFilters.ts | 14 +- .../agent-profiles/ProfileFormDialog.tsx | 608 +++++++++--------- .../web/src/components/chat/ChatFilePanel.tsx | 12 +- .../components/library/FilePreviewModal.tsx | 13 +- .../components/project-message-view/index.tsx | 34 +- .../useSessionLifecycle.ts | 21 +- apps/web/src/contexts/ThemeContext.tsx | 35 +- apps/web/src/hooks/useAdminAnalytics.ts | 29 +- apps/web/src/hooks/useAdminErrors.ts | 20 +- apps/web/src/hooks/useAdminHealth.ts | 29 +- apps/web/src/hooks/useAdminLogQuery.ts | 18 +- apps/web/src/hooks/useAgentChat.ts | 31 +- apps/web/src/hooks/useBootLogStream.ts | 11 +- apps/web/src/hooks/useIsMobile.ts | 19 +- apps/web/src/hooks/useIsStandalone.ts | 28 +- apps/web/src/hooks/useNodeLogs.ts | 35 +- apps/web/src/hooks/useNodeSystemInfo.ts | 23 +- apps/web/src/hooks/useTrialDraft.ts | 12 + apps/web/src/hooks/useWorkspacePorts.ts | 16 +- apps/web/src/pages/AccountMap.tsx | 10 +- .../pages/project-chat/useProjectChatState.ts | 42 +- apps/web/src/pages/workspace/index.tsx | 49 +- .../src/pages/workspace/useWorkspaceCore.ts | 48 +- .../acp-client/src/components/AgentPanel.tsx | 14 +- .../src/hooks/usePrefersReducedMotion.ts | 38 +- packages/terminal/src/MultiTerminal.tsx | 286 ++++---- packages/terminal/src/Terminal.tsx | 81 +-- .../terminal/src/hooks/useTerminalSessions.ts | 30 +- packages/ui/src/hooks/useClickOutside.ts | 9 +- packages/ui/src/hooks/useEscapeKey.ts | 9 +- packages/ui/src/hooks/useMediaQuery.ts | 17 + packages/ui/src/index.ts | 1 + 41 files changed, 927 insertions(+), 910 deletions(-) create mode 100644 packages/ui/src/hooks/useMediaQuery.ts diff --git a/apps/web/src/components/AppShell.tsx b/apps/web/src/components/AppShell.tsx index e8a5c79e6..c960b9c06 100644 --- a/apps/web/src/components/AppShell.tsx +++ b/apps/web/src/components/AppShell.tsx @@ -138,12 +138,8 @@ export function AppShell({ children }: AppShellProps) { setShowGlobalNav(false); }, [location.pathname]); - // Clear project name when leaving project context - useEffect(() => { - if (!projectId) { - setProjectNameState(undefined); - } - }, [projectId]); + // Derive display project name — clear when leaving project context + const displayProjectName = projectId ? projectName : undefined; const handleSignOut = async () => { try { @@ -214,7 +210,7 @@ export function AppShell({ children }: AppShellProps) { currentPath={location.pathname} onNavigate={(path) => { navigate(path); setDrawerOpen(false); }} onSignOut={handleSignOut} - projectName={projectId ? (projectName || 'Project') : undefined} + projectName={projectId ? (displayProjectName || 'Project') : undefined} infraSection={mobileInfraSection} projectListSection={mobileProjectListSection} showGlobalNav={showGlobalNav} @@ -257,7 +253,7 @@ export function AppShell({ children }: AppShellProps) { useImperativeHandle(ref, () => ({ focusInput: () => agentPanelRef.current?.focusInput(), })); - const wsUrlCacheRef = useRef<{ url: string; resolvedAt: number } | null>(null); + const wsUrlCacheRef = useRef<{ key: string; url: string; resolvedAt: number } | null>(null); // Resolve transcription API URL once (stable across renders) const transcribeApiUrl = useMemo(() => getTranscribeApiUrl(), []); @@ -114,15 +114,13 @@ export const ChatSession = React.forwardRef [workspaceId, sessionId] ); - useEffect(() => { - wsUrlCacheRef.current = null; - }, [wsHostInfo, workspaceId, sessionId, worktreePath]); - const resolveWsUrl = useCallback(async (): Promise => { if (!wsHostInfo) return null; + // Key the cache by inputs so stale session/worktree URLs cannot be reused + const cacheKey = `${wsHostInfo}|${workspaceId}|${sessionId}|${worktreePath ?? ''}`; const cached = wsUrlCacheRef.current; - if (cached && Date.now() - cached.resolvedAt < 15_000) { + if (cached && cached.key === cacheKey && Date.now() - cached.resolvedAt < 15_000) { return cached.url; } @@ -138,7 +136,7 @@ export const ChatSession = React.forwardRef const sessionQuery = `&sessionId=${encodeURIComponent(sessionId)}`; const worktreeQuery = worktreePath ? `&worktree=${encodeURIComponent(worktreePath)}` : ''; const url = `${wsHostInfo}/agent/ws?token=${encodeURIComponent(token)}${sessionQuery}${worktreeQuery}`; - wsUrlCacheRef.current = { url, resolvedAt: Date.now() }; + wsUrlCacheRef.current = { key: cacheKey, url, resolvedAt: Date.now() }; return url; } catch (err) { reportError({ diff --git a/apps/web/src/components/GitDiffView.tsx b/apps/web/src/components/GitDiffView.tsx index 885e1512e..75b9f53d9 100644 --- a/apps/web/src/components/GitDiffView.tsx +++ b/apps/web/src/components/GitDiffView.tsx @@ -80,8 +80,12 @@ export const GitDiffView: FC = ({ } }, [workspaceUrl, workspaceId, token, filePath, staged, worktree]); + // Track the identity of the cached full content so stale content is cleared + // when the file/worktree/staged context changes. + const [fullContentKey, setFullContentKey] = useState(''); + const currentFullKey = `${filePath}:${worktree ?? ''}:${staged}`; + const fetchFullFile = useCallback(async () => { - if (fullContent !== null) return; // already loaded setFullLoading(true); try { const result = await getGitFile( @@ -93,6 +97,7 @@ export const GitDiffView: FC = ({ worktree ?? undefined ); setFullContent(result.content); + setFullContentKey(currentFullKey); } catch { // Fallback: just show diff if full file fails setFullContent(null); @@ -100,18 +105,20 @@ export const GitDiffView: FC = ({ } finally { setFullLoading(false); } - }, [workspaceUrl, workspaceId, token, filePath, fullContent, worktree]); + }, [workspaceUrl, workspaceId, token, filePath, worktree, staged, currentFullKey]); + + const handleToggleFull = useCallback(() => { + setViewMode('full'); + // Fetch if we don't have content or it's for a different file/worktree/staged + if (fullContent === null || fullContentKey !== currentFullKey) { + fetchFullFile(); + } + }, [fullContent, fullContentKey, currentFullKey, fetchFullFile]); useEffect(() => { fetchDiff(); }, [fetchDiff]); - useEffect(() => { - if (viewMode === 'full') { - fetchFullFile(); - } - }, [viewMode, fetchFullFile]); - useEffect(() => { const handleKeyDown = (e: KeyboardEvent) => { if (e.key === 'Escape') onClose(); @@ -183,7 +190,7 @@ export const GitDiffView: FC = ({ setViewMode('full')} + onClick={handleToggleFull} /> diff --git a/apps/web/src/components/GlobalCommandPalette.tsx b/apps/web/src/components/GlobalCommandPalette.tsx index 300a079d8..d43f4729f 100644 --- a/apps/web/src/components/GlobalCommandPalette.tsx +++ b/apps/web/src/components/GlobalCommandPalette.tsx @@ -531,9 +531,12 @@ export function GlobalCommandPalette({ onClose }: GlobalCommandPaletteProps) { return flat; }, [groups]); + // Clamp selectedIndex so it never exceeds the result list length + const clampedIndex = Math.min(selectedIndex, Math.max(flatResults.length - 1, 0)); + // Active descendant ID for ARIA - const activeDescendantId = flatResults[selectedIndex] - ? `gcp-option-${resultKey(flatResults[selectedIndex])}` + const activeDescendantId = flatResults[clampedIndex] + ? `gcp-option-${resultKey(flatResults[clampedIndex])}` : undefined; // Auto-focus on mount @@ -546,12 +549,7 @@ export function GlobalCommandPalette({ onClose }: GlobalCommandPaletteProps) { if (selectedRef.current && typeof selectedRef.current.scrollIntoView === 'function') { selectedRef.current.scrollIntoView({ block: 'nearest' }); } - }, [selectedIndex]); - - // Reset selection when query changes - useEffect(() => { - setSelectedIndex(0); - }, [query]); + }, [clampedIndex]); // Focus trap — keep Tab within the dialog useEffect(() => { @@ -599,8 +597,8 @@ export function GlobalCommandPalette({ onClose }: GlobalCommandPaletteProps) { break; case 'Enter': e.preventDefault(); - if (flatResults[selectedIndex]) { - executeResult(flatResults[selectedIndex]); + if (flatResults[clampedIndex]) { + executeResult(flatResults[clampedIndex]); } break; case 'Escape': @@ -654,7 +652,7 @@ export function GlobalCommandPalette({ onClose }: GlobalCommandPaletteProps) { aria-controls="gcp-listbox" aria-activedescendant={activeDescendantId} value={query} - onChange={(e) => setQuery(e.target.value)} + onChange={(e) => { setQuery(e.target.value); setSelectedIndex(0); }} onKeyDown={handleKeyDown} placeholder="Search pages, projects, chats, nodes..." className="w-full bg-transparent border-none text-fg-primary text-sm outline-none font-[inherit] placeholder:text-fg-muted focus:ring-0" @@ -689,7 +687,7 @@ export function GlobalCommandPalette({ onClose }: GlobalCommandPaletteProps) { {group.results.map((result) => { flatIndex++; const currentFlatIndex = flatIndex; - const isSelected = currentFlatIndex === selectedIndex; + const isSelected = currentFlatIndex === clampedIndex; return (
| null>(null); - useEffect(() => { - timerRef.current = setTimeout(onDismiss, AUTO_DISMISS_MS); - return () => { - if (timerRef.current) clearTimeout(timerRef.current); - }; - }, [onDismiss]); + if (orphanedSessions.length === 0) return; + const timeoutId = setTimeout(onDismiss, AUTO_DISMISS_MS); + return () => clearTimeout(timeoutId); + }, [onDismiss, orphanedSessions.length]); if (orphanedSessions.length === 0) return null; diff --git a/apps/web/src/components/ProjectAgentCard.tsx b/apps/web/src/components/ProjectAgentCard.tsx index 8a30dd781..c1d36a244 100644 --- a/apps/web/src/components/ProjectAgentCard.tsx +++ b/apps/web/src/components/ProjectAgentCard.tsx @@ -18,7 +18,7 @@ import { VALID_PERMISSION_MODES, } from '@simple-agent-manager/shared'; import { Alert, Button, Card, StatusBadge } from '@simple-agent-manager/ui'; -import { useEffect, useState } from 'react'; +import { useState } from 'react'; import { AgentKeyCard } from './AgentKeyCard'; import { ModelSelect } from './ModelSelect'; @@ -70,6 +70,8 @@ export function ProjectAgentCard({ onSaveDefault, onClearDefault, }: ProjectAgentCardProps) { + // State is initialized from props; parent uses a key that includes defaultValue + // so this component remounts with fresh state when the server data changes. const [model, setModel] = useState(defaultValue?.model ?? ''); const [permissionMode, setPermissionMode] = useState( defaultValue?.permissionMode ?? '', @@ -79,11 +81,6 @@ export function ProjectAgentCard({ const [error, setError] = useState(null); const [success, setSuccess] = useState(false); - useEffect(() => { - setModel(defaultValue?.model ?? ''); - setPermissionMode(defaultValue?.permissionMode ?? ''); - }, [defaultValue]); - const hasCredentialOverride = (projectCredentials?.length ?? 0) > 0; const activeUserCred = userCredentials.find((c) => c.isActive); const hasUserCredential = userCredentials.length > 0; diff --git a/apps/web/src/components/ProjectAgentsSection.tsx b/apps/web/src/components/ProjectAgentsSection.tsx index 162ea83e0..0bfdde408 100644 --- a/apps/web/src/components/ProjectAgentsSection.tsx +++ b/apps/web/src/components/ProjectAgentsSection.tsx @@ -16,6 +16,8 @@ import type { import { Alert, Spinner } from '@simple-agent-manager/ui'; import { useCallback, useEffect, useState } from 'react'; +// NOTE: useEffect is still used for the loadData fetch, not for prop-to-state sync. + import { useToast } from '../hooks/useToast'; import { deleteProjectAgentCredential, @@ -42,14 +44,12 @@ export function ProjectAgentsSection({ const [agents, setAgents] = useState([]); const [projectCreds, setProjectCreds] = useState([]); const [userCreds, setUserCreds] = useState([]); - const [defaults, setDefaults] = useState(initialAgentDefaults ?? {}); + // Derived from prop; no prop-sync effect needed. Mutations update the parent + // via onUpdated(), which passes back fresh initialAgentDefaults. + const defaults: ProjectAgentDefaults = initialAgentDefaults ?? {}; const [loading, setLoading] = useState(true); const [error, setError] = useState(null); - useEffect(() => { - setDefaults(initialAgentDefaults ?? {}); - }, [initialAgentDefaults]); - const loadData = useCallback(async () => { try { setError(null); @@ -111,7 +111,6 @@ export function ProjectAgentsSection({ } const payload = Object.keys(next).length === 0 ? null : next; const updated = await updateProject(projectId, { agentDefaults: payload }); - setDefaults(updated.agentDefaults ?? {}); onUpdated(updated.agentDefaults ?? null); }; @@ -120,7 +119,6 @@ export function ProjectAgentsSection({ delete next[agentType]; const payload = Object.keys(next).length === 0 ? null : next; const updated = await updateProject(projectId, { agentDefaults: payload }); - setDefaults(updated.agentDefaults ?? {}); onUpdated(updated.agentDefaults ?? null); }; @@ -158,7 +156,7 @@ export function ProjectAgentsSection({ const userAgentCreds = userCreds.filter((c) => c.agentType === agent.id); return ( 0 ? projectAgentCreds : null} userCredentials={userAgentCreds} diff --git a/apps/web/src/components/RepoSelector.tsx b/apps/web/src/components/RepoSelector.tsx index 3ffe988f9..d9e88f9f8 100644 --- a/apps/web/src/components/RepoSelector.tsx +++ b/apps/web/src/components/RepoSelector.tsx @@ -1,6 +1,6 @@ import type { Repository } from '@simple-agent-manager/shared'; import { Alert, Input, Spinner } from '@simple-agent-manager/ui'; -import { useCallback,useEffect, useRef, useState } from 'react'; +import { useCallback,useEffect, useMemo, useRef, useState } from 'react'; import { createPortal } from 'react-dom'; import { listRepositories } from '../lib/api'; @@ -29,7 +29,6 @@ export function RepoSelector({ placeholder = 'https://github.com/user/repo or select from list', }: RepoSelectorProps) { const [repositories, setRepositories] = useState([]); - const [filteredRepos, setFilteredRepos] = useState([]); const [showDropdown, setShowDropdown] = useState(false); const [loading, setLoading] = useState(false); const [error, setError] = useState(null); @@ -67,16 +66,14 @@ export function RepoSelector({ fetchRepos(); }, [installationId]); - // Filter repositories based on input - useEffect(() => { + // Filter repositories based on input (derived at render time) + const filteredRepos = useMemo(() => { if (value.startsWith('http') || value.startsWith('git@')) { - setFilteredRepos([]); - return; + return []; } if (!value) { - setFilteredRepos(repositories.slice(0, 25)); - return; + return repositories.slice(0, 25); } const searchTerm = value.toLowerCase(); @@ -85,7 +82,7 @@ export function RepoSelector({ ); // Show more results (25) to help users with many repos find what they need - setFilteredRepos(filtered.slice(0, 25)); + return filtered.slice(0, 25); }, [value, repositories]); // Handle clicks outside dropdown diff --git a/apps/web/src/components/ScalingSettings.tsx b/apps/web/src/components/ScalingSettings.tsx index 7efa8fde0..878413cb0 100644 --- a/apps/web/src/components/ScalingSettings.tsx +++ b/apps/web/src/components/ScalingSettings.tsx @@ -10,7 +10,7 @@ import { type ScalingParamMeta, } from '@simple-agent-manager/shared'; import { Button } from '@simple-agent-manager/ui'; -import { useCallback,useEffect, useState } from 'react'; +import { useCallback, useEffect, useState } from 'react'; import { useToast } from '../hooks/useToast'; import { listCredentials,updateProject } from '../lib/api'; @@ -92,20 +92,59 @@ export function ScalingSettings({ projectId: string; project: Project; reload: () => Promise; +}) { + const [configuredProviders, setConfiguredProviders] = useState([]); + + // Fetch configured providers (legitimate data-fetch effect) + useEffect(() => { + listCredentials() + .then((creds) => { + const providers = [...new Set( + creds + .filter((c) => c.connected) + .map((c) => c.provider) + )]; + setConfiguredProviders(providers); + }) + .catch((err: unknown) => { console.error('Failed to load credentials', err); }); + }, []); + + // Key the form by project.updatedAt so it remounts with fresh state + // when the project is reloaded after a save, removing the prop-sync effect. + return ( + + ); +} + +function ScalingSettingsForm({ + projectId, + project, + reload, + configuredProviders, +}: { + projectId: string; + project: Project; + reload: () => Promise; + configuredProviders: CredentialProvider[]; }) { const toast = useToast(); - // Provider & Location state + // State initialized from props; no sync effect needed because the parent + // keys this component by project.updatedAt. const [selectedProvider, setSelectedProvider] = useState( project.defaultProvider ?? null ); const [selectedLocation, setSelectedLocation] = useState( project.defaultLocation ?? null ); - const [configuredProviders, setConfiguredProviders] = useState([]); const [savingLocation, setSavingLocation] = useState(false); - // Scaling params state const [scalingValues, setScalingValues] = useState>(() => { const initial: Record = {}; for (const p of SCALING_PARAMS) { @@ -118,32 +157,6 @@ export function ScalingSettings({ ); const [savingScaling, setSavingScaling] = useState(false); - // Fetch configured providers - useEffect(() => { - listCredentials() - .then((creds) => { - const providers = [...new Set( - creds - .filter((c) => c.connected) - .map((c) => c.provider) - )]; - setConfiguredProviders(providers); - }) - .catch((err: unknown) => { console.error('Failed to load credentials', err); }); - }, []); - - // Sync from project prop - useEffect(() => { - setSelectedProvider(project.defaultProvider ?? null); - setSelectedLocation(project.defaultLocation ?? null); - const updated: Record = {}; - for (const p of SCALING_PARAMS) { - updated[p.key] = (project[p.key as keyof Project] as number | null) ?? null; - } - setScalingValues(updated); - setNodeIdleTimeoutMs(project.nodeIdleTimeoutMs ?? null); - }, [project]); - const handleSaveProviderLocation = useCallback(async () => { setSavingLocation(true); try { diff --git a/apps/web/src/components/account-map/hooks/useMapFilters.ts b/apps/web/src/components/account-map/hooks/useMapFilters.ts index e7dcad312..8ff427e91 100644 --- a/apps/web/src/components/account-map/hooks/useMapFilters.ts +++ b/apps/web/src/components/account-map/hooks/useMapFilters.ts @@ -1,5 +1,5 @@ import type { Edge,Node } from '@xyflow/react'; -import { useCallback, useMemo, useRef,useState } from 'react'; +import { useCallback, useMemo, useState } from 'react'; export type EntityType = 'project' | 'node' | 'workspace' | 'session' | 'task' | 'idea'; @@ -28,8 +28,6 @@ interface UseMapFiltersResult { hasActiveFilters: boolean; matchCount: number; totalCount: number; - /** True when type filters changed the visible node count — signals auto-reorganize. */ - filterNodeCountChanged: boolean; } const ALL_TYPES: EntityType[] = ['project', 'node', 'workspace', 'session', 'task']; @@ -37,7 +35,6 @@ const ALL_TYPES: EntityType[] = ['project', 'node', 'workspace', 'session', 'tas export function useMapFilters({ nodes, edges }: UseMapFiltersOptions): UseMapFiltersResult { const [searchQuery, setSearchQuery] = useState(''); const [activeFilters, setActiveFilters] = useState>(() => new Set(ALL_TYPES)); - const prevFilteredCountRef = useRef(null); const toggleFilter = useCallback((type: EntityType) => { setActiveFilters((prev) => { @@ -56,7 +53,7 @@ export function useMapFilters({ nodes, edges }: UseMapFiltersOptions): UseMapFil setSearchQuery(''); }, []); - const { filteredNodes, filteredEdges, matchCount, totalCount, filterNodeCountChanged } = useMemo(() => { + const { filteredNodes, filteredEdges, matchCount, totalCount } = useMemo(() => { const lowerQuery = searchQuery.toLowerCase().trim(); // Type filters: fully REMOVE nodes whose type is toggled off @@ -120,17 +117,11 @@ export function useMapFilters({ nodes, edges }: UseMapFiltersOptions): UseMapFil }, })); - // Detect if the type filter changed the visible node count (triggers auto-reorganize) - const currentCount = typeFiltered.length; - const changed = prevFilteredCountRef.current !== null && prevFilteredCountRef.current !== currentCount; - prevFilteredCountRef.current = currentCount; - return { filteredNodes: styledNodes, filteredEdges: styledEdges, matchCount: matchingIds.size, totalCount: typeFiltered.length, - filterNodeCountChanged: changed, }; }, [nodes, edges, searchQuery, activeFilters]); @@ -147,6 +138,5 @@ export function useMapFilters({ nodes, edges }: UseMapFiltersOptions): UseMapFil hasActiveFilters, matchCount, totalCount, - filterNodeCountChanged, }; } diff --git a/apps/web/src/components/agent-profiles/ProfileFormDialog.tsx b/apps/web/src/components/agent-profiles/ProfileFormDialog.tsx index ce93b3f79..5282a499d 100644 --- a/apps/web/src/components/agent-profiles/ProfileFormDialog.tsx +++ b/apps/web/src/components/agent-profiles/ProfileFormDialog.tsx @@ -18,7 +18,7 @@ import { VALID_PERMISSION_MODES, } from '@simple-agent-manager/shared'; import { Button, Dialog, Input } from '@simple-agent-manager/ui'; -import { type FC, type ReactNode, useEffect, useState } from 'react'; +import { type FC, type ReactNode, useEffect, useMemo, useState } from 'react'; import { getProject, getProviderCatalog } from '../../lib/api'; import { ModelSelect } from '../ModelSelect'; @@ -240,68 +240,74 @@ export const ProfileFormDialog: FC = ({ onSave, projectId, }) => { + // Key the form body so React remounts it with fresh state when the + // profile changes or the dialog re-opens, eliminating the prop-sync effect. + const formKey = useMemo( + () => (isOpen ? `${profile?.id ?? 'new'}-${Date.now()}` : 'closed'), + // eslint-disable-next-line react-hooks/exhaustive-deps -- intentional: remount on each open + [isOpen, profile?.id], + ); + + return ( + + {isOpen && ( + + )} + + ); +}; + +// --------------------------------------------------------------------------- +// Inner form body — keyed by the parent so it remounts with fresh state +// --------------------------------------------------------------------------- + +interface ProfileFormBodyProps { + profile: AgentProfile | null; + onSave: (data: CreateAgentProfileRequest | UpdateAgentProfileRequest) => Promise; + onClose: () => void; + projectId: string; +} + +function ProfileFormBody({ profile, onSave, onClose, projectId }: ProfileFormBodyProps) { const isEdit = !!profile; - const [name, setName] = useState(''); - const [description, setDescription] = useState(''); - const [agentType, setAgentType] = useState(DEFAULT_AGENT_TYPE); - const [model, setModel] = useState(''); - const [effort, setEffort] = useState(DEFAULT_AGENT_EFFORT); - const [permissionMode, setPermissionMode] = useState(''); - const [systemPromptAppend, setSystemPromptAppend] = useState(''); - const [maxTurns, setMaxTurns] = useState(''); - const [timeoutMinutes, setTimeoutMinutes] = useState(''); - const [vmSizeOverride, setVmSizeOverride] = useState(''); - const [workspaceProfile, setWorkspaceProfile] = useState(''); - const [devcontainerConfigName, setDevcontainerConfigName] = useState(''); - const [taskMode, setTaskMode] = useState(''); - const [githubCliPolicy, setGithubCliPolicy] = - useState(DEFAULT_GITHUB_CLI_POLICY); + // All state initialized from the profile prop; no sync effect needed. + const [name, setName] = useState(profile?.name ?? ''); + const [description, setDescription] = useState(profile?.description ?? ''); + const [agentType, setAgentType] = useState(profile?.agentType ?? DEFAULT_AGENT_TYPE); + const [model, setModel] = useState(profile?.model ?? ''); + const [effort, setEffort] = useState(profile?.effort ?? DEFAULT_AGENT_EFFORT); + const [permissionMode, setPermissionMode] = useState(profile?.permissionMode ?? ''); + const [systemPromptAppend, setSystemPromptAppend] = useState(profile?.systemPromptAppend ?? ''); + const [maxTurns, setMaxTurns] = useState( + profile?.maxTurns != null ? String(profile.maxTurns) : '', + ); + const [timeoutMinutes, setTimeoutMinutes] = useState( + profile?.timeoutMinutes != null ? String(profile.timeoutMinutes) : '', + ); + const [vmSizeOverride, setVmSizeOverride] = useState(profile?.vmSizeOverride ?? ''); + const [workspaceProfile, setWorkspaceProfile] = useState(profile?.workspaceProfile ?? ''); + const [devcontainerConfigName, setDevcontainerConfigName] = useState( + profile?.devcontainerConfigName ?? '', + ); + const [taskMode, setTaskMode] = useState(profile?.taskMode ?? ''); + const [githubCliPolicy, setGithubCliPolicy] = useState( + profile?.githubCliPolicy ?? DEFAULT_GITHUB_CLI_POLICY, + ); const [catalogs, setCatalogs] = useState([]); const [projectProvider, setProjectProvider] = useState(null); const [projectLocation, setProjectLocation] = useState(null); const [saving, setSaving] = useState(false); const [error, setError] = useState(null); - // Reset form when opening/closing or when profile changes + // Fetch provider catalog and project defaults (legitimate data-fetch effect) useEffect(() => { - if (isOpen && profile) { - setName(profile.name); - setDescription(profile.description ?? ''); - setAgentType(profile.agentType); - setModel(profile.model ?? ''); - setEffort(profile.effort ?? DEFAULT_AGENT_EFFORT); - setPermissionMode(profile.permissionMode ?? ''); - setSystemPromptAppend(profile.systemPromptAppend ?? ''); - setMaxTurns(profile.maxTurns != null ? String(profile.maxTurns) : ''); - setTimeoutMinutes(profile.timeoutMinutes != null ? String(profile.timeoutMinutes) : ''); - setVmSizeOverride(profile.vmSizeOverride ?? ''); - setWorkspaceProfile(profile.workspaceProfile ?? ''); - setDevcontainerConfigName(profile.devcontainerConfigName ?? ''); - setTaskMode(profile.taskMode ?? ''); - setGithubCliPolicy(profile.githubCliPolicy ?? DEFAULT_GITHUB_CLI_POLICY); - } else if (isOpen) { - setName(''); - setDescription(''); - setAgentType(DEFAULT_AGENT_TYPE); - setModel(''); - setEffort(DEFAULT_AGENT_EFFORT); - setPermissionMode(''); - setSystemPromptAppend(''); - setMaxTurns(''); - setTimeoutMinutes(''); - setVmSizeOverride(''); - setWorkspaceProfile(''); - setDevcontainerConfigName(''); - setTaskMode(''); - setGithubCliPolicy(DEFAULT_GITHUB_CLI_POLICY); - } - setError(null); - }, [isOpen, profile]); - - useEffect(() => { - if (!isOpen) return; - let cancelled = false; Promise.all([getProviderCatalog(), getProject(projectId)]) .then(([catalogResponse, project]) => { @@ -321,7 +327,7 @@ export const ProfileFormDialog: FC = ({ return () => { cancelled = true; }; - }, [isOpen, projectId]); + }, [projectId]); const effectiveProvider = profile?.provider ?? projectProvider; const activeCatalog = selectProviderCatalog(catalogs, effectiveProvider); @@ -383,270 +389,268 @@ export const ProfileFormDialog: FC = ({ }; return ( - -
{ - e.preventDefault(); - void handleSubmit(); - }} - > -

- {isEdit ? 'Edit Profile' : 'Create Agent Profile'} -

- - {error && ( -
- {error} -
- )} + { + e.preventDefault(); + void handleSubmit(); + }} + > +

+ {isEdit ? 'Edit Profile' : 'Create Agent Profile'} +

+ + {error && ( +
+ {error} +
+ )} - {/* Name & Description — always visible */} -
-