scriptoriumadmin/Roo-Code
1
0
Fork 0
mirror of https://github.com/RooVetGit/Roo-Code.git synced 2026-10-07 02:58:15 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

fix: prevent false unsaved-changes prompt after Mode/API change (#8230)

Browse source
This commit is contained in:
Hannes Rudolph 2025-10-12 14:47:22 -06:00
parent 3a47c55a2e
commit 2503f3c9e7
2 changed files with 25 additions and 14 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

27
webview-ui/src/components/settings/SettingsView.tsx
View file

@ -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)

12
webview-ui/src/components/settings/__tests__/SettingsView.unsaved-changes.spec.tsx
View file

@ -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