Skip to content

Conversation

@brunobergher
Copy link
Collaborator

@brunobergher brunobergher commented Sep 30, 2025

Description

Up to now, the "Done" button in settings would always warn about unsaved changes, even when there weren't any.
This fixes the behavior of one of the panels, which was flipping a dirty bit unnecessarily.

Test Procedure

There are automated tests, but to manually check it:

  1. Go to settings
  2. Click done – you should get no popup
  3. Go back to settings
  4. Change something
  5. Click done – you should get a popup

Important

Fixes overeager unsaved changes dialog in settings by refining change detection logic in SettingsView.tsx and ApiOptions.tsx.

  • Behavior:
    • Fixes overeager unsaved changes dialog in SettingsView.tsx by refining change detection logic.
    • Adjusts setApiConfigurationField in ApiOptions.tsx to pass false for non-user actions.
  • Tests:
    • Adds SettingsView.change-detection.spec.tsx to test change detection logic.
    • Adds SettingsView.unsaved-changes.spec.tsx to verify unsaved changes dialog behavior.
  • Misc:
    • Updates setCachedState logic in SettingsView.tsx to prevent unnecessary change detection.

This description was created by Ellipsis for 4e54cf9. You can customize this summary. It will automatically update as commits are pushed.

@dosubot dosubot bot added size:XL This PR changes 500-999 lines, ignoring generated files. bug Something isn't working labels Sep 30, 2025
// This prevents the dirty state when the component initializes and auto-syncs values
const isInitialSync = !isUserAction && previousValue === undefined && value !== undefined
// Treat undefined, null, and empty string as uninitialized states
const isInitialSync =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice improvement: treating undefined, null, and empty string as uninitialized prevents false change detection. Consider extracting this check into a helper for reuse and clarity.

This comment was generated because it violated a code review rule: irule_tTqpIuNs8DV0QFGj.

@hannesrudolph hannesrudolph added the Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. label Sep 30, 2025
Copy link
Contributor

@roomote roomote bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found some issues that need attention. See inline comments for details.

setApiConfigurationField("apiModelId", selectedModelId)
// Pass false as third parameter to indicate this is not a user action
// This is an internal sync, not a user-initiated change
setApiConfigurationField("apiModelId", selectedModelId, false)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Internal sync path correctly marks isUserAction=false. To avoid future drift, audit similar internal sync sites (provider/model defaults, migrations) and consider centralizing via a small helper to ensure they always pass false.

const isInitialSync = !isUserAction && previousValue === undefined && value !== undefined
// Treat undefined, null, and empty string as uninitialized states
const isInitialSync =
!isUserAction &&
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P3] The isInitialSync detection is duplicated logic you’ll likely want consistent across fields. Consider extracting a small helper (e.g., isInitializingValue(prev, next, isUserAction)) and unit-testing it so future edits don’t regress this behavior.

