From a3ec04ad99c179645f7eada5979efa7a7377b677 Mon Sep 17 00:00:00 2001 From: Gabriel Brown Date: Sat, 11 Jul 2026 17:09:12 -0400 Subject: [PATCH] fix(ui): re-sync settings/thread forms on loaded record --- .../spoons/spoon-agent-settings-form.tsx | 43 +++++- .../threads/thread-workspace-form.tsx | 30 +++- .../next/tests/component/form-resync.test.tsx | 145 ++++++++++++++++++ 3 files changed, 216 insertions(+), 2 deletions(-) create mode 100644 apps/next/tests/component/form-resync.test.tsx diff --git a/apps/next/src/components/spoons/spoon-agent-settings-form.tsx b/apps/next/src/components/spoons/spoon-agent-settings-form.tsx index 225fcc6..d290e8a 100644 --- a/apps/next/src/components/spoons/spoon-agent-settings-form.tsx +++ b/apps/next/src/components/spoons/spoon-agent-settings-form.tsx @@ -1,6 +1,6 @@ 'use client'; -import { useState } from 'react'; +import { useEffect, useRef, useState } from 'react'; import { useMutation, useQuery } from 'convex/react'; import { Bot } from 'lucide-react'; import { toast } from 'sonner'; @@ -33,6 +33,7 @@ const runtimeLabels: Record = { }; type AgentSettings = { + _id?: Id<'spoonAgentSettings'>; enabled: boolean; runtime?: RuntimeName | 'openai_direct'; defaultBaseBranch?: string; @@ -127,6 +128,46 @@ export const SpoonAgentSettingsForm = ({ : settings.reasoningEffort, ); + // The parent gates rendering on a different query than `settings`, so this + // form can mount before the agent settings record resolves. Re-seed every + // field once the real record arrives (and again if the parent swaps to a + // different spoon) so a Save cannot persist the initial defaults over the + // user's saved configuration. + const hydratedSpoonId = useRef | null>(null); + useEffect(() => { + if (!settings || hydratedSpoonId.current === spoon._id) return; + const timeout = window.setTimeout(() => { + setEnabled(settings.enabled); + setDefaultBaseBranch( + settings.defaultBaseBranch ?? + spoon.forkDefaultBranch ?? + spoon.upstreamDefaultBranch, + ); + setBranchPrefix(settings.branchPrefix); + setInstallCommand(settings.installCommand ?? ''); + setCheckCommand(settings.checkCommand ?? ''); + setTestCommand(settings.testCommand ?? ''); + setEnvFilePath(settings.envFilePath ?? '.env.local'); + setCustomEnvFilePath(settings.customEnvFilePath ?? ''); + setMaterializeEnvFileByDefault( + settings.materializeEnvFileByDefault ?? false, + ); + setAutoDetectCommands(settings.autoDetectCommands ?? true); + setAllowUserFileEditing(settings.allowUserFileEditing ?? true); + setAiProviderProfileId(settings.aiProviderProfileId ?? '__default'); + setAgentModel(settings.aiProviderProfileId ? settings.agentModel : ''); + setReasoningEffort( + !settings.aiProviderProfileId + ? 'medium' + : settings.reasoningEffort === 'none' + ? 'minimal' + : settings.reasoningEffort, + ); + hydratedSpoonId.current = spoon._id; + }, 0); + return () => window.clearTimeout(timeout); + }, [settings, spoon]); + const supportedRuntimes = (selectedProfile?.supportedRuntimes ?? []) as RuntimeName[]; const settingsRuntime = settings?.runtime as RuntimeName | undefined; diff --git a/apps/next/src/components/threads/thread-workspace-form.tsx b/apps/next/src/components/threads/thread-workspace-form.tsx index a3645e1..c102765 100644 --- a/apps/next/src/components/threads/thread-workspace-form.tsx +++ b/apps/next/src/components/threads/thread-workspace-form.tsx @@ -1,6 +1,6 @@ 'use client'; -import { useState } from 'react'; +import { useEffect, useRef, useState } from 'react'; import { useRouter } from 'next/navigation'; import { useMutation, useQuery } from 'convex/react'; import { MessageSquarePlus } from 'lucide-react'; @@ -34,6 +34,7 @@ const runtimeLabels: Record = { }; type AgentSettings = { + _id?: Id<'spoonAgentSettings'>; defaultBaseBranch?: string; runtime?: RuntimeName | 'openai_direct'; agentModel: string; @@ -82,6 +83,33 @@ export const ThreadWorkspaceForm = ({ ); const [runtime, setRuntime] = useState(); const [submitting, setSubmitting] = useState(false); + + // The parent gates rendering on a different query than `agentSettings`, so + // this form can mount before the agent settings record resolves. Re-seed the + // settings-derived fields once the real record arrives (and again if the + // parent swaps to a different spoon) so the form reflects saved config rather + // than the initial defaults. + const hydratedSpoonId = useRef | null>(null); + useEffect(() => { + if (!agentSettings || hydratedSpoonId.current === spoon._id) return; + const timeout = window.setTimeout(() => { + setBaseBranch( + agentSettings.defaultBaseBranch ?? + spoon.forkDefaultBranch ?? + spoon.upstreamDefaultBranch, + ); + setMaterializeEnvFile(agentSettings.materializeEnvFileByDefault ?? false); + setEnvFilePath( + agentSettings.envFilePath === 'custom' + ? (agentSettings.customEnvFilePath ?? '.env.local') + : (agentSettings.envFilePath ?? '.env.local'), + ); + setAiProviderProfileId(agentSettings.aiProviderProfileId ?? '__settings'); + hydratedSpoonId.current = spoon._id; + }, 0); + return () => window.clearTimeout(timeout); + }, [agentSettings, spoon]); + const effectiveProviderProfileId = aiProviderProfileId === '__settings' ? (agentSettings?.aiProviderProfileId ?? defaultProfile?._id) diff --git a/apps/next/tests/component/form-resync.test.tsx b/apps/next/tests/component/form-resync.test.tsx new file mode 100644 index 0000000..a71c0eb --- /dev/null +++ b/apps/next/tests/component/form-resync.test.tsx @@ -0,0 +1,145 @@ +import { cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react'; +import { getFunctionName } from 'convex/server'; +import { afterEach, describe, expect, it, vi } from 'vitest'; + +import { api } from '@spoon/backend/convex/_generated/api.js'; + +import { SpoonAgentSettingsForm } from '../../src/components/spoons/spoon-agent-settings-form'; + +const { mockUseMutation, mockUseQuery } = vi.hoisted(() => ({ + mockUseMutation: vi.fn(), + mockUseQuery: vi.fn(), +})); + +vi.mock('convex/react', () => ({ + useMutation: mockUseMutation, + useQuery: mockUseQuery, +})); + +vi.mock('sonner', () => ({ + toast: { + error: vi.fn(), + success: vi.fn(), + }, +})); + +const spoon = { + _id: 'spoon-1', + forkDefaultBranch: 'main', + upstreamDefaultBranch: 'main', +} as never; + +const configuredProfile = { + _id: 'profile-1', + name: 'OpenAI', + provider: 'openai', + enabled: true, + configured: true, + isDefault: true, + supportedRuntimes: ['codex'], + reasoningEffort: 'high', +}; + +const modelCatalog = { + profiles: [ + { + profileId: 'profile-1', + configured: true, + defaultModel: 'gpt-5', + models: [{ id: 'gpt-5', label: 'GPT-5' }], + }, + ], +}; + +const loadedSettings = { + _id: 'settings-1' as never, + enabled: true, + runtime: 'codex' as const, + defaultBaseBranch: 'develop', + branchPrefix: 'custom/prefix', + installCommand: 'bun install', + agentModel: 'gpt-5', + reasoningEffort: 'high' as const, + envFilePath: '.env', + aiProviderProfileId: 'profile-1' as never, +}; + +const wireQueries = () => { + const listMineName = getFunctionName(api.aiProviderProfiles.listMine); + const catalogName = getFunctionName(api.aiProviderModels.listAvailableForUser); + mockUseQuery.mockImplementation( + (ref: Parameters[0]) => { + const name = getFunctionName(ref); + if (name === listMineName) return [configuredProfile]; + if (name === catalogName) return modelCatalog; + return undefined; + }, + ); +}; + +afterEach(() => { + cleanup(); + vi.clearAllMocks(); +}); + +describe('SpoonAgentSettingsForm re-sync on loaded record', () => { + it('reflects the loaded settings after the record resolves late', async () => { + wireQueries(); + mockUseMutation.mockReturnValue(vi.fn()); + + // Parent gates on a different query, so this form mounts before the + // agent settings query resolves (settings === undefined). + const { rerender } = render( + , + ); + + expect(screen.getByLabelText(/branch prefix/i)).toHaveValue('spoon/agent'); + + // The settings query resolves with a saved record carrying non-defaults. + rerender(); + + await waitFor(() => + expect(screen.getByLabelText(/branch prefix/i)).toHaveValue( + 'custom/prefix', + ), + ); + expect(screen.getByLabelText(/default base branch/i)).toHaveValue( + 'develop', + ); + expect(screen.getByLabelText(/install command/i)).toHaveValue( + 'bun install', + ); + }); + + it('saves the loaded values, not the stale defaults, after a late resolve', async () => { + wireQueries(); + const update = vi.fn().mockResolvedValue('settings-1'); + mockUseMutation.mockReturnValue(update); + + const { rerender } = render( + , + ); + rerender(); + + // Wait for the late record to hydrate the form before saving. + await waitFor(() => + expect(screen.getByLabelText(/branch prefix/i)).toHaveValue( + 'custom/prefix', + ), + ); + + fireEvent.click( + screen.getByRole('button', { name: /save agent settings/i }), + ); + + await waitFor(() => expect(update).toHaveBeenCalledTimes(1)); + expect(update).toHaveBeenCalledWith( + expect.objectContaining({ + branchPrefix: 'custom/prefix', + defaultBaseBranch: 'develop', + installCommand: 'bun install', + agentModel: 'gpt-5', + }), + ); + }); +});