feat(settings): persist extension runtime settings - #637
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe change adds extension-defined runtime settings. It introduces registry support, SQL and MongoDB persistence, synchronized validation and updates, admin API routes, application wiring, and a dashboard settings interface. ChangesRuntime settings
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Dashboard
participant AdminHandler
participant RuntimeSettingsService
participant RuntimeSettingsStore
Dashboard->>AdminHandler: Request runtime setting descriptors
AdminHandler->>RuntimeSettingsService: List settings
RuntimeSettingsService-->>AdminHandler: Ordered descriptors
AdminHandler-->>Dashboard: Settings response
Dashboard->>AdminHandler: Update setting key and value
AdminHandler->>RuntimeSettingsService: Validate and apply update
RuntimeSettingsService->>RuntimeSettingsStore: Persist value
RuntimeSettingsStore-->>RuntimeSettingsService: Save result
RuntimeSettingsService-->>AdminHandler: Updated descriptor
AdminHandler-->>Dashboard: Update response
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Confidence Score: 5/5The PR is safe to merge; no blocking failure remains. The exercised admin update flow persisted a valid setting, synchronized it to a separate running instance, and preserved state when an invalid value was submitted.
 T-Rex LogsWhat T-Rex did
Reviews (2): Last reviewed commit: "fix(settings): stop reconciliation befor..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/admin/handler_runtime_settings_test.go`:
- Around line 60-99: Add test cases alongside TestRuntimeSettingsListAndUpdate
and TestRuntimeSettingManagedByEnvironmentIsReadOnly covering unknown-key PUTs
returning 404 with runtime_setting_not_found, invalid option values returning
400, and GET/PUT requests on a handler without WithRuntimeSettings returning the
feature-unavailable response. Reuse the existing Echo route setup and runtime
settings test helpers, and verify the setting remains unchanged where
applicable.
In `@internal/runtimesettings/service_test.go`:
- Around line 48-104: Add a test alongside TestServicePersistsAndRestoresSetting
that uses a stub Store whose Set method returns an error, then calls
Service.Update and verifies the setting’s value is restored to its previous
state via Apply. Assert the update returns the store error while preserving the
existing locked and persistence test coverage.
In `@internal/runtimesettings/service.go`:
- Around line 31-72: Reject non-locked settings with empty Options during
registration in Service.New, returning a startup error immediately after the
descriptor.Locked check; update ext/ext.go documentation for
SettingDescriptor/RuntimeSetting to state that Options must include every
accepted value and that empty Options makes an unlocked setting non-editable.
Apply the service.go change at lines 31-72 and the documentation change in
ext/ext.go at lines 84-113.
- Around line 87-111: Update the Service.Update method to add the same
nil-receiver guard used by List and Close before accessing s.mu or other fields.
Return the established nil-service error consistently, while preserving the
existing update behavior for non-nil receivers.
In `@web/dashboard/src/pages/settings/RuntimeSettings.svelte`:
- Around line 37-38: Update the select disabled-state logic in the settings UI
so every select is disabled whenever any save is in flight, not only when
savingKey matches that setting. Keep locked settings disabled as before, and
ensure the save guard in save remains consistent with this global in-flight
behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: cf009c00-ad9d-474f-8eb7-4cc190dd07f7
⛔ Files ignored due to path filters (4)
internal/admin/dashboard/static/dist/assets/index-6sHfhgfb.jsis excluded by!**/dist/**internal/admin/dashboard/static/dist/assets/index-B-Rv4AUL.jsis excluded by!**/dist/**internal/admin/dashboard/static/dist/assets/index-B_MQg975.cssis excluded by!**/dist/**internal/admin/dashboard/static/dist/index.htmlis excluded by!**/dist/**
📒 Files selected for processing (16)
ext/ext.goext/registry.goext/registry_test.gointernal/admin/handler.gointernal/admin/handler_runtime_settings.gointernal/admin/handler_runtime_settings_test.gointernal/admin/routes.gointernal/admin/routes_test.gointernal/app/app.gointernal/runtimesettings/service.gointernal/runtimesettings/service_test.gointernal/runtimesettings/store.gointernal/runtimesettings/store_mongodb.gointernal/runtimesettings/store_sql.goweb/dashboard/src/pages/settings/RuntimeSettings.svelteweb/dashboard/src/pages/settings/SettingsPage.svelte
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/runtimesettings/store_mongodb_test.go`:
- Around line 11-29: Extend TestMongoDBStoreRoundTrip with a nil-database call
to NewMongoDBStore and assert that it returns an error, covering the
constructor’s nil-database rejection while preserving the existing
mongotest-backed round-trip checks.
In `@internal/runtimesettings/sync.go`:
- Around line 39-71: Update Service.sync to continue iterating through all keys
after store.Get or setting.Apply fails, while recording each failure and
returning an aggregate error after the loop. Preserve the existing invalid-value
handling and successful application behavior, and include each failure’s setting
key and underlying error in the aggregate.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 004930c0-6783-4be5-8804-c045f4be5749
⛔ Files ignored due to path filters (2)
internal/admin/dashboard/static/dist/assets/index-D6HB9TkL.jsis excluded by!**/dist/**internal/admin/dashboard/static/dist/index.htmlis excluded by!**/dist/**
📒 Files selected for processing (11)
ext/ext.gointernal/admin/handler_runtime_settings.gointernal/admin/handler_runtime_settings_test.gointernal/runtimesettings/service.gointernal/runtimesettings/service_test.gointernal/runtimesettings/store.gointernal/runtimesettings/store_mongodb.gointernal/runtimesettings/store_mongodb_test.gointernal/runtimesettings/store_sql.gointernal/runtimesettings/sync.goweb/dashboard/src/pages/settings/RuntimeSettings.svelte
…n-settings # Conflicts: # internal/admin/dashboard/static/dist/assets/index-B_MQg975.css # internal/admin/dashboard/static/dist/assets/index-CDxZDTJg.css # internal/admin/dashboard/static/dist/assets/index-Dv37Tmj5.css # internal/admin/dashboard/static/dist/index.html
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/app/subsystems.go`:
- Around line 109-112: Update shutdownOrder so runtimeSettings is stopped before
providers, ensuring no runtime-setting Apply operations can occur during
provider teardown. Preserve the existing subsystem close mechanism and ordering
of the remaining shutdown entries.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: aad73ddd-6f42-4dc0-85ae-7c12829202ec
⛔ Files ignored due to path filters (3)
internal/admin/dashboard/static/dist/assets/index-5TzGYjSi.jsis excluded by!**/dist/**internal/admin/dashboard/static/dist/assets/index-C9yo6Tyg.cssis excluded by!**/dist/**internal/admin/dashboard/static/dist/index.htmlis excluded by!**/dist/**
📒 Files selected for processing (3)
internal/app/app.gointernal/app/subsystems.gointernal/app/subsystems_test.go
…n-settings # Conflicts: # internal/admin/dashboard/static/dist/index.html
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Why
GoModel Pro needs a deployment-wide prompt-compression level that can change at runtime without putting Pro-specific behavior into open core or browser-local storage.
Verification
go test ./...npm testnpm run checknpm run buildSummary by CodeRabbit