@daniel-lxs daniel-lxs moved this from Triage to PR [Needs Review] in Roo Code Roadmap Sep 30, 2025
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Sep 30, 2025
@hannesrudolph hannesrudolph added PR - Needs Review and removed Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. labels Sep 30, 2025
@mrubens mrubens merged commit 9bcd991 into main Oct 1, 2025
26 checks passed
@mrubens mrubens deleted the bb/unsaved-settings branch October 1, 2025 11:39
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Oct 1, 2025
@github-project-automation github-project-automation bot moved this from PR [Needs Review] to Done in Roo Code Roadmap Oct 1, 2025
brunobergher added a commit that referenced this pull request Oct 1, 2025
* origin/main:
  fix: Addresses overeager 'there are unsaved changes' dialog in settings (#8410)
  Add structured data to the homepage (#8427)
  fix: show send button when only images are selected in chat textarea (#8423)
  Include reasoning messages in cloud tasks (#8401)
  chore: Remove unsupported Gemini 2.5 Flash Image Preview free model (#8359)
  A couple more sonnet 4.5 fixes (#8421)
  Changeset version bump (#8414)
  chore: add changeset for v3.28.14 (#8413)
  feat: add GLM-4.6 model support for z.ai provider (#8408)
mini2s added a commit to zgsm-ai/costrict that referenced this pull request Oct 1, 2025
* chore(deps): update dependency @changesets/cli to v2.29.6 (RooCodeInc#7376)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* chore(deps): update dependency nock to v14.0.10 (RooCodeInc#6465)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* chore(deps): update dependency eslint-plugin-turbo to v2.5.6 (RooCodeInc#7764)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* refactor(utils): simplify shell detection logic

* Merge remote-tracking branch 'upstream/main' into roo-to-main

* test: update shell detection test expectation and mocks

* test: add mock cleanup in shell detection tests

* test: update shell detection tests to prioritize PowerShell 7

* fix(workspace-event-monitor): increase max retries and improve retry logic

* refactor(codebase): improve error handling and workspace validation

* Revert PR RooCodeInc#7188 - Restore temperature parameter to fix TabbyApi/ExLlamaV2 crashes (RooCodeInc#7594)

* fix: reduce CodeBlock button z-index to prevent overlap with popovers (RooCodeInc#7783)

Fixes RooCodeInc#7703 - CodeBlock language dropdown and copy button were appearing above popovers due to z-index: 100. Reduced to z-index: 40 to maintain proper layering hierarchy while keeping buttons functional.

* Make ollama models info transport work like lmstudio (RooCodeInc#7679)

* feat: add click-to-edit, ESC-to-cancel, and fix padding consistency for chat messages (RooCodeInc#7790)

* feat: add click-to-edit, ESC-to-cancel, and fix padding consistency

- Enable click-to-edit for past messages by making message text clickable
- Add ESC key handler to cancel edit mode in ChatTextArea
- Fix padding consistency between past and queued message editors
- Adjust right padding for edit mode to accommodate cancel button

Fixes RooCodeInc#7788

* fix: adjust padding and layout for ChatTextArea in edit mode

* refactor: replace hardcoded pr-[72px] with standard Tailwind pr-20 class

---------

Co-authored-by: Roo Code <[email protected]>
Co-authored-by: Hannes Rudolph <[email protected]>
Co-authored-by: daniel-lxs <[email protected]>

* Let people paste in the auth redirect url (RooCodeInc#7805)

Co-authored-by: roomote[bot] <219738659+roomote[bot]@users.noreply.github.com>
Co-authored-by: Bruno Bergher <[email protected]>

* test: change console.error to console.warn in tests

* fix: resolve chat message edit/delete duplication issues (RooCodeInc#7793)

* fix: add GIT_EDITOR env var to merge-resolver mode for non-interactive rebase (RooCodeInc#7819)

* UI: Render reasoning as plain italic (match <thinking>) (RooCodeInc#7752)

Co-authored-by: Roo Code <[email protected]>
Co-authored-by: Hannes Rudolph <[email protected]>
Co-authored-by: daniel-lxs <[email protected]>

* Add taskSyncEnabled to userSettingsConfigSchema (RooCodeInc#7827)

feat: add taskSyncEnabled to userSettingsConfigSchema

Co-authored-by: Roo Code <[email protected]>

* Release: v1.75.0 (RooCodeInc#7829)

chore: bump version to v1.75.0

* fix: prevent negative cost values and improve label visibility in evals chart (RooCodeInc#7830)

Co-authored-by: Roo Code <[email protected]>

* Fix Groq context window display (RooCodeInc#7839)

* test: enhance vscode mock implementations and error handling

* feat(chat): replace edit button with copy functionality

* refactor(core): enhance binary file detection and encoding handling

* separate task sync roomote control (RooCodeInc#7799)

* feat: separate Task Sync and Roomote Control settings

- Add new taskSyncEnabled setting to control task content syncing
- Keep remoteControlEnabled for Roomote Control functionality
- Task Sync controls whether task content is sent to cloud
- Roomote Control controls whether cloud can send instructions back
- Roomote Control now depends on Task Sync being enabled
- Usage metrics (tokens, cost) always reported regardless of settings
- Update UI with two separate toggles and clear descriptions
- Add info text explaining usage metrics are always reported

* feat: add missing translations for Task Sync and Roomote Control settings

- Added taskSync, taskSyncDescription, remoteControlRequiresTaskSync, and usageMetricsAlwaysReported keys to all non-English cloud.json files
- Updated cloudBenefit keys to match English structure
- Ensured all languages have consistent translation keys for the new Task Sync and Roomote Control features

* Cloud: cleanup taskSyncEnabled additions

* fix: correct indentation localization files

---------

Co-authored-by: Roo Code <[email protected]>

* feat: In-extension dismissible upsells for Roo Code Cloud (RooCodeInc#7850)

* First pass at separate upsell dialog

* Revert PR RooCodeInc#7188 - Restore temperature parameter to fix TabbyApi/ExLlamaV2 crashes (RooCodeInc#7594)

* fix: reduce CodeBlock button z-index to prevent overlap with popovers (RooCodeInc#7783)

Fixes RooCodeInc#7703 - CodeBlock language dropdown and copy button were appearing above popovers due to z-index: 100. Reduced to z-index: 40 to maintain proper layering hierarchy while keeping buttons functional.

* Make ollama models info transport work like lmstudio (RooCodeInc#7679)

* feat: add click-to-edit, ESC-to-cancel, and fix padding consistency for chat messages (RooCodeInc#7790)

* feat: add click-to-edit, ESC-to-cancel, and fix padding consistency

- Enable click-to-edit for past messages by making message text clickable
- Add ESC key handler to cancel edit mode in ChatTextArea
- Fix padding consistency between past and queued message editors
- Adjust right padding for edit mode to accommodate cancel button

Fixes RooCodeInc#7788

* fix: adjust padding and layout for ChatTextArea in edit mode

* refactor: replace hardcoded pr-[72px] with standard Tailwind pr-20 class

---------

Co-authored-by: Roo Code <[email protected]>
Co-authored-by: Hannes Rudolph <[email protected]>
Co-authored-by: daniel-lxs <[email protected]>

* Let people paste in the auth redirect url (RooCodeInc#7805)

Co-authored-by: roomote[bot] <219738659+roomote[bot]@users.noreply.github.com>
Co-authored-by: Bruno Bergher <[email protected]>

* fix: resolve chat message edit/delete duplication issues (RooCodeInc#7793)

* fix: add GIT_EDITOR env var to merge-resolver mode for non-interactive rebase (RooCodeInc#7819)

* UI: Render reasoning as plain italic (match <thinking>) (RooCodeInc#7752)

Co-authored-by: Roo Code <[email protected]>
Co-authored-by: Hannes Rudolph <[email protected]>
Co-authored-by: daniel-lxs <[email protected]>

* Add taskSyncEnabled to userSettingsConfigSchema (RooCodeInc#7827)

feat: add taskSyncEnabled to userSettingsConfigSchema

Co-authored-by: Roo Code <[email protected]>

* Release: v1.75.0 (RooCodeInc#7829)

chore: bump version to v1.75.0

* fix: prevent negative cost values and improve label visibility in evals chart (RooCodeInc#7830)

Co-authored-by: Roo Code <[email protected]>

* Fix Groq context window display (