From d4a87accb85b94eb1ff480f2334d143185bff623 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Sun, 19 Jul 2026 22:16:11 -0700 Subject: [PATCH] FN-8395: autosave settings changes Settings edits now persist automatically with safe feedback and close handling. - Debounce and serialize settings persistence, retaining newer edits through in-flight saves and unmounts. - Remove the Settings Save action and unsaved-changes prompt while flushing pending section and workflow-lane updates. - Update Settings documentation, release metadata, and regression coverage for auto-save behavior. Files changed: .changeset/fn-8395-settings-autosave.md | 7 + docs/dashboard-guide.md | 11 +- docs/settings-reference.md | 8 +- .../dashboard/app/components/SettingsModal.tsx | 402 ++++++++++++++++----- .../__tests__/SettingsModal.general.test.tsx | 154 ++++++-- .../SettingsModal.keyboardShortcuts.test.tsx | 7 +- .../__tests__/SettingsModal.models-auth.test.tsx | 46 +-- .../SettingsModal.remote-notifications.test.tsx | 38 +- .../SettingsModal.scheduling-merge.test.tsx | 58 ++- .../__tests__/SettingsModal.test-harness.tsx | 9 +- .../__tests__/SettingsModalNodeRouting.test.tsx | 2 - .../settings/sections/ProjectModelsSection.tsx | 15 +- 12 files changed, 548 insertions(+), 209 deletions(-) Fusion-Task-Id: FN-8395 Fusion-Task-Lineage: 77f4c0d3-d0cc-4973-9831-c70eb2a2b49b Co-authored-by: Fusion (runfusion.ai) --- .changeset/fn-8395-settings-autosave.md | 7 + docs/dashboard-guide.md | 11 +- docs/settings-reference.md | 8 +- .../app/components/SettingsModal.tsx | 402 ++++++++++++++---- .../__tests__/SettingsModal.general.test.tsx | 154 +++++-- .../SettingsModal.keyboardShortcuts.test.tsx | 7 +- .../SettingsModal.models-auth.test.tsx | 46 +- ...ettingsModal.remote-notifications.test.tsx | 38 +- .../SettingsModal.scheduling-merge.test.tsx | 58 ++- .../__tests__/SettingsModal.test-harness.tsx | 9 +- .../SettingsModalNodeRouting.test.tsx | 2 - .../sections/ProjectModelsSection.tsx | 15 +- 12 files changed, 548 insertions(+), 209 deletions(-) create mode 100644 .changeset/fn-8395-settings-autosave.md diff --git a/.changeset/fn-8395-settings-autosave.md b/.changeset/fn-8395-settings-autosave.md new file mode 100644 index 0000000000..6b13d2b947 --- /dev/null +++ b/.changeset/fn-8395-settings-autosave.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": minor +--- + +summary: Save Settings edits automatically and safely flush pending changes when closing. +category: fix +dev: Removes the main Settings footer Save action in favor of debounced persistence and status feedback. diff --git a/docs/dashboard-guide.md b/docs/dashboard-guide.md index 0426a37b9d..7f0ad3c6ca 100644 --- a/docs/dashboard-guide.md +++ b/docs/dashboard-guide.md @@ -29,6 +29,9 @@ On desktop and tablet, the Settings navigation rail has no hard divider between Every user-editable setting's help text (the `.settings-description`/`` hint under a field) states its own default value — for example “Default: 3.”, “Default: enabled.”, or “No default — unset (inherits the global setting).” for values that fall back to another scope. Canonical default values come from `DEFAULT_GLOBAL_SETTINGS` / `DEFAULT_PROJECT_SETTINGS` in `packages/core/src/settings-schema.ts`; the dashboard copy never invents a number. A guard test (`settings-default-descriptions.test.tsx`) enforces that every surfaced setting states its default and that every `DEFAULT_SETTINGS` key is either documented or explicitly allowlisted as not surfaced in the Settings UI. + +Settings form changes save automatically after a short pause. The footer no longer includes a **Save** button and closing Settings does not ask about unsaved changes: Close, Escape, and clicking outside the modal first flush any pending edit. The footer shows quiet **Saving…**, **Saved**, or save-failure status; correct the value and retry after a failure. + ## Reset Settings - Optional-group, foreach, and loop containers show their template nodes inside the block. Canvas connections between the surrounding workflow and the block attach to the container boundary; connections between template nodes stay inside the block. Optional groups also draw non-editable entry/exit connector lines between the boundary and the template entry/exit nodes so single-step blocks such as Plan Review and Code Review do not look disconnected; those visual connectors are not saved into workflow IR. - The Settings panel is value-first for built-in workflows and groups workflow settings by Models, Review & Approval, Step Execution, and Advanced. Known workflow model values use the same model dropdown picker as **Settings → Project Models** so provider/model pairs and their inline Thinking Level companions are saved together; custom or non-model string values can still use typed inputs. Definitions remain available for custom workflow schema authoring. -- The main Settings modal also exposes the default workflow's Plan/Triage, Executor, Reviewer, and declared Planning/Reviewer fallback model lanes from **Project Models**; the modal's primary **Save** action writes those dropdown values as workflow setting values for the active default workflow. Project Models also includes project-scoped **Merger** and **Title Summarization** lanes (not workflow-moved). Settings fallback model pickers (Global Fallback Model, workflow fallbacks, and project Title Summarizer Fallback) include the same inline Thinking Level selector when their companion key exists. +- The main Settings modal also exposes the default workflow's Plan/Triage, Executor, Reviewer, and declared Planning/Reviewer fallback model lanes from **Project Models**; those dropdown values auto-save as workflow setting values for the active default workflow. Project Models also includes project-scoped **Merger** and **Title Summarization** lanes (not workflow-moved). Settings fallback model pickers (Global Fallback Model, workflow fallbacks, and project Title Summarizer Fallback) include the same inline Thinking Level selector when their companion key exists. - On desktop, the editor uses a multi-panel canvas layout for editing the graph and adjacent workflow metadata. The **Show simple editor** toggle switches that same workflow into the graph-outline editor with dedicated **Graph**, **Add**, **Settings**, **Fields**, **Columns**, and **Actions** tabs. - On viewports `<=768px`, the editor switches to a full-screen mobile sheet. Global workflow entry points open to the workflow list with no workflow preselected and prompt users to select a workflow to edit; the Board/List workflow dropdown row edit action opens directly to the selected workflow editor when that selected workflow is available. - Simple/mobile editing uses a graph outline instead of making the canvas the primary control. The outline shows nodes, branch/rework edges, column placement, and optional-group/foreach/loop template children as tappable rows and chips that open the same node and edge detail editors as desktop. The structural **start** node opens an inspector for the workflow entry column when the workflow defines columns; the **Name** field remains unavailable because the start label is structural. For custom workflows, editable outline rows also expose **Move up** and **Move down** controls that reorder steps within their current column or template parent; built-in workflows remain read-only and hide those controls. @@ -1594,7 +1597,11 @@ Dashboard remote controls live in **Settings → Remote Access**. From this section, operators can: - Configure Tailscale and Cloudflare provider fields -- Save provider options such as Tailscale **Accept routes** and **Remember last running state** with the main Settings **Save** button; starting a tunnel is not required for these settings to persist. +- Provider options such as Tailscale **Accept routes** and **Remember last running state** auto-save after an edit; starting a tunnel is not required for these settings to persist. + +### Settings auto-save + +Settings form edits auto-save after a short debounce. The Settings footer has no Save button and never shows an unsaved-changes leave warning. Closing Settings, including with Close, Escape, or the backdrop, flushes a pending edit before dismissal so the final change is retained. - Activate the current provider - Start/stop tunnel lifecycle manually - Generate login URLs / QR payloads using persistent or short-lived token mode diff --git a/docs/settings-reference.md b/docs/settings-reference.md index 00531ae56a..040dfd7e18 100644 --- a/docs/settings-reference.md +++ b/docs/settings-reference.md @@ -271,7 +271,7 @@ govern that execution belong to the workflow. **Where to set them.** The common model lanes for a project's default workflow are available directly in **Settings → Project Models → Default workflow model lanes**: Plan/Triage, Executor, Reviewer, and the Planning/Reviewer fallback lanes declared -by the default workflow. Primary Plan/Triage, Executor, Reviewer, and declared fallback rows show an inline Thinking Level control when the workflow declares the companion `*ThinkingLevel` setting; unset means inherit. Those dropdown controls use the shared model picker and are persisted by the Settings modal's primary **Save** action, which writes +by the default workflow. Primary Plan/Triage, Executor, Reviewer, and declared fallback rows show an inline Thinking Level control when the workflow declares the companion `*ThinkingLevel` setting; unset means inherit. Those dropdown controls use the shared model picker and are auto-saved by the Settings modal after an edit, which writes workflow setting values for the active project's default workflow; they do not restore the old project settings keys. The global **Fallback Model** remains in Settings → General Models and includes its own inline Thinking Level selector for `fallbackThinkingLevel`; workflow-specific fallbacks are also editable from @@ -400,7 +400,7 @@ When `triageProactiveSubtaskSplittingEnabled` is `true` (the default), triage ma In the dashboard Settings modal, Project Models exposes Plan/Triage, Executor, Reviewer, and declared fallback dropdown controls for the default workflow. The -modal's primary **Save** action persists pending default-workflow model lane +Settings modal auto-save persists pending default-workflow model lane overrides; there is no separate workflow-model save button. The workflow editor's Settings → Values tab uses the same dropdown picker for declared provider/model pairs, including fallbacks. Former locations for advanced workflow policy still @@ -1028,7 +1028,7 @@ Short-lived token bounds are enforced server-side: ## Model Selection Hierarchy -Fusion resolves task models through workflow-backed lane values first, then global lane defaults, then the project/global default model fallback. The common workflow lanes are stored as setting values on the project's default workflow and can be edited with dropdown controls from Settings -> Project Models -> Default workflow model lanes (persisted by the Settings modal's primary Save) or from workflow editor -> Settings -> Values for declared workflow lanes and fallbacks. General-scope fallback selection remains the global Fallback Model picker in Settings -> General Models. +Fusion resolves task models through workflow-backed lane values first, then global lane defaults, then the project/global default model fallback. The common workflow lanes are stored as setting values on the project's default workflow and can be edited with dropdown controls from Settings -> Project Models -> Default workflow model lanes (auto-saved by the Settings modal) or from workflow editor -> Settings -> Values for declared workflow lanes and fallbacks. General-scope fallback selection remains the global Fallback Model picker in Settings -> General Models. Direct-chat defaults are project-scoped and independent of task workflow lanes. Configure them in **Settings -> Project Models -> Chat**. `chatDefaultKind: "agent"` resolves only when `chatDefaultAgentId` is set; `chatDefaultKind: "model"` resolves only when both `chatDefaultModelProvider` and `chatDefaultModelId` are set, with optional `chatDefaultThinkingLevel`. If `chatNewSessionMode` is `"always-default"` and that target resolves, every New Chat entry point creates the session directly. If the target is incomplete, or the mode is unset/`"prompt"`, Fusion opens the New Chat dialog instead and preselects the resolved default when one exists. Chat Rooms additionally support a per-room `thinkingLevel` default that applies to every room responder; clearing it inherits the resolved project/global default. @@ -1792,4 +1792,4 @@ Escalation is enabled only when the toggle is true and either a complete provide ### `mobileNavPrimaryItems` -Project-scoped ordered list of up to six mobile footer quick actions. The default remains `command-center`, `tasks`, `agents`, `missions`, `chat`, `mailbox`. Settings shows selected items in order with move/remove controls and an add dropdown for eligible navigation destinations (including More-sheet actions and gated views); edits preview in the live footer before Save. Unknown ids plus `more`, Terminal/scripts, shell controls, plugin views, and separators are ignored. Omitted available destinations remain reachable in More, whose trailing footer tab is always present. Disabled feature-gated destinations render nowhere until their feature is enabled. +Project-scoped ordered list of up to six mobile footer quick actions. The default remains `command-center`, `tasks`, `agents`, `missions`, `chat`, `mailbox`. Settings shows selected items in order with move/remove controls and an add dropdown for eligible navigation destinations (including More-sheet actions and gated views); edits preview in the live footer and auto-save after editing. Unknown ids plus `more`, Terminal/scripts, shell controls, plugin views, and separators are ignored. Omitted available destinations remain reachable in More, whose trailing footer tab is always present. Disabled feature-gated destinations render nowhere until their feature is enabled. diff --git a/packages/dashboard/app/components/SettingsModal.tsx b/packages/dashboard/app/components/SettingsModal.tsx index 61b9297295..8779a9578d 100644 --- a/packages/dashboard/app/components/SettingsModal.tsx +++ b/packages/dashboard/app/components/SettingsModal.tsx @@ -1129,10 +1129,22 @@ export function SettingsModal({ const modalRef = useRef(null); const settingsContentRef = useRef(null); const workflowLaneSaverRef = useRef(null); + /* + FNXC:SettingsAutoSave 2026-08-03-01:00: + Workflow lane edits live outside the shared Settings form. Track their revision + alongside form dirtiness so Option 1 auto-save and every close path flush them + too; a completion only clears the revision it actually persisted. + */ + const workflowLaneRevisionRef = useRef(0); + const [workflowLanesDirty, setWorkflowLanesDirty] = useState(false); + const markWorkflowLanesDirty = useCallback(() => { + workflowLaneRevisionRef.current += 1; + setWorkflowLanesDirty(true); + }, []); const registerWorkflowLaneSaver = useCallback((saver: SectionSaveHandler | null) => { /* FNXC:ProjectModelsWorkflowLanes 2026-07-14-09:07: - Project Models workflow lane edits are workflow setting-values, not normal project settings. Keep the latest saver registered across section unmounts so the primary Settings Save still flushes project-scoped workflow overrides when operators navigate away before saving. + Project Models workflow lane edits are workflow setting-values, not normal project settings. Keep the latest saver registered across section unmounts so auto-save and close flushing retain project-scoped workflow overrides after navigation. */ if (saver) { workflowLaneSaverRef.current = saver; @@ -1202,7 +1214,17 @@ export function SettingsModal({ const [loading, setLoading] = useState(true); // Guards the Save action against double-submit (rapid clicks / Enter) while the // parallel global+project writes are in flight. - const [isSaving, setIsSaving] = useState(false); + const [, setIsSaving] = useState(false); + const [autoSaveStatus, setAutoSaveStatus] = useState<"idle" | "saving" | "saved" | "error">("idle"); + const autoSaveTimerRef = useRef | null>(null); + const [autoSaveReady, setAutoSaveReady] = useState(false); + const autoSaveActivationSnapshotRef = useRef(null); + const persistInFlightRef = useRef(false); + const trailingPersistRef = useRef(false); + const lastPersistSucceededRef = useRef(true); + const persistSettingsRef = useRef<(() => Promise) | null>(null); + const latestAutoSaveStateRef = useRef({ dirty: false, changed: false }); + const requestSectionChangeRef = useRef<((sectionId: SectionId) => void) | null>(null); // Track initial values to detect explicit clears for null-as-delete semantics const [initialValues, setInitialValues] = useState(null); // Track scoped settings for inheritance detection (fetched alongside merged settings) @@ -1339,6 +1361,7 @@ export function SettingsModal({ const [overlapPathPickerIndex, setOverlapPathPickerIndex] = useState(null); const [worktreesDirPickerOpen, setWorktreesDirPickerOpen] = useState(false); const [worktreeCopyFilePickerIndex, setWorktreeCopyFilePickerIndex] = useState(null); + /* FNXC:SettingsReset 2026-07-04-00:20: Reset Settings confirmation dialog state (FN-7506). `resetInFlight` guards both @@ -1447,7 +1470,14 @@ export function SettingsModal({ // Claims the upcoming section change so the scroll-to-top below yields to the // jump; otherwise the row we just scrolled to would be scrolled away from. settingsJumpPendingRef.current = true; - setActiveSection(sectionId); + /* + FNXC:SettingsAutoSave 2026-08-03-00:00: + Search navigation is an operator-initiated section change too. Route it + through the same flush path as sidebar/mobile navigation so a pending edit + to raw global GitLab fields cannot be re-scoped as a project save after the + search changes activeSection. + */ + requestSectionChangeRef.current?.(sectionId as SectionId); setHighlightedSettingKey(key); }, []); @@ -2988,21 +3018,6 @@ export function SettingsModal({ } }, [favoriteModels, favoriteProviders]); - // Modal-only: Escape dismisses the dialog. Embedded view is navigated away via the left sidebar, not Escape. - // FNXC:SettingsReset 2026-07-04-00:30: Skipped while the Reset Settings confirmation dialog is - // open so Escape closes only that dialog (its own listener below), not the whole Settings modal. - useEffect(() => { - if (!escapeEnabled) return; - const handleKey = (e: KeyboardEvent) => { - if (e.key === "Escape" && !resetDialogOpen) onClose(); - }; - document.addEventListener("keydown", handleKey); - return () => document.removeEventListener("keydown", handleKey); - }, [onClose, escapeEnabled, resetDialogOpen]); - - // Modal-only: backdrop click dismisses. Embedded view has no overlay backdrop. - const modalOverlayDismissProps = useOverlayDismiss(onClose); - const overlayDismissProps = overlayDismissEnabled ? modalOverlayDismissProps : {}; /** * Lane status types: @@ -3401,38 +3416,66 @@ export function SettingsModal({ }); }, []); - const handleSave = useCallback(async () => { - if (isSaving) return; - if (prefixError || presetDraft) return; + /* + FNXC:SettingsAutoSave 2026-08-02-12:00: + FN-8395 implements issue #2343 Option 1: form-backed Settings persist through + this debounced single-flight path, never a Save button or dirty-leave prompt. + Each request works from a captured render snapshot and advances only matching + baselines, so an older response cannot erase a newer edit. + */ + const persistSettings = useCallback(async (): Promise => { + if (persistInFlightRef.current) { + trailingPersistRef.current = true; + return false; + } + if (prefixError || presetDraft) { + lastPersistSucceededRef.current = false; + return false; + } - const limits = form.researchSettings?.limits; + const formSnapshot = form; + const scopedSettingsSnapshot = scopedSettings; + const initialValuesSnapshot = initialValues; + const initialScopedValuesSnapshot = initialScopedValues; + const globalMaxConcurrentSnapshot = globalMaxConcurrent; + const activeSectionSnapshot = activeSection; + const globalGitlabSettingsSnapshot = globalGitlabSettings; + const workflowLaneRevisionSnapshot = workflowLaneRevisionRef.current; + const limits = formSnapshot.researchSettings?.limits; if (limits?.maxConcurrentRuns !== undefined && (!Number.isFinite(limits.maxConcurrentRuns) || limits.maxConcurrentRuns < 1)) { setResearchLimitError("Research max concurrent runs must be at least 1."); - return; + lastPersistSucceededRef.current = false; + return false; } if (limits?.maxSourcesPerRun !== undefined && (!Number.isFinite(limits.maxSourcesPerRun) || limits.maxSourcesPerRun < 1)) { setResearchLimitError("Research max sources per run must be at least 1."); - return; + lastPersistSucceededRef.current = false; + return false; } if (limits?.maxDurationMs !== undefined && (!Number.isFinite(limits.maxDurationMs) || limits.maxDurationMs < 1000)) { setResearchLimitError("Research max duration must be at least 1000 ms."); - return; + lastPersistSucceededRef.current = false; + return false; } if (limits?.requestTimeoutMs !== undefined && (!Number.isFinite(limits.requestTimeoutMs) || limits.requestTimeoutMs < 1000)) { setResearchLimitError("Research request timeout must be at least 1000 ms."); - return; + lastPersistSucceededRef.current = false; + return false; } setResearchLimitError(null); - const shortcutValidationError = describeShortcutValidation(form.dashboardKeyboardShortcuts ?? {}); + const shortcutValidationError = describeShortcutValidation(formSnapshot.dashboardKeyboardShortcuts ?? {}); if (shortcutValidationError) { addToast(shortcutValidationError, "error"); - return; + lastPersistSucceededRef.current = false; + return false; } + persistInFlightRef.current = true; setIsSaving(true); + setAutoSaveStatus("saving"); try { - const normalizedWorktreeCopyFiles = normalizeWorktreeCopyFilesForSave(form.worktreeCopyFiles); + const normalizedWorktreeCopyFiles = normalizeWorktreeCopyFilesForSave(formSnapshot.worktreeCopyFiles); /* FNXC:WindowsTerminalStartup 2026-07-04-06:30: Worktrunk status is now only auto-probed once the integration is enabled, so a @@ -3443,43 +3486,43 @@ export function SettingsModal({ that fresh result instead. */ let worktrunkVerifiedForSave = worktrunkInstallVerified; - if (form.worktrunk?.enabled === true && !worktrunkVerifiedForSave) { + if (formSnapshot.worktrunk?.enabled === true && !worktrunkVerifiedForSave) { const freshWorktrunkStatus = await worktrunkInstall.refresh(); worktrunkVerifiedForSave = freshWorktrunkStatus.status === "installed"; } /* FNXC:GitLabEnablement 2026-07-02-00:00: - The global source-control section must edit raw global GitLab settings, not the merged project-effective form. Otherwise a project override can silently overwrite the global GitLab default on a no-op save. + The global source-control section must edit raw global GitLab settings, not the merged project-effective formSnapshot. Otherwise a project override can silently overwrite the global GitLab default on a no-op save. FNXC:SourceControl 2026-07-15-20:30: Section id moved with the controls (was "global-general"). This must name whichever section renders the global GitLab rows: a stale id here would send the merged, project-effective values to the global patch — the exact overwrite the scoped-state indirection exists to prevent. */ - const gitlabFormForSave = activeSection === "source-control-global" && globalGitlabSettings ? globalGitlabSettings : form; + const gitlabFormForSave = activeSectionSnapshot === "source-control-global" && globalGitlabSettingsSnapshot ? globalGitlabSettingsSnapshot : formSnapshot; const payload = { - ...form, - worktreeInitCommand: form.worktreeInitCommand?.trim() || undefined, - worktreesDir: form.worktreesDir?.trim() || undefined, + ...formSnapshot, + worktreeInitCommand: formSnapshot.worktreeInitCommand?.trim() || undefined, + worktreesDir: formSnapshot.worktreesDir?.trim() || undefined, worktrunk: { - enabled: worktrunkVerifiedForSave && form.worktrunk?.enabled === true, - binaryPath: form.worktrunk?.binaryPath?.trim() || undefined, - onFailure: form.worktrunk?.onFailure ?? "fail", + enabled: worktrunkVerifiedForSave && formSnapshot.worktrunk?.enabled === true, + binaryPath: formSnapshot.worktrunk?.binaryPath?.trim() || undefined, + onFailure: formSnapshot.worktrunk?.onFailure ?? "fail", }, - maxAutoMergeRetries: resolveMaxAutoMergeRetriesForSettingsForm(form), - executorToolFailureRetryCount: resolveNonNegativeExecutorToolFailureSetting(form.executorToolFailureRetryCount, 2), - executorToolFailureRetryBackoffMs: resolveNonNegativeExecutorToolFailureSetting(form.executorToolFailureRetryBackoffMs, 2000), - executorToolFailureThreshold: Math.max(1, Math.floor(Number(form.executorToolFailureThreshold ?? 3) || 3)), - executorModelEscalationEnabled: form.executorModelEscalationEnabled === true, - executorEscalationProvider: form.executorEscalationProvider?.trim() || undefined, - executorEscalationModelId: form.executorEscalationModelId?.trim() || undefined, - executorEscalationNodeId: form.executorEscalationNodeId?.trim() || undefined, - taskPrefix: form.taskPrefix?.trim() || undefined, - githubTrackingDefaultRepo: form.githubTrackingDefaultRepo?.trim() || undefined, + maxAutoMergeRetries: resolveMaxAutoMergeRetriesForSettingsForm(formSnapshot), + executorToolFailureRetryCount: resolveNonNegativeExecutorToolFailureSetting(formSnapshot.executorToolFailureRetryCount, 2), + executorToolFailureRetryBackoffMs: resolveNonNegativeExecutorToolFailureSetting(formSnapshot.executorToolFailureRetryBackoffMs, 2000), + executorToolFailureThreshold: Math.max(1, Math.floor(Number(formSnapshot.executorToolFailureThreshold ?? 3) || 3)), + executorModelEscalationEnabled: formSnapshot.executorModelEscalationEnabled === true, + executorEscalationProvider: formSnapshot.executorEscalationProvider?.trim() || undefined, + executorEscalationModelId: formSnapshot.executorEscalationModelId?.trim() || undefined, + executorEscalationNodeId: formSnapshot.executorEscalationNodeId?.trim() || undefined, + taskPrefix: formSnapshot.taskPrefix?.trim() || undefined, + githubTrackingDefaultRepo: formSnapshot.githubTrackingDefaultRepo?.trim() || undefined, /* FNXC:DashboardShortcuts 2026-07-04-00:00: FN-7553 normalizes every declared shortcut action (derived from resolveDashboardKeyboardShortcuts' key set) on save, not just quickChat/terminal, so newly-added actions get the same trim/normalize-before-persist treatment. */ dashboardKeyboardShortcuts: Object.fromEntries( - (Object.entries(resolveDashboardKeyboardShortcuts(form.dashboardKeyboardShortcuts)) as [DashboardShortcutAction, string][]) + (Object.entries(resolveDashboardKeyboardShortcuts(formSnapshot.dashboardKeyboardShortcuts)) as [DashboardShortcutAction, string][]) .map(([action, shortcut]) => [action, normalizeKeyboardShortcut(shortcut).normalized]), ) as DashboardKeyboardShortcutMap, gitlabEnabled: gitlabFormForSave.gitlabEnabled, @@ -3490,23 +3533,23 @@ export function SettingsModal({ reportRoadmapDedupeEnabled: gitlabFormForSave.reportRoadmapDedupeEnabled, reportRoadmapLabel: gitlabFormForSave.reportRoadmapLabel?.trim() || undefined, reportRoadmapRepo: gitlabFormForSave.reportRoadmapRepo?.trim() || undefined, - githubAuthToken: form.githubAuthToken?.trim() || undefined, - prTitlePromptInstructions: form.prTitlePromptInstructions?.trim() || undefined, - prDescriptionPromptInstructions: form.prDescriptionPromptInstructions?.trim() || undefined, + githubAuthToken: formSnapshot.githubAuthToken?.trim() || undefined, + prTitlePromptInstructions: formSnapshot.prTitlePromptInstructions?.trim() || undefined, + prDescriptionPromptInstructions: formSnapshot.prDescriptionPromptInstructions?.trim() || undefined, /* FNXC:MergeSettings 2026-07-04-09:18: Push target text is meaningful only when direct post-merge pushing is enabled. Hiding the input must not keep submitting a stale remote/branch from the form state; clearing it lets project settings fall back to the default origin target when the toggle is disabled. */ - pushRemote: form.pushAfterMerge ? form.pushRemote?.trim() || undefined : undefined, - overlapIgnorePaths: (form.overlapIgnorePaths ?? []).map((path) => path.trim()).filter((path) => path.length > 0), - worktreeCopyFiles: normalizedWorktreeCopyFiles.length > 0 || initialScopedValues?.project?.worktreeCopyFiles !== undefined + pushRemote: formSnapshot.pushAfterMerge ? formSnapshot.pushRemote?.trim() || undefined : undefined, + overlapIgnorePaths: (formSnapshot.overlapIgnorePaths ?? []).map((path) => path.trim()).filter((path) => path.length > 0), + worktreeCopyFiles: normalizedWorktreeCopyFiles.length > 0 || initialScopedValuesSnapshot?.project?.worktreeCopyFiles !== undefined ? normalizedWorktreeCopyFiles : undefined, - experimentalFeatures: normalizeExperimentalFeaturesForSave(form.experimentalFeatures), + experimentalFeatures: normalizeExperimentalFeaturesForSave(formSnapshot.experimentalFeatures), }; // FNXC:SourceControl 2026-07-15-20:30: Both GitLab URL-cache refreshes follow their editing sections ("general"/"global-general" before the move). - if (activeSection === "source-control") { + if (activeSectionSnapshot === "source-control") { resolveGitlabConfig({ project: { gitlabInstanceUrl: payload.gitlabInstanceUrl, @@ -3514,7 +3557,7 @@ export function SettingsModal({ }, }); } - if (activeSection === "source-control-global") { + if (activeSectionSnapshot === "source-control-global") { resolveGitlabConfig({ global: { gitlabInstanceUrl: payload.gitlabInstanceUrl, @@ -3530,12 +3573,12 @@ export function SettingsModal({ // isolation; see settings/save-split.ts. const { globalPatch, projectPatch } = splitSettingsSave({ payload, - initialValues, - initialScopedValues, - activeSection, - scopedMcpValues: scopedSettings ? { - global: resolveScopedMcpSettings("global", scopedSettings), - project: resolveScopedMcpSettings("project", scopedSettings), + initialValues: initialValuesSnapshot, + initialScopedValues: initialScopedValuesSnapshot, + activeSection: activeSectionSnapshot, + scopedMcpValues: scopedSettingsSnapshot ? { + global: resolveScopedMcpSettings("global", scopedSettingsSnapshot), + project: resolveScopedMcpSettings("project", scopedSettingsSnapshot), } : undefined, }); @@ -3546,22 +3589,207 @@ export function SettingsModal({ await Promise.all([ Object.keys(globalPatch).length > 0 ? updateGlobalSettings(globalPatch) : Promise.resolve(), Object.keys(projectPatch).length > 0 ? updateSettings(projectPatch, projectId) : Promise.resolve(), - globalMaxConcurrent !== initialGlobalMaxConcurrentRef.current - ? updateGlobalConcurrency({ globalMaxConcurrent: globalMaxConcurrent ?? 4 }) + globalMaxConcurrentSnapshot !== initialGlobalMaxConcurrentRef.current + ? updateGlobalConcurrency({ globalMaxConcurrent: globalMaxConcurrentSnapshot ?? 4 }) : Promise.resolve(), ]); await workflowLaneSaverRef.current?.(); - addToast(t("settings.general.settingsSaved", "Settings saved"), "success"); - onClose(); + // Only clear workflow-lane dirtiness when no newer lane edit arrived. + if (workflowLaneRevisionRef.current === workflowLaneRevisionSnapshot) { + setWorkflowLanesDirty(false); + } + + // Quiet state feedback avoids a toast for each debounced edit. + setAutoSaveStatus("saved"); + /* + FNXC:SettingsAutoSave 2026-08-02-20:50: + A completed request may describe an older form snapshot. Advance only the + keys that request actually wrote so a response can never bless unrelated, + newer edits as already persisted. + */ + setInitialValues((current) => current ? { ...current, ...globalPatch } : current); + setInitialScopedValues((current) => { + if (!current) return current; + const mergePatch = (base: Record, patch: Record) => Object.fromEntries( + Object.entries({ ...base, ...patch }).map(([key, value]) => [key, value === null ? undefined : value]), + ); + return { + global: mergePatch(current.global as Record, globalPatch as Record) as GlobalSettings, + project: mergePatch(current.project, projectPatch as Record) as Partial, + }; + }); + if (globalMaxConcurrentSnapshot !== initialGlobalMaxConcurrentRef.current) { + initialGlobalMaxConcurrentRef.current = globalMaxConcurrentSnapshot; + } + /* + FNXC:SettingsAutoSave 2026-08-02-21:45: + A successful snapshot becomes the next autosave comparison point. If the + user edited while this request was in flight, the live snapshot differs + and the effect queues exactly one trailing write. + */ + autoSaveActivationSnapshotRef.current = JSON.stringify({ + form: formSnapshot, + scopedSettings: scopedSettingsSnapshot, + globalGitlabSettings: globalGitlabSettingsSnapshot, + globalMaxConcurrent: globalMaxConcurrentSnapshot, + }); + lastPersistSucceededRef.current = true; + return true; } catch (err) { - if (err instanceof WorkflowLaneFlushRejection) return; + lastPersistSucceededRef.current = false; + if (err instanceof WorkflowLaneFlushRejection) return false; + setAutoSaveStatus("error"); addToast(getErrorMessage(err), "error"); + return false; } finally { + persistInFlightRef.current = false; setIsSaving(false); + if (trailingPersistRef.current) { + trailingPersistRef.current = false; + void persistSettingsRef.current?.(); + } } - }, [form, globalGitlabSettings, globalMaxConcurrent, prefixError, presetDraft, initialValues, initialScopedValues, scopedSettings, onClose, addToast, projectId, activeSection, isSaving, t]); + }, [form, globalGitlabSettings, globalMaxConcurrent, prefixError, presetDraft, initialValues, initialScopedValues, scopedSettings, addToast, projectId, activeSection, t]); + + persistSettingsRef.current = persistSettings; + const settingsDirty = useMemo(() => { + const dirtyPayload = activeSection === "source-control-global" && globalGitlabSettings + ? { ...form, ...globalGitlabSettings } + : form; + const { globalPatch, projectPatch } = splitSettingsSave({ + payload: dirtyPayload, + initialValues, + initialScopedValues, + activeSection, + scopedMcpValues: scopedSettings ? { + global: resolveScopedMcpSettings("global", scopedSettings), + project: resolveScopedMcpSettings("project", scopedSettings), + } : undefined, + }); + return Object.keys(globalPatch).length > 0 || Object.keys(projectPatch).length > 0 + || globalMaxConcurrent !== initialGlobalMaxConcurrentRef.current + || workflowLanesDirty; + }, [form, globalGitlabSettings, globalMaxConcurrent, initialScopedValues, initialValues, scopedSettings, activeSection, workflowLanesDirty]); + + const autoSaveSnapshot = useMemo(() => JSON.stringify({ form, scopedSettings, globalGitlabSettings, globalMaxConcurrent, workflowLaneRevision: workflowLaneRevisionRef.current }), [form, globalGitlabSettings, globalMaxConcurrent, scopedSettings, workflowLanesDirty]); + const hasAutoSaveChange = autoSaveActivationSnapshotRef.current !== null + && autoSaveActivationSnapshotRef.current !== autoSaveSnapshot; + latestAutoSaveStateRef.current = { dirty: settingsDirty, changed: hasAutoSaveChange }; + + useEffect(() => { + /* + FNXC:SettingsAutoSave 2026-08-02-21:35: + Some legacy form values are normalized differently from their raw scoped + settings. Snapshot the hydrated form before enabling autosave so opening + Settings cannot write those untouched defaults; later user edits change the + snapshot and are persisted through the normal dirty split. + */ + if (!loading && autoSaveActivationSnapshotRef.current === null) { + autoSaveActivationSnapshotRef.current = autoSaveSnapshot; + setAutoSaveReady(true); + } + }, [autoSaveSnapshot, loading]); + + useEffect(() => { + if (loading || !autoSaveReady || !hasAutoSaveChange || !settingsDirty || prefixError || presetDraft) return; + if (persistInFlightRef.current) { + trailingPersistRef.current = true; + return; + } + if (autoSaveTimerRef.current) clearTimeout(autoSaveTimerRef.current); + autoSaveTimerRef.current = setTimeout(() => { + autoSaveTimerRef.current = null; + void persistSettingsRef.current?.(); + }, 500); + return () => { + if (autoSaveTimerRef.current) clearTimeout(autoSaveTimerRef.current); + }; + }, [loading, autoSaveReady, hasAutoSaveChange, settingsDirty, prefixError, presetDraft, form, scopedSettings, globalGitlabSettings, globalMaxConcurrent, workflowLanesDirty, activeSection]); + + const requestClose = useCallback(async () => { + if (autoSaveTimerRef.current) { + clearTimeout(autoSaveTimerRef.current); + autoSaveTimerRef.current = null; + } + /* + FNXC:SettingsAutoSave 2026-08-02-20:50: + Close is never a discard path. If a request is already running, queue its + latest trailing snapshot and wait for that queue to drain before the modal + unmounts; otherwise flush the current dirty snapshot synchronously. + */ + if (persistInFlightRef.current) { + trailingPersistRef.current = true; + } else if (settingsDirty && hasAutoSaveChange) { + await persistSettingsRef.current?.(); + } + while (persistInFlightRef.current || trailingPersistRef.current) { + await new Promise((resolve) => window.setTimeout(resolve, 0)); + } + // A failed/validation-blocked flush must leave Settings open for correction. + if (!lastPersistSucceededRef.current) return; + onClose(); + }, [onClose, settingsDirty, hasAutoSaveChange]); + + const requestSectionChange = useCallback(async (sectionId: SectionId) => { + if (sectionId === activeSection) return; + if (autoSaveTimerRef.current) { + clearTimeout(autoSaveTimerRef.current); + autoSaveTimerRef.current = null; + } + if ((settingsDirty && hasAutoSaveChange) || persistInFlightRef.current) { + if (persistInFlightRef.current) { + trailingPersistRef.current = true; + while (persistInFlightRef.current || trailingPersistRef.current) { + await new Promise((resolve) => window.setTimeout(resolve, 0)); + } + } else { + const persisted = await persistSettingsRef.current?.(); + if (!persisted) return; + } + // Do not re-scope a failed raw-global edit after navigating away from it. + if (!lastPersistSucceededRef.current) return; + } + setActiveSection(sectionId); + }, [activeSection, settingsDirty, hasAutoSaveChange]); + + requestSectionChangeRef.current = (sectionId) => { void requestSectionChange(sectionId); }; + + useEffect(() => () => { + if (autoSaveTimerRef.current) { + clearTimeout(autoSaveTimerRef.current); + autoSaveTimerRef.current = null; + } + /* + FNXC:SettingsAutoSave 2026-08-03-22:15: + Parent-driven unmount is also a dismissal path. Retain a dirty snapshot's + flush even when a debounce timer is not present at cleanup, rather than + treating the timer itself as the source of durability. + */ + if (!persistInFlightRef.current && latestAutoSaveStateRef.current.dirty && latestAutoSaveStateRef.current.changed) { + // The ref always points at the latest render snapshot, even during unmount. + void persistSettingsRef.current?.(); + } + }, []); + + useEffect(() => { + if (!escapeEnabled) return; + const handleKey = (event: KeyboardEvent) => { + if (event.key === "Escape" && !resetDialogOpen) void requestClose(); + }; + document.addEventListener("keydown", handleKey); + return () => document.removeEventListener("keydown", handleKey); + }, [escapeEnabled, requestClose, resetDialogOpen]); + + const modalOverlayDismissProps = useOverlayDismiss(() => { void requestClose(); }); + /* + FNXC:SettingsAutoSave 2026-08-02-21:45: + Backdrop dismissal remains preference-gated, but every enabled modal path + shares requestClose so its latest dirty snapshot is flushed before unmount. + */ + const overlayDismissProps = !isEmbedded && overlayDismissEnabled ? modalOverlayDismissProps : {}; + /* FNXC:SettingsReset 2026-07-04-00:25: @@ -3946,6 +4174,7 @@ export function SettingsModal({ addToast={addToast} onOpenWorkflowSettings={onOpenWorkflowSettings} registerWorkflowLaneSaver={registerWorkflowLaneSaver} + onWorkflowLanesChange={markWorkflowLanesDirty} models={{ modelLanes: MODEL_LANES, getLaneStatus, @@ -4314,7 +4543,7 @@ export function SettingsModal({ {!isEmbedded && ( - )} @@ -4329,7 +4558,7 @@ export function SettingsModal({ {isEmbedded && viewportMode === "mobile" && ( - )} - - + + )} diff --git a/packages/dashboard/app/components/__tests__/SettingsModal.general.test.tsx b/packages/dashboard/app/components/__tests__/SettingsModal.general.test.tsx index 9256ff75c1..3624bc77d9 100644 --- a/packages/dashboard/app/components/__tests__/SettingsModal.general.test.tsx +++ b/packages/dashboard/app/components/__tests__/SettingsModal.general.test.tsx @@ -1,7 +1,8 @@ import { afterEach, beforeEach, describe, it, expect, vi } from "vitest"; -import { render, screen, fireEvent, waitFor, within, cleanup } from "@testing-library/react"; +import { act, render, screen, fireEvent, waitFor, within, cleanup } from "@testing-library/react"; import path from "path"; import { SettingsModal } from "../SettingsModal"; +import { ModalDismissPreferenceProvider } from "../../hooks/useOverlayDismiss"; import { mockFetchSettings, mockFetchSettingsByScope, @@ -393,7 +394,6 @@ describe("SettingsModal", () => { await settingsModalUser.click(screen.getByRole("checkbox", { name: /Enable MCP servers for this scope/i })); await settingsModalUser.click(screen.getByRole("button", { name: /^General · Global$/ })); - await settingsModalUser.click(screen.getByRole("button", { name: "Save" })); await waitFor(() => { expect(mockUpdateSettings).toHaveBeenCalledWith( @@ -709,7 +709,6 @@ describe("SettingsModal", () => { await settingsModalUser.selectOptions(screen.getByLabelText("AI merge"), "deterministic"); await settingsModalUser.selectOptions(screen.getByLabelText("Integration worktree"), "cwd-main"); - await settingsModalUser.click(screen.getByRole("button", { name: "Save" })); await waitFor(() => { expect(mockUpdateSettings).toHaveBeenCalledTimes(1); @@ -786,7 +785,6 @@ describe("SettingsModal", () => { expect(checkbox).not.toBeChecked(); await settingsModalUser.click(checkbox); - await settingsModalUser.click(screen.getByRole("button", { name: "Save" })); await waitFor(() => { expect(mockUpdateSettings).toHaveBeenCalledWith( @@ -974,7 +972,6 @@ describe("SettingsModal", () => { await waitForSettingsModalReady(); await settingsModalUser.click(screen.getByRole("checkbox", { name: "Dismiss modals by clicking outside" })); - await settingsModalUser.click(screen.getByRole("button", { name: "Save" })); await waitFor(() => { expect(mockUpdateGlobalSettings).toHaveBeenCalled(); @@ -993,7 +990,6 @@ describe("SettingsModal", () => { await waitForSettingsModalReady(); await settingsModalUser.click(screen.getByRole("checkbox", { name: "Skip confirmation dialogs for critical actions" })); - await settingsModalUser.click(screen.getByRole("button", { name: "Save" })); await waitFor(() => { expect(mockUpdateGlobalSettings).toHaveBeenCalled(); @@ -1012,7 +1008,6 @@ describe("SettingsModal", () => { await waitForSettingsModalReady(); await settingsModalUser.click(screen.getByRole("checkbox", { name: "Save tool output in agent logs" })); - await settingsModalUser.click(screen.getByRole("button", { name: "Save" })); await waitFor(() => { expect(mockUpdateGlobalSettings).toHaveBeenCalled(); @@ -1031,7 +1026,6 @@ describe("SettingsModal", () => { await waitForSettingsModalReady(); await settingsModalUser.click(screen.getByRole("checkbox", { name: "Enable proactive task-chat updates" })); - await settingsModalUser.click(screen.getByRole("button", { name: "Save" })); await waitFor(() => { expect(mockUpdateGlobalSettings).toHaveBeenCalled(); @@ -1051,7 +1045,6 @@ describe("SettingsModal", () => { await settingsModalUser.click(screen.getByRole("checkbox", { name: "Save AI thinking for permanent agents" })); await settingsModalUser.click(screen.getByRole("checkbox", { name: "Save AI thinking for ephemeral / task-worker agents" })); - await settingsModalUser.click(screen.getByRole("button", { name: "Save" })); await waitFor(() => { expect(mockUpdateGlobalSettings).toHaveBeenCalled(); @@ -1081,7 +1074,6 @@ describe("SettingsModal", () => { }); await settingsModalUser.selectOptions(screen.getByRole("combobox", { name: "Global default tracking repo" }), "octo/global-default"); - await settingsModalUser.click(screen.getByRole("button", { name: "Save" })); await waitFor(() => { expect(mockUpdateGlobalSettings).toHaveBeenCalled(); @@ -1105,7 +1097,6 @@ describe("SettingsModal", () => { await settingsModalUser.type(screen.getByLabelText("Global GitLab instance URL"), " https://gitlab.company.test/ "); await settingsModalUser.type(screen.getByLabelText("Global GitLab API base URL (optional / advanced)"), " https://gitlab.company.test/api/v4/ "); - await settingsModalUser.click(screen.getByRole("button", { name: "Save" })); await waitFor(() => { expect(mockUpdateGlobalSettings).toHaveBeenCalled(); @@ -1135,8 +1126,6 @@ describe("SettingsModal", () => { expect(enableToggle).not.toBeChecked(); expect(screen.getByLabelText("Global GitLab instance URL")).toBeDisabled(); - await settingsModalUser.click(screen.getByRole("button", { name: "Save" })); - expect(mockUpdateGlobalSettings).not.toHaveBeenCalledWith(expect.objectContaining({ gitlabEnabled: true })); }); @@ -1151,7 +1140,6 @@ describe("SettingsModal", () => { await waitForSettingsModalReady(); await settingsModalUser.click(screen.getByLabelText("Enable GitLab integration")); - await settingsModalUser.click(screen.getByRole("button", { name: "Save" })); await waitFor(() => { expect(mockUpdateGlobalSettings).toHaveBeenCalledWith(expect.objectContaining({ gitlabEnabled: true })); @@ -1190,7 +1178,6 @@ describe("SettingsModal", () => { await settingsModalUser.click(enableToggle); expect(enableToggle).not.toBeChecked(); - await settingsModalUser.click(screen.getByRole("button", { name: "Save" })); await waitFor(() => { expect(mockUpdateGlobalSettings).toHaveBeenCalledWith(expect.objectContaining({ gitlabEnabled: false })); @@ -1219,7 +1206,6 @@ describe("SettingsModal", () => { expect(screen.getByRole("heading", { name: "Agent Provisioning Approvals" })).toBeInTheDocument(); await settingsModalUser.selectOptions(screen.getByLabelText("Approval mode"), "always"); - await settingsModalUser.click(screen.getByRole("button", { name: "Save" })); await waitFor(() => { expect(mockUpdateSettings).toHaveBeenCalled(); @@ -1241,7 +1227,6 @@ describe("SettingsModal", () => { expect(category).toBeEnabled(); expect(screen.getByRole("option", { name: "Ideas" })).toHaveValue("DC_ideas"); await settingsModalUser.selectOptions(category, "DC_ideas"); - await settingsModalUser.click(screen.getByRole("button", { name: "Save" })); await waitFor(() => expect(mockUpdateSettings.mock.calls[0]?.[0]).toMatchObject({ reportDiscussionCategory: "DC_ideas" })); }); @@ -1356,7 +1341,6 @@ describe("SettingsModal", () => { fireEvent.change(screen.getByLabelText("Bug report override"), { target: { value: "draft-review" } }); fireEvent.click(screen.getByLabelText("Deduplicate reports against public roadmap")); fireEvent.change(screen.getByLabelText("Public roadmap label"), { target: { value: "planned" } }); - fireEvent.click(screen.getByRole("button", { name: "Save" })); await waitFor(() => expect(mockUpdateSettings).toHaveBeenCalled()); expect(mockUpdateSettings.mock.calls[0]?.[0]).toEqual(expect.objectContaining({ @@ -1412,7 +1396,6 @@ describe("SettingsModal", () => { expect(ephemeralToggle.checked).toBe(true); await settingsModalUser.click(ephemeralToggle); - await settingsModalUser.click(screen.getByRole("button", { name: "Save" })); await waitFor(() => { expect(mockUpdateSettings).toHaveBeenCalled(); @@ -1507,7 +1490,6 @@ describe("SettingsModal", () => { expect(mailCleanupSelect.value).toBe("0"); await settingsModalUser.selectOptions(mailCleanupSelect, "7"); - await settingsModalUser.click(screen.getByRole("button", { name: "Save" })); await waitFor(() => { expect(mockUpdateSettings).toHaveBeenCalled(); @@ -1519,7 +1501,6 @@ describe("SettingsModal", () => { mockUpdateSettings.mockClear(); await settingsModalUser.selectOptions(mailCleanupSelect, "0"); - await settingsModalUser.click(screen.getByRole("button", { name: "Save" })); await waitFor(() => { expect(mockUpdateSettings).toHaveBeenCalled(); @@ -1546,7 +1527,6 @@ describe("SettingsModal", () => { await settingsModalUser.type(recentInput, "7"); await settingsModalUser.type(fetchLimitInput, "60"); await settingsModalUser.type(summaryMaxInput, "900"); - await settingsModalUser.click(screen.getByRole("button", { name: "Save" })); await waitFor(() => { expect(mockUpdateSettings).toHaveBeenCalled(); @@ -1571,7 +1551,6 @@ describe("SettingsModal", () => { await settingsModalUser.selectOptions(modeSelect, "new-tasks"); await settingsModalUser.type(screen.getByPlaceholderText("owner/repo"), "octo/repo"); - await settingsModalUser.click(screen.getByRole("button", { name: "Save" })); await waitFor(() => { expect(mockUpdateSettings).toHaveBeenCalled(); @@ -1604,7 +1583,6 @@ describe("SettingsModal", () => { await settingsModalUser.type(screen.getByLabelText("GitLab instance URL"), " https://gitlab.example.com/gitlab/ "); await settingsModalUser.type(screen.getByLabelText("GitLab API base URL (optional / advanced)"), " https://api.example.com/v4/ "); - await settingsModalUser.click(screen.getByRole("button", { name: "Save" })); await waitFor(() => { expect(mockUpdateSettings).toHaveBeenCalled(); @@ -1640,7 +1618,6 @@ describe("SettingsModal", () => { await waitForSettingsModalReady(); await settingsModalUser.click(screen.getByLabelText("Enable GitLab integration")); - await settingsModalUser.click(screen.getByRole("button", { name: "Save" })); await waitFor(() => { expect(mockUpdateSettings).toHaveBeenCalled(); @@ -1670,7 +1647,6 @@ describe("SettingsModal", () => { await settingsModalUser.clear(screen.getByLabelText("GitLab instance URL")); await settingsModalUser.clear(screen.getByLabelText("GitLab API base URL (optional / advanced)")); - await settingsModalUser.click(screen.getByRole("button", { name: "Save" })); await waitFor(() => { expect(mockUpdateSettings).toHaveBeenCalled(); @@ -1694,7 +1670,6 @@ describe("SettingsModal", () => { expect(screen.getByText(/does not turn GitHub tracking on for ordinary new tasks/i)).toBeInTheDocument(); await settingsModalUser.click(importLinkToggle); - await settingsModalUser.click(screen.getByRole("button", { name: "Save" })); await waitFor(() => { expect(mockUpdateSettings).toHaveBeenCalled(); @@ -1727,7 +1702,6 @@ describe("SettingsModal", () => { expect(importLinkToggle.checked).toBe(true); await settingsModalUser.click(importLinkToggle); - await settingsModalUser.click(screen.getByRole("button", { name: "Save" })); await waitFor(() => { expect(mockUpdateSettings).toHaveBeenCalled(); @@ -1759,7 +1733,6 @@ describe("SettingsModal", () => { await settingsModalUser.selectOptions(modeSelect, "off"); await settingsModalUser.clear(screen.getByPlaceholderText("owner/repo")); - await settingsModalUser.click(screen.getByRole("button", { name: "Save" })); await waitFor(() => { expect(mockUpdateSettings).toHaveBeenCalled(); @@ -1804,7 +1777,6 @@ describe("SettingsModal", () => { ) as HTMLInputElement; await settingsModalUser.click(dedupToggle); - await settingsModalUser.click(screen.getByRole("button", { name: "Save" })); await waitFor(() => { expect(mockUpdateSettings).toHaveBeenCalled(); @@ -1816,7 +1788,6 @@ describe("SettingsModal", () => { mockUpdateSettings.mockClear(); await settingsModalUser.click(dedupToggle); - await settingsModalUser.click(screen.getByRole("button", { name: "Save" })); await waitFor(() => { expect(mockUpdateSettings).toHaveBeenCalled(); @@ -1862,7 +1833,6 @@ describe("SettingsModal", () => { expect(await within(repoSelect).findByRole("option", { name: "octo/repo" })).toBeInTheDocument(); await settingsModalUser.selectOptions(repoSelect, "octo/repo"); - await settingsModalUser.click(screen.getByRole("button", { name: "Save" })); await waitFor(() => { expect(mockUpdateSettings).toHaveBeenCalled(); @@ -1915,7 +1885,6 @@ describe("SettingsModal", () => { await settingsModalUser.click(screen.getByRole("button", { name: /Appearance/ })); await settingsModalUser.click(screen.getByRole("button", { name: "Largest" })); - await settingsModalUser.click(screen.getByRole("button", { name: "Save" })); await waitFor(() => { expect(mockUpdateGlobalSettings).toHaveBeenCalled(); @@ -1926,6 +1895,125 @@ describe("SettingsModal", () => { }); }); + describe("auto-save", () => { + const changeProjectToggle = () => fireEvent.click(screen.getByLabelText("Show capacity risk banner")); + + it("removes the Save button in both modal and embedded presentations", async () => { + const { unmount } = renderModal({ initialSection: "general" }); + await waitForSettingsModalReady(); + expect(screen.queryByRole("button", { name: /^Save$/i })).not.toBeInTheDocument(); + unmount(); + + renderModal({ initialSection: "general", presentation: "embedded" }); + await waitForSettingsModalReady(); + expect(screen.queryByRole("button", { name: /^Save$/i })).not.toBeInTheDocument(); + }); + + it.each([ + ["footer Close", async (container: HTMLElement) => settingsModalUser.click(screen.getAllByRole("button", { name: "Close" }).at(-1)!)], + ["header close", async (container: HTMLElement) => settingsModalUser.click(container.querySelector(".modal-close") as HTMLButtonElement)], + ["Escape", async () => { fireEvent.keyDown(document, { key: "Escape" }); }], + ["backdrop", async (container: HTMLElement) => { + const overlay = container.querySelector(".settings-modal-overlay") as HTMLElement; + fireEvent.mouseDown(overlay); + fireEvent.mouseUp(overlay); + }], + ])("flushes the latest edit through %s without a leave warning", async (path, dismiss) => { + const onClose = vi.fn(); + const result = path === "backdrop" + ? render() + : renderModal({ initialSection: "general", onClose }); + await waitForSettingsModalReady(); + changeProjectToggle(); + await dismiss(result.container); + + await waitFor(() => expect(mockUpdateSettings).toHaveBeenCalledWith( + expect.objectContaining({ capacityRiskBannerEnabled: true }), + undefined, + )); + expect(onClose).toHaveBeenCalledTimes(1); + expect(screen.queryByRole("dialog", { name: /unsaved/i })).not.toBeInTheDocument(); + }); + + it("flushes the latest edit through the embedded mobile close affordance", async () => { + const onClose = vi.fn(); + renderModal({ initialSection: "general", presentation: "embedded", onClose }); + await waitForSettingsModalReady(); + changeProjectToggle(); + await settingsModalUser.click(screen.getByRole("button", { name: "Close" })); + + await waitFor(() => expect(mockUpdateSettings).toHaveBeenCalledWith( + expect.objectContaining({ capacityRiskBannerEnabled: true }), + undefined, + )); + expect(onClose).toHaveBeenCalledTimes(1); + }); + + it("coalesces rapid edits into one debounced write", async () => { + renderModal({ initialSection: "general" }); + await waitForSettingsModalReady(); + vi.useFakeTimers(); + + const toggle = screen.getByLabelText("Show capacity risk banner"); + fireEvent.click(toggle); + fireEvent.click(toggle); + fireEvent.click(toggle); + await act(async () => { await vi.advanceTimersByTimeAsync(500); }); + expect(mockUpdateSettings).toHaveBeenCalledTimes(1); + expect(mockUpdateSettings).toHaveBeenLastCalledWith(expect.objectContaining({ capacityRiskBannerEnabled: true }), undefined); + vi.useRealTimers(); + }); + + it("persists global concurrency and scoped MCP edits without Save", async () => { + renderModal({ initialSection: "scheduling-global" }); + await waitForSettingsModalReady(); + fireEvent.change(await screen.findByLabelText("Global Max Concurrent"), { target: { value: "7" } }); + await waitFor(() => expect(mockUpdateGlobalConcurrency).toHaveBeenCalledWith({ globalMaxConcurrent: 7 })); + + cleanup(); + renderModal({ initialSection: "mcp" }); + await waitForSettingsModalReady(); + fireEvent.click(await screen.findByLabelText("Enable MCP servers for this scope")); + await waitFor(() => expect(mockUpdateSettings).toHaveBeenLastCalledWith( + expect.objectContaining({ mcpServers: expect.objectContaining({ enabled: true }) }), + undefined, + )); + }); + + it("keeps Settings open after a persist failure and retries on the next edit", async () => { + const addToast = vi.fn(); + mockUpdateSettings.mockRejectedValueOnce(new Error("offline")); + renderModal({ initialSection: "general", addToast }); + await waitForSettingsModalReady(); + + changeProjectToggle(); + await waitFor(() => expect(mockUpdateSettings).toHaveBeenCalledTimes(1)); + expect(addToast).toHaveBeenCalledWith("offline", "error"); + expect(screen.getByRole("dialog")).toBeInTheDocument(); + + changeProjectToggle(); + changeProjectToggle(); + await waitFor(() => expect(mockUpdateSettings).toHaveBeenCalledTimes(2)); + expect(screen.queryByRole("button", { name: /^Save$/i })).not.toBeInTheDocument(); + }); + + it("trails an in-flight snapshot so an older response cannot overwrite the final edit", async () => { + let finishFirstSave: (() => void) | undefined; + mockUpdateSettings.mockImplementationOnce(() => new Promise((resolve) => { finishFirstSave = resolve; })); + renderModal({ initialSection: "general" }); + await waitForSettingsModalReady(); + + changeProjectToggle(); + await waitFor(() => expect(mockUpdateSettings).toHaveBeenCalledTimes(1)); + changeProjectToggle(); + await act(async () => { finishFirstSave?.(); }); + + await waitFor(() => expect(mockUpdateSettings).toHaveBeenCalledTimes(2)); + expect(mockUpdateSettings.mock.calls[0]?.[0]).toEqual(expect.objectContaining({ capacityRiskBannerEnabled: true })); + expect(mockUpdateSettings.mock.calls[1]?.[0]).toEqual(expect.objectContaining({ capacityRiskBannerEnabled: false })); + }); + }); + /* FNXC:SettingsReset 2026-07-04-00:50: FN-7506 Reset Settings coverage: dialog open/close (button, Cancel, overlay, Escape) without diff --git a/packages/dashboard/app/components/__tests__/SettingsModal.keyboardShortcuts.test.tsx b/packages/dashboard/app/components/__tests__/SettingsModal.keyboardShortcuts.test.tsx index 094eea00f7..922f672906 100644 --- a/packages/dashboard/app/components/__tests__/SettingsModal.keyboardShortcuts.test.tsx +++ b/packages/dashboard/app/components/__tests__/SettingsModal.keyboardShortcuts.test.tsx @@ -117,7 +117,7 @@ describe("SettingsModal Keyboard Shortcuts section", () => { const openFilesInput = await screen.findByRole("textbox", { name: "Open Files" }); await user.clear(openFilesInput); await user.type(openFilesInput, "alt+e"); - await user.click(screen.getByRole("button", { name: "Save" })); + await waitFor(() => expect(mockUpdateGlobalSettings).toHaveBeenCalled()); const globalPayload = mockUpdateGlobalSettings.mock.calls[0]?.[0] as Record; @@ -138,8 +138,7 @@ describe("SettingsModal Keyboard Shortcuts section", () => { await user.type(openFilesInput, "Ctrl+K"); expect(screen.getByRole("alert")).toHaveTextContent("both use Ctrl+K"); - await user.click(screen.getByRole("button", { name: "Save" })); - expect(addToast).toHaveBeenCalledWith(expect.stringContaining("both use Ctrl+K"), "error"); + await waitFor(() => expect(addToast).toHaveBeenCalledWith(expect.stringContaining("both use Ctrl+K"), "error")); expect(mockUpdateGlobalSettings).not.toHaveBeenCalled(); }); @@ -149,7 +148,7 @@ describe("SettingsModal Keyboard Shortcuts section", () => { const newTaskInput = await screen.findByRole("textbox", { name: "New Task" }); await user.clear(newTaskInput); - await user.click(screen.getByRole("button", { name: "Save" })); + await waitFor(() => expect(mockUpdateGlobalSettings).toHaveBeenCalled()); const globalPayload = mockUpdateGlobalSettings.mock.calls[0]?.[0] as Record; diff --git a/packages/dashboard/app/components/__tests__/SettingsModal.models-auth.test.tsx b/packages/dashboard/app/components/__tests__/SettingsModal.models-auth.test.tsx index ba8fe5d1e3..911051ad9f 100644 --- a/packages/dashboard/app/components/__tests__/SettingsModal.models-auth.test.tsx +++ b/packages/dashboard/app/components/__tests__/SettingsModal.models-auth.test.tsx @@ -267,7 +267,6 @@ describe("SettingsModal", () => { await settingsModalUser.selectOptions(screen.getByLabelText("OpenRouter routing sort"), "latency"); await settingsModalUser.click(screen.getByLabelText("Require parameters")); - await settingsModalUser.click(screen.getByRole("button", { name: "Save" })); await waitFor(() => { expect(mockUpdateGlobalSettings).toHaveBeenCalled(); @@ -350,7 +349,6 @@ describe("SettingsModal", () => { await settingsModalUser.click(screen.getByRole("button", { name: "Models · Project" })); await settingsModalUser.click(screen.getByLabelText("Project Default Model")); await settingsModalUser.click(screen.getByText("GPT-4o")); - await settingsModalUser.click(screen.getByText("Save")); await waitFor(() => { expect(mockUpdateSettings).toHaveBeenCalledTimes(1); @@ -472,7 +470,7 @@ describe("SettingsModal", () => { ["Plan/Triage Model", { planningProvider: "openai", planningModelId: "gpt-4o" }], ["Executor Model", { executionProvider: "openai", executionModelId: "gpt-4o" }], ["Reviewer Model", { validatorProvider: "openai", validatorModelId: "gpt-4o" }], - ])("persists %s edits through the primary Settings Save", async (laneLabel, expectedPatch) => { + ])("auto-saves %s edits without closing Settings", async (laneLabel, expectedPatch) => { mockUpdateWorkflowSettingValues.mockResolvedValue({ stored: expectedPatch, effective: expectedPatch, @@ -483,7 +481,6 @@ describe("SettingsModal", () => { await settingsModalUser.click(screen.getByLabelText(laneLabel)); await settingsModalUser.click(await screen.findByText("GPT-4o")); - await settingsModalUser.click(screen.getByRole("button", { name: "Save" })); await waitFor(() => { expect(mockUpdateWorkflowSettingValues).toHaveBeenCalledWith( @@ -492,7 +489,21 @@ describe("SettingsModal", () => { "proj-1", ); }); - expect(onClose).toHaveBeenCalled(); + expect(onClose).not.toHaveBeenCalled(); + }); + + it("flushes a pending workflow lane edit when Settings closes", async () => { + const expectedPatch = { planningProvider: "openai", planningModelId: "gpt-4o" }; + const onClose = vi.fn(); + mockUpdateWorkflowSettingValues.mockResolvedValue({ stored: expectedPatch, effective: expectedPatch, orphaned: [] }); + await setupWorkflowModelLaneTest({ renderProps: { onClose } }); + + await settingsModalUser.click(screen.getByLabelText("Plan/Triage Model")); + await settingsModalUser.click(await screen.findByText("GPT-4o")); + await settingsModalUser.click(screen.getAllByRole("button", { name: "Close" }).at(-1)!); + + await waitFor(() => expect(mockUpdateWorkflowSettingValues).toHaveBeenCalledWith("workflow-custom", expectedPatch, "proj-1")); + expect(onClose).toHaveBeenCalledTimes(1); }); it("renders saved workflow model lane values as project overrides after reload", async () => { @@ -506,7 +517,6 @@ describe("SettingsModal", () => { await settingsModalUser.click(screen.getByLabelText("Plan/Triage Model")); await settingsModalUser.click(await screen.findByText("GPT-4o")); - await settingsModalUser.click(screen.getByRole("button", { name: "Save" })); await waitFor(() => { expect(mockUpdateWorkflowSettingValues).toHaveBeenCalledWith("workflow-custom", expectedPatch, "proj-1"); }); @@ -543,7 +553,7 @@ describe("SettingsModal", () => { expect(screen.queryByText("Reviewer Fallback Model")).not.toBeInTheDocument(); }); - it("persists fallback workflow model lane edits through the primary Settings Save", async () => { + it("auto-saves fallback workflow model lane edits", async () => { const expectedPatch = { planningFallbackProvider: "openai", planningFallbackModelId: "gpt-4o" }; mockUpdateWorkflowSettingValues.mockResolvedValue({ stored: expectedPatch, @@ -555,7 +565,6 @@ describe("SettingsModal", () => { await settingsModalUser.click(screen.getByLabelText("Planning Fallback Model")); await settingsModalUser.click(await screen.findByText("GPT-4o")); - await settingsModalUser.click(screen.getByRole("button", { name: "Save" })); await waitFor(() => { expect(mockUpdateWorkflowSettingValues).toHaveBeenCalledWith( @@ -564,10 +573,10 @@ describe("SettingsModal", () => { "proj-1", ); }); - expect(onClose).toHaveBeenCalled(); + expect(onClose).not.toHaveBeenCalled(); }); - it("resets fallback workflow model lanes by sending null patches from the primary Settings Save", async () => { + it("auto-saves fallback workflow model lane resets as null patches", async () => { await setupWorkflowModelLaneTest({ stored: { validatorFallbackProvider: "anthropic", validatorFallbackModelId: "claude-sonnet-4-5" }, effective: { validatorFallbackProvider: "anthropic", validatorFallbackModelId: "claude-sonnet-4-5" }, @@ -576,7 +585,6 @@ describe("SettingsModal", () => { const lane = screen.getByTestId("workflow-model-lane-validator-fallback"); expect(within(lane).getByText("Override (Project)")).toBeInTheDocument(); await settingsModalUser.click(within(lane).getByRole("button", { name: "Reset" })); - await settingsModalUser.click(screen.getByRole("button", { name: "Save" })); await waitFor(() => { expect(mockUpdateWorkflowSettingValues).toHaveBeenCalledWith( @@ -599,15 +607,14 @@ describe("SettingsModal", () => { const onClose = vi.fn(); await setupWorkflowModelLaneTest({ renderProps: { onClose } }); - await settingsModalUser.click(screen.getByRole("button", { name: "Save" })); await waitFor(() => { - expect(onClose).toHaveBeenCalled(); + expect(onClose).not.toHaveBeenCalled(); }); expect(mockUpdateWorkflowSettingValues).not.toHaveBeenCalled(); }); - it("preserves pending workflow lane edits when Project Models unmounts before primary Save", async () => { + it("flushes pending workflow lane edits when Project Models unmounts", async () => { const expectedPatch = { planningProvider: "openai", planningModelId: "gpt-4o" }; mockUpdateWorkflowSettingValues.mockResolvedValue({ stored: expectedPatch, @@ -625,7 +632,6 @@ describe("SettingsModal", () => { await waitFor(() => { expect(screen.queryByTestId("workflow-model-lane-planning")).not.toBeInTheDocument(); }); - await settingsModalUser.click(screen.getByRole("button", { name: "Save" })); await waitFor(() => { expect(mockUpdateWorkflowSettingValues).toHaveBeenCalledWith( @@ -634,10 +640,10 @@ describe("SettingsModal", () => { "proj-1", ); }); - expect(onClose).toHaveBeenCalled(); + expect(onClose).not.toHaveBeenCalled(); }); - it("resets workflow model lanes by sending null patches from the primary Settings Save", async () => { + it("auto-saves workflow model lane resets as null patches", async () => { await setupWorkflowModelLaneTest({ stored: { executionProvider: "anthropic", executionModelId: "claude-sonnet-4-5" }, effective: { executionProvider: "anthropic", executionModelId: "claude-sonnet-4-5" }, @@ -645,7 +651,6 @@ describe("SettingsModal", () => { const lane = screen.getByTestId("workflow-model-lane-execution"); await settingsModalUser.click(within(lane).getByRole("button", { name: "Reset" })); - await settingsModalUser.click(screen.getByRole("button", { name: "Save" })); await waitFor(() => { expect(mockUpdateWorkflowSettingValues).toHaveBeenCalledWith( @@ -673,7 +678,6 @@ describe("SettingsModal", () => { await settingsModalUser.click(screen.getByLabelText("Plan/Triage Model")); await settingsModalUser.click(await screen.findByText("GPT-4o")); - await settingsModalUser.click(screen.getByRole("button", { name: "Save" })); await waitFor(() => { expect(mockUpdateWorkflowSettingValues).toHaveBeenCalledWith( @@ -696,7 +700,6 @@ describe("SettingsModal", () => { await settingsModalUser.click(screen.getByLabelText("Plan/Triage Model")); await settingsModalUser.click(await screen.findByText("GPT-4o")); - await settingsModalUser.click(screen.getByRole("button", { name: "Save" })); await waitFor(() => { expect(screen.getByTestId("workflow-model-lane-error-planning")).toHaveTextContent("planningProvider is not declared"); @@ -728,7 +731,6 @@ describe("SettingsModal", () => { expect(mockFetchWorkflowSettingValues).not.toHaveBeenCalled(); expect(screen.queryByTestId("save-workflow-model-lanes")).not.toBeInTheDocument(); - await settingsModalUser.click(screen.getByRole("button", { name: "Save" })); expect(mockUpdateWorkflowSettingValues).not.toHaveBeenCalled(); }); }); @@ -1691,7 +1693,7 @@ describe("SettingsModal", () => { await waitForSettingsModalReady(); expect(screen.getByRole("heading", { name: "Authentication" })).toBeInTheDocument(); - expect(screen.getByRole("button", { name: "Save" })).toBeInTheDocument(); + expect(screen.queryByRole("button", { name: "Save" })).not.toBeInTheDocument(); expect(await screen.findByTestId("droid-cli-provider-card")).toBeInTheDocument(); expect(screen.getAllByTestId("droid-cli-provider-card")).toHaveLength(1); }); diff --git a/packages/dashboard/app/components/__tests__/SettingsModal.remote-notifications.test.tsx b/packages/dashboard/app/components/__tests__/SettingsModal.remote-notifications.test.tsx index 88f643409f..3a1b8dcdb9 100644 --- a/packages/dashboard/app/components/__tests__/SettingsModal.remote-notifications.test.tsx +++ b/packages/dashboard/app/components/__tests__/SettingsModal.remote-notifications.test.tsx @@ -357,7 +357,7 @@ describe("SettingsModal", () => { await settingsModalUser.click(screen.getByLabelText("Accept routes")); await openAdvancedSettings(); await settingsModalUser.click(screen.getByLabelText("Remember last running state")); - await settingsModalUser.click(screen.getByRole("button", { name: "Save" })); + await waitFor(() => { expect(mockUpdateGlobalSettings).toHaveBeenCalledWith( @@ -388,7 +388,7 @@ describe("SettingsModal", () => { await settingsModalUser.click(screen.getByLabelText("Accept routes")); await openAdvancedSettings(); await settingsModalUser.click(screen.getByLabelText("Remember last running state")); - await settingsModalUser.click(screen.getByRole("button", { name: "Save" })); + await waitFor(() => { expect(mockUpdateGlobalSettings).toHaveBeenCalledWith( @@ -462,7 +462,7 @@ describe("SettingsModal", () => { await settingsModalUser.click(screen.getByLabelText("Accept routes")); await openAdvancedSettings(); await settingsModalUser.click(screen.getByLabelText("Remember last running state")); - await settingsModalUser.click(screen.getByRole("button", { name: "Save" })); + await waitFor(() => { expect(mockUpdateGlobalSettings).toHaveBeenCalledWith( @@ -765,7 +765,7 @@ describe("SettingsModal", () => { expect(delayInput).toBeDisabled(); await settingsModalUser.selectOptions(modeSelect, "sticky-only"); fireEvent.change(delayInput, { target: { value: "45000" } }); - await settingsModalUser.click(screen.getByRole("button", { name: "Save" })); + await waitFor(() => { expect(mockUpdateGlobalSettings).toHaveBeenCalledWith( @@ -780,7 +780,7 @@ describe("SettingsModal", () => { it("persists terminal-only selection on save", async () => { const modeSelect = screen.getByLabelText("Failure notification mode") as HTMLSelectElement; await settingsModalUser.selectOptions(modeSelect, "terminal-only"); - await settingsModalUser.click(screen.getByRole("button", { name: "Save" })); + await waitFor(() => { expect(mockUpdateGlobalSettings).toHaveBeenCalledWith( @@ -922,7 +922,7 @@ describe("SettingsModal", () => { }); }); - it("sends unsaved ntfy form config before saving", async () => { + it("sends current ntfy form config to tests while auto-saving", async () => { mockFetchSettings.mockResolvedValueOnce({ ...defaultSettings, ntfyEnabled: false, ntfyTopic: undefined }); await renderModalSection("notifications", "Notifications"); @@ -945,8 +945,12 @@ describe("SettingsModal", () => { undefined, ); }); - expect(mockUpdateSettings).not.toHaveBeenCalled(); - expect(mockUpdateGlobalSettings).not.toHaveBeenCalled(); + await waitFor(() => expect(mockUpdateGlobalSettings).toHaveBeenCalledWith(expect.objectContaining({ + ntfyEnabled: true, + ntfyTopic: "fresh-topic", + ntfyBaseUrl: "https://ntfy.override.example//", + ntfyAccessToken: "override-token", + }))); }); it("keeps ntfy test disabled until the current form has a valid topic", async () => { @@ -977,7 +981,7 @@ describe("SettingsModal", () => { await settingsModalUser.click(screen.getByText("Advanced", { selector: "summary" })); const tokenInput = screen.getByLabelText("Access token (optional)"); await settingsModalUser.clear(tokenInput); - await settingsModalUser.click(screen.getByRole("button", { name: "Save" })); + await waitFor(() => { expect(mockUpdateGlobalSettings).toHaveBeenCalledWith( @@ -1125,7 +1129,7 @@ describe("SettingsModal", () => { fireEvent.change(screen.getByLabelText("Evaluator Model"), { target: { value: "gpt-5" } }); await settingsModalUser.selectOptions(screen.getByLabelText("Follow-up Policy"), "auto-create"); fireEvent.change(screen.getByLabelText("Retention (days)"), { target: { value: "14" } }); - await settingsModalUser.click(screen.getByText("Save")); + await waitFor(() => { expect(mockUpdateSettings).toHaveBeenCalledWith( @@ -1164,7 +1168,7 @@ describe("SettingsModal", () => { await settingsModalUser.clear(screen.getByLabelText("Evaluator Provider")); await settingsModalUser.clear(screen.getByLabelText("Evaluator Model")); - await settingsModalUser.click(screen.getByText("Save")); + await waitFor(() => { expect(mockUpdateSettings).toHaveBeenCalledWith( @@ -1195,7 +1199,7 @@ describe("SettingsModal", () => { fireEvent.change(screen.getByLabelText("Memory Retention Count"), { target: { value: "21" } }); fireEvent.change(screen.getByLabelText("Memory Backup Directory"), { target: { value: ".fusion/backups/custom-memory" } }); await settingsModalUser.selectOptions(screen.getByLabelText("Memory Backup Scope"), "agents"); - await settingsModalUser.click(screen.getByText("Save")); + await waitFor(() => { expect(mockUpdateSettings).toHaveBeenCalledWith( @@ -1261,7 +1265,7 @@ describe("SettingsModal", () => { fireEvent.change(screen.getByLabelText("Max Synthesis Rounds"), { target: { value: "3" } }); await settingsModalUser.click(screen.getByRole("checkbox", { name: /^GitHub$/i })); await settingsModalUser.click(screen.getByRole("checkbox", { name: /^Local Docs$/i })); - await settingsModalUser.click(screen.getByText("Save")); + await waitFor(() => { expect(mockUpdateGlobalSettings).toHaveBeenCalledWith( @@ -1321,7 +1325,7 @@ describe("SettingsModal", () => { expect(webSearch).toBeChecked(); await settingsModalUser.click(screen.getByRole("checkbox", { name: "Page Fetch" })); - await settingsModalUser.click(screen.getByText("Save")); + await waitFor(() => { expect(mockUpdateSettings).toHaveBeenCalledWith( @@ -1346,7 +1350,7 @@ describe("SettingsModal", () => { await settingsModalUser.click(screen.getByLabelText("Enable research in this project")); const maxConcurrent = await screen.findByLabelText("Max Concurrent Runs"); fireEvent.change(maxConcurrent, { target: { value: "4" } }); - await settingsModalUser.click(screen.getByText("Save")); + await waitFor(() => { expect(mockUpdateSettings).toHaveBeenCalledWith( @@ -1371,7 +1375,7 @@ describe("SettingsModal", () => { const maxConcurrent = await screen.findByLabelText("Max Concurrent Runs"); fireEvent.change(maxConcurrent, { target: { value: "0" } }); - await settingsModalUser.click(screen.getByText("Save")); + expect(await screen.findByText("Research max concurrent runs must be at least 1.")).toBeInTheDocument(); }); @@ -1514,7 +1518,7 @@ describe("SettingsModal", () => { expect(screen.getByRole("checkbox", { name: "LLM Synthesis" }).closest(".settings-research-source-grid")).toBe(sourceGrid); fireEvent.change(maxConcurrent, { target: { value: "0" } }); - await settingsModalUser.click(screen.getByText("Save")); + expect(await screen.findByText("Research max concurrent runs must be at least 1.")).toBeInTheDocument(); }); diff --git a/packages/dashboard/app/components/__tests__/SettingsModal.scheduling-merge.test.tsx b/packages/dashboard/app/components/__tests__/SettingsModal.scheduling-merge.test.tsx index f784c0ecab..04a8cb83ec 100644 --- a/packages/dashboard/app/components/__tests__/SettingsModal.scheduling-merge.test.tsx +++ b/packages/dashboard/app/components/__tests__/SettingsModal.scheduling-merge.test.tsx @@ -243,7 +243,7 @@ describe("SettingsModal", () => { await settingsModalUser.click(screen.getByLabelText(/ignore hidden dot paths in overlap checks/i)); await settingsModalUser.type(screen.getByPlaceholderText("docs/"), "generated/*"); - await settingsModalUser.click(screen.getByText("Save")); + await waitFor(() => { expect(mockUpdateSettings).toHaveBeenCalledTimes(1); @@ -267,7 +267,7 @@ describe("SettingsModal", () => { fireEvent.click(screen.getByRole("button", { name: "Scheduling · Project" })); await settingsModalUser.click(screen.getByLabelText(/ignore hidden dot paths in overlap checks/i)); - await settingsModalUser.click(screen.getByText("Save")); + await waitFor(() => { expect(mockUpdateSettings).toHaveBeenCalledTimes(1); @@ -317,7 +317,7 @@ describe("SettingsModal", () => { const inputs = screen.getAllByPlaceholderText("docs/"); await settingsModalUser.type(inputs[1], "generated/*"); - await settingsModalUser.click(screen.getByText("Save")); + await waitFor(() => { expect(mockUpdateSettings).toHaveBeenCalledTimes(1); @@ -342,7 +342,7 @@ describe("SettingsModal", () => { expect(select.value).toBe("lite"); await settingsModalUser.selectOptions(select, "off"); - await settingsModalUser.click(screen.getByText("Save")); + await waitFor(() => { expect(mockUpdateSettings).toHaveBeenCalledTimes(1); @@ -384,7 +384,7 @@ describe("SettingsModal", () => { const toggle = screen.getByLabelText("Let engineer agents auto-claim backlog tasks") as HTMLInputElement; expect(toggle.checked).toBe(false); await settingsModalUser.click(toggle); - await settingsModalUser.click(screen.getByText("Save")); + await waitFor(() => { expect(mockUpdateSettings).toHaveBeenCalledTimes(1); @@ -408,7 +408,7 @@ describe("SettingsModal", () => { const toggle = screen.getByLabelText("Let engineer agents auto-claim backlog tasks") as HTMLInputElement; expect(toggle.checked).toBe(true); await settingsModalUser.click(toggle); - await settingsModalUser.click(screen.getByText("Save")); + await waitFor(() => { expect(mockUpdateSettings).toHaveBeenCalledTimes(1); @@ -491,7 +491,7 @@ describe("SettingsModal", () => { const input = screen.getByLabelText("Worktrees Directory") as HTMLInputElement; fireEvent.change(input, { target: { value: "~/.fn-worktrees/{repo}" } }); - await settingsModalUser.click(screen.getByRole("button", { name: "Save" })); + await waitFor(() => expect(mockUpdateSettings).toHaveBeenCalled()); const payload = mockUpdateSettings.mock.calls[0][0] as Record; @@ -522,7 +522,7 @@ describe("SettingsModal", () => { await settingsModalUser.click(screen.getByRole("button", { name: "Add file" })); const updatedInputs = screen.getAllByLabelText("File to copy into new worktrees") as HTMLInputElement[]; await settingsModalUser.type(updatedInputs[2], " README.md "); - await settingsModalUser.click(screen.getByRole("button", { name: "Save" })); + await waitFor(() => expect(mockUpdateSettings).toHaveBeenCalled()); const payload = mockUpdateSettings.mock.calls[0][0] as Record; @@ -543,7 +543,7 @@ describe("SettingsModal", () => { await waitForSettingsModalReady(); await settingsModalUser.click(screen.getByRole("button", { name: "Remove copied worktree file" })); - await settingsModalUser.click(screen.getByRole("button", { name: "Save" })); + await waitFor(() => expect(mockUpdateSettings).toHaveBeenCalled()); const payload = mockUpdateSettings.mock.calls[0][0] as Record; @@ -810,7 +810,8 @@ describe("SettingsModal", () => { renderModal({ initialSection: "worktrees" }); await waitForSettingsModalReady(); - await settingsModalUser.click(screen.getByRole("button", { name: "Save" })); + // FNXC:SettingsAutoSave 2026-06-22-21:52: The removed footer Save action means this persistence assertion needs a real edit; changing the path exercises the same clamp. + fireEvent.change(screen.getByLabelText("Worktrunk binary path"), { target: { value: "/missing/worktrunk" } }); await waitFor(() => expect(mockUpdateSettings).toHaveBeenCalled()); const payload = mockUpdateSettings.mock.calls[0][0] as { @@ -841,7 +842,7 @@ describe("SettingsModal", () => { await settingsModalUser.selectOptions(onFailureSelect, onFailure); } - await settingsModalUser.click(screen.getByRole("button", { name: "Save" })); + await waitFor(() => expect(mockUpdateSettings).toHaveBeenCalled()); const payload = mockUpdateSettings.mock.calls[0][0] as { @@ -1268,7 +1269,7 @@ describe("SettingsModal", () => { await settingsModalUser.clear(pushRemoteInput); await settingsModalUser.type(pushRemoteInput, " upstream main "); - await settingsModalUser.click(screen.getByText("Save")); + await waitFor(() => { expect(mockUpdateSettings).toHaveBeenCalledTimes(1); @@ -1321,7 +1322,7 @@ describe("SettingsModal", () => { expect(screen.queryByLabelText("Push Remote")).not.toBeInTheDocument(); expect(screen.queryByText("Git remote to push to")).not.toBeInTheDocument(); - await settingsModalUser.click(screen.getByText("Save")); + await waitFor(() => { expect(mockUpdateSettings).toHaveBeenCalledTimes(1); @@ -1337,7 +1338,7 @@ describe("SettingsModal", () => { expect(select).toHaveValue("workflow"); await settingsModalUser.selectOptions(select, "require-all"); - await settingsModalUser.click(screen.getByText("Save")); + await waitFor(() => { expect(mockUpdateSettings).toHaveBeenCalledTimes(1); @@ -1360,18 +1361,11 @@ describe("SettingsModal", () => { expect(screen.queryByLabelText("Verification auto-fix retries")).not.toBeInTheDocument(); }); - it("never sends verificationFixRetries through the save payload", async () => { + it("does not persist anything from a clean merge section", async () => { renderModal({ initialSection: "merge" }); await waitForSettingsModalReady(); - await settingsModalUser.click(screen.getByRole("button", { name: "Save" })); - - await waitFor(() => { - expect(mockUpdateSettings).toHaveBeenCalled(); - }); - - const payload = mockUpdateSettings.mock.calls[0][0] as Record; - expect(payload).not.toHaveProperty("verificationFixRetries"); + expect(mockUpdateSettings).not.toHaveBeenCalled(); }); it("opens workflow settings from the redirect stub", async () => { @@ -1415,7 +1409,7 @@ describe("SettingsModal", () => { await settingsModalUser.selectOptions(authModeSelect, "token"); await settingsModalUser.type(screen.getByLabelText("GitHub personal access token"), "ghp_test_token"); - await settingsModalUser.click(screen.getByRole("button", { name: "Save" })); + await waitFor(() => { expect(mockUpdateSettings).toHaveBeenCalled(); @@ -1456,7 +1450,7 @@ describe("SettingsModal", () => { await settingsModalUser.selectOptions(screen.getByLabelText("GitLab token type"), tokenType); await settingsModalUser.type(screen.getByLabelText("GitLab access token"), " glpat_test_token "); - await settingsModalUser.click(screen.getByRole("button", { name: "Save" })); + await waitFor(() => { expect(mockUpdateSettings).toHaveBeenCalled(); @@ -1484,7 +1478,7 @@ describe("SettingsModal", () => { await settingsModalUser.clear(screen.getByLabelText("GitLab access token")); await settingsModalUser.selectOptions(screen.getByLabelText("GitLab token type"), "project"); - await settingsModalUser.click(screen.getByRole("button", { name: "Save" })); + await waitFor(() => { expect(mockUpdateSettings).toHaveBeenCalled(); @@ -1740,7 +1734,7 @@ describe("SettingsModal", () => { await settingsModalUser.click(devServerToggle); expect(devServerToggle).not.toBeChecked(); - await settingsModalUser.click(screen.getByText("Save")); + await waitFor(() => { expect(mockUpdateGlobalSettings).toHaveBeenCalledTimes(1); @@ -1761,7 +1755,7 @@ describe("SettingsModal", () => { await openExperimentalFeaturesSection(); await settingsModalUser.click(screen.getByLabelText("my-feature")); - await settingsModalUser.click(screen.getByText("Save")); + await waitFor(() => { expect(mockUpdateGlobalSettings).toHaveBeenCalledTimes(1); @@ -1795,7 +1789,7 @@ describe("SettingsModal", () => { await settingsModalUser.click(screen.getByLabelText("my-feature")); - await settingsModalUser.click(screen.getByText("Save")); + await waitFor(() => { expect(mockUpdateGlobalSettings).toHaveBeenCalledTimes(1); @@ -1843,7 +1837,7 @@ describe("SettingsModal", () => { await settingsModalUser.click(leftSidebarToggle); expect(leftSidebarToggle).not.toBeChecked(); - await settingsModalUser.click(screen.getByText("Save")); + await waitFor(() => { expect(mockUpdateGlobalSettings).toHaveBeenCalledTimes(1); @@ -1928,7 +1922,7 @@ describe("SettingsModal", () => { await settingsModalUser.click(checkbox); // Save - await settingsModalUser.click(screen.getByText("Save")); + await waitFor(() => { expect(mockUpdateGlobalSettings).toHaveBeenCalledTimes(1); @@ -1982,7 +1976,7 @@ describe("SettingsModal", () => { await settingsModalUser.click(checkboxB); // Save - await settingsModalUser.click(screen.getByText("Save")); + await waitFor(() => { expect(mockUpdateGlobalSettings).toHaveBeenCalledTimes(1); diff --git a/packages/dashboard/app/components/__tests__/SettingsModal.test-harness.tsx b/packages/dashboard/app/components/__tests__/SettingsModal.test-harness.tsx index 09a4ca6f8d..89bf1ad52c 100644 --- a/packages/dashboard/app/components/__tests__/SettingsModal.test-harness.tsx +++ b/packages/dashboard/app/components/__tests__/SettingsModal.test-harness.tsx @@ -203,19 +203,20 @@ export async function expectSettingPersists({ section, label, kind, value, scope fireEvent.change(control, { target: { value: String(value) } }); } - fireEvent.click(screen.getByRole("button", { name: /^Save$/i })); + fireEvent.click(screen.getAllByRole("button", { name: "Close" })[0]); if (scope === "global") { await waitFor(() => expect(mockUpdateGlobalSettings).toHaveBeenCalled()); - expect(mockUpdateGlobalSettings.mock.calls[0]?.[0]).toEqual( + expect(mockUpdateGlobalSettings).toHaveBeenCalledWith( expect.objectContaining({ [expectedKey]: value }), ); return; } await waitFor(() => expect(mockUpdateSettings).toHaveBeenCalled()); - expect(mockUpdateSettings.mock.calls[0]?.[0]).toEqual( + expect(mockUpdateSettings).toHaveBeenCalledWith( expect.objectContaining({ [expectedKey]: value }), + undefined, ); } @@ -225,7 +226,7 @@ export async function assertProjectModelSavePayload(provider: string, modelId: s fireEvent.change(screen.getByLabelText("Default Provider"), { target: { value: provider } }); fireEvent.change(screen.getByLabelText("Default Model"), { target: { value: modelId } }); - fireEvent.click(screen.getByRole("button", { name: /^Save$/i })); + fireEvent.click(screen.getAllByRole("button", { name: "Close" })[0]); await waitFor(() => { expect(mockUpdateSettings).toHaveBeenCalledWith( diff --git a/packages/dashboard/app/components/__tests__/SettingsModalNodeRouting.test.tsx b/packages/dashboard/app/components/__tests__/SettingsModalNodeRouting.test.tsx index 48dc0c9020..a4cbb55828 100644 --- a/packages/dashboard/app/components/__tests__/SettingsModalNodeRouting.test.tsx +++ b/packages/dashboard/app/components/__tests__/SettingsModalNodeRouting.test.tsx @@ -230,8 +230,6 @@ describe("SettingsModal Node Routing section", () => { fireEvent.change(screen.getByLabelText("Default Execution Node"), { target: { value: "node-remote-1" } }); fireEvent.change(screen.getByLabelText("Unavailable Node Policy"), { target: { value: "fallback-local" } }); - fireEvent.click(screen.getByRole("button", { name: "Save" })); - await waitFor(() => expect(mockUpdateSettings).toHaveBeenCalledTimes(1)); const payload = mockUpdateSettings.mock.calls[0][0]; expect(payload).toMatchObject({ diff --git a/packages/dashboard/app/components/settings/sections/ProjectModelsSection.tsx b/packages/dashboard/app/components/settings/sections/ProjectModelsSection.tsx index 15993f765a..14c1ef6405 100644 --- a/packages/dashboard/app/components/settings/sections/ProjectModelsSection.tsx +++ b/packages/dashboard/app/components/settings/sections/ProjectModelsSection.tsx @@ -147,8 +147,9 @@ export interface ProjectModelsSectionProps extends SectionBaseProps { addToast: (message: string, type?: ToastType) => void; onOpenWorkflowSettings?: () => void; registerWorkflowLaneSaver?: (saver: SectionSaveHandler | null) => void; + onWorkflowLanesChange?: () => void; } -export function ProjectModelsSection({ form, setForm, models, projectId, onOpenWorkflowSettings, registerWorkflowLaneSaver, }: ProjectModelsSectionProps) { +export function ProjectModelsSection({ form, setForm, models, projectId, onOpenWorkflowSettings, registerWorkflowLaneSaver, onWorkflowLanesChange, }: ProjectModelsSectionProps) { const { t } = useTranslation("app"); const { agents, loading: agentsLoading } = useAgentsMapCache(projectId); const { modelLanes, getLaneStatus, getLaneValue, updateLaneValue, resetLaneValue, getLaneThinkingValue, updateLaneThinkingValue, resetLaneThinkingValue, availableModels, modelsLoading, favoriteProviders, favoriteModels, onToggleFavorite, onToggleModelFavorite, editingPresetId, setEditingPresetId, presetDraft, setPresetDraft, onSavePresetDraft, confirmDelete, } = models; @@ -212,6 +213,13 @@ export function ProjectModelsSection({ form, setForm, models, projectId, onOpenW ...Object.fromEntries(Object.entries(workflowPending).filter(([, value]) => value !== null)), }), [workflowPayload, workflowPending]); const setWorkflowPairValue = useCallback((pair: WorkflowModelPair, value: string) => { + /* + FNXC:SettingsAutoSave 2026-08-03-01:00: + FN-8395 removes the primary Save button. Notify the Settings shell for + every workflow-lane mutation because these pending overrides are local + section state rather than keys in its shared form. + */ + onWorkflowLanesChange?.(); setWorkflowRejections((current) => { const next = { ...current }; delete next[pair.providerId]; @@ -231,10 +239,11 @@ export function ProjectModelsSection({ form, setForm, models, projectId, onOpenW [pair.modelId]: value.slice(slashIdx + 1), }; }); - }, []); + }, [onWorkflowLanesChange]); const setWorkflowThinkingValue = useCallback((pair: WorkflowModelPair, value: string) => { if (!pair.thinkingId) return; + onWorkflowLanesChange?.(); setWorkflowRejections((current) => { if (!pair.thinkingId || !current[pair.thinkingId]) return current; @@ -243,7 +252,7 @@ export function ProjectModelsSection({ form, setForm, models, projectId, onOpenW return next; }); setWorkflowPending((current) => ({ ...current, [pair.thinkingId as string]: value || null })); - }, []); + }, [onWorkflowLanesChange]); const resetWorkflowPairValue = useCallback((pair: WorkflowModelPair) => { setWorkflowPairValue(pair, ""); setWorkflowThinkingValue(pair, "");