From 2503f3c9e74ed1fcedc731783040f0ce31a13c46 Mon Sep 17 00:00:00 2001
From: Hannes Rudolph <hrudolph@gmail.com>
Date: Sun, 12 Oct 2025 14:47:22 -0600
Subject: [PATCH] fix: prevent false unsaved-changes prompt after Mode/API
 change (#8230)

---
 .../src/components/settings/SettingsView.tsx  | 27 +++++++++++++------
 .../SettingsView.unsaved-changes.spec.tsx     | 12 ++++-----
 2 files changed, 25 insertions(+), 14 deletions(-)

diff --git a/webview-ui/src/components/settings/SettingsView.tsx b/webview-ui/src/components/settings/SettingsView.tsx
index 93b1b39e50..237d98c254 100644
--- a/webview-ui/src/components/settings/SettingsView.tsx
+++ b/webview-ui/src/components/settings/SettingsView.tsx
@@ -126,6 +126,15 @@ const SettingsView = forwardRef<SettingsViewRef, SettingsViewProps>(({ onDone, t
 	const prevApiConfigName = useRef(currentApiConfigName)
 	const confirmDialogHandler = useRef<() => void>()
 
+	// Guard: ignore programmatic syncs during initial mount to prevent false dirty state
+	const isInitialMountRef = useRef(true)
+	useEffect(() => {
+		const id = setTimeout(() => {
+			isInitialMountRef.current = false
+		}, 0)
+		return () => clearTimeout(id)
+	}, [])
+
 	const [cachedState, setCachedState] = useState(() => extensionState)
 
 	const {
@@ -239,15 +248,17 @@ const SettingsView = forwardRef<SettingsViewRef, SettingsViewProps>(({ onDone, t
 
 				const previousValue = prevState.apiConfiguration?.[field]
 
-				// Only skip change detection for automatic initialization (not user actions)
-				// This prevents the dirty state when the component initializes and auto-syncs values
-				// Treat undefined, null, and empty string as uninitialized states
+				// Only skip change detection for:
+				// - Automatic initialization (undefined/null/empty -> value)
+				// - Any non-user programmatic syncs happening during the initial mount of SettingsView
+				//   (prevents dirty state when provider/model defaults auto-sync on open)
 				const isInitialSync =
-					!isUserAction &&
-					(previousValue === undefined || previousValue === "" || previousValue === null) &&
-					value !== undefined &&
-					value !== "" &&
-					value !== null
+					(!isUserAction &&
+						(previousValue === undefined || previousValue === "" || previousValue === null) &&
+						value !== undefined &&
+						value !== "" &&
+						value !== null) ||
+					(!isUserAction && isInitialMountRef.current)
 
 				if (!isInitialSync) {
 					setChangeDetected(true)
diff --git a/webview-ui/src/components/settings/__tests__/SettingsView.unsaved-changes.spec.tsx b/webview-ui/src/components/settings/__tests__/SettingsView.unsaved-changes.spec.tsx
index 6f7d01ee86..d7298a2cbd 100644
--- a/webview-ui/src/components/settings/__tests__/SettingsView.unsaved-changes.spec.tsx
+++ b/webview-ui/src/components/settings/__tests__/SettingsView.unsaved-changes.spec.tsx
@@ -29,7 +29,7 @@ vi.mock("@src/i18n/TranslationContext", () => ({
 
 // Mock UI components
 vi.mock("@src/components/ui", () => ({
-	AlertDialog: ({ children }: any) => <div>{children}</div>,
+	AlertDialog: ({ open, children }: any) => (open ? <div data-testid="alert-dialog">{children}</div> : null),
 	AlertDialogContent: ({ children }: any) => <div>{children}</div>,
 	AlertDialogTitle: ({ children }: any) => <div>{children}</div>,
 	AlertDialogDescription: ({ children }: any) => <div>{children}</div>,
@@ -218,7 +218,7 @@ describe("SettingsView - Unsaved Changes Detection", () => {
 	// TODO: Fix underlying issue - dialog appears even when no user changes have been made
 	// This happens because some component is triggering setCachedStateField during initialization
 	// without properly marking it as a non-user action
-	it.skip("should not show unsaved changes when settings are automatically initialized", async () => {
+	it("should not show unsaved changes when settings are automatically initialized", async () => {
 		const onDone = vi.fn()
 
 		render(
@@ -252,7 +252,7 @@ describe("SettingsView - Unsaved Changes Detection", () => {
 	})
 
 	// TODO: Fix underlying issue - see above
-	it.skip("should not trigger unsaved changes for automatic model initialization", async () => {
+	it("should not trigger unsaved changes for automatic model initialization", async () => {
 		const onDone = vi.fn()
 
 		// Mock ApiOptions to simulate ModelPicker initialization
@@ -351,7 +351,7 @@ describe("SettingsView - Unsaved Changes Detection", () => {
 	})
 
 	// TODO: Fix underlying issue - see above
-	it.skip("should handle initialization from undefined to value without triggering unsaved changes", async () => {
+	it("should handle initialization from undefined to value without triggering unsaved changes", async () => {
 		const onDone = vi.fn()
 
 		// Start with undefined apiModelId
@@ -395,7 +395,7 @@ describe("SettingsView - Unsaved Changes Detection", () => {
 	})
 
 	// TODO: Fix underlying issue - see above
-	it.skip("should handle initialization from null to value without triggering unsaved changes", async () => {
+	it("should handle initialization from null to value without triggering unsaved changes", async () => {
 		const onDone = vi.fn()
 
 		// Start with null apiModelId
@@ -439,7 +439,7 @@ describe("SettingsView - Unsaved Changes Detection", () => {
 	})
 
 	// TODO: Fix underlying issue - see above
-	it.skip("should not trigger changes when ApiOptions syncs model IDs during mount", async () => {
+	it("should not trigger changes when ApiOptions syncs model IDs during mount", async () => {
 		const onDone = vi.fn()
 
 		// This specifically tests the bug we fixed where ApiOptions' useEffect