Refactor provider settings to declarative metadata#2452
Conversation
- Centralize provider field metadata and rendering - Reuse the same form in add-instance and instance cards - Add coverage for schema-driven field derivation and config merging
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
github-actions
Bot
added
size:XL
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
ApprovabilityVerdict: Approved Mechanical refactor moving provider settings field definitions from hardcoded UI arrays to declarative schema annotations. No runtime behavior changes - same fields render with same values. The unresolved comments identify minor UI polish issues, not correctness or security concerns. You can customize Macroscope's approvability policy. Learn more. |
- Derive form labels and descriptions from contract annotations - Simplify provider metadata and reuse shared empty config draft - Cover annotated field metadata in settings form tests
macroscopeapp
Bot
dismissed
their stale review
Dismissing prior approval to re-evaluate cc102a0
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| {description} | ||
| </label> | ||
| </FieldFrame> | ||
| ); |
There was a problem hiding this comment.
Textarea in card variant bypasses DraftInput buffering pattern
Low Severity
The textarea control type in the card variant uses a standard Textarea with onChange (firing on every keystroke), while the text and password controls correctly use DraftInput which buffers keystrokes and commits only on blur. DraftInput was specifically designed to prevent per-keystroke settings saves and server round-trips. If a textarea field is added to a provider's card settings, each keystroke will trigger updateConfig → onUpdate → full settings propagation, causing exactly the lag DraftInput was built to avoid. The card variant's textarea path needs an equivalent buffered input component to be consistent with the other controls.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit cc102a0. Configure here.
- Derive provider settings UI from schema annotations - Remove separate provider UI metadata and hide internal fields in contracts
macroscopeapp
Bot
dismissed
their stale review
Dismissing prior approval to re-evaluate f6731f0
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Boolean fields ignore clearWhenEmpty semantics
- Added a check in the boolean path of nextProviderConfigWithFieldValue to delete the key when clearWhenEmpty is "omit" and the value is false (the default), mirroring the existing string field behavior.
Or push these changes by commenting:
@cursor push 944381290b
Preview (944381290b)
diff --git a/apps/web/src/components/settings/ProviderSettingsForm.tsx b/apps/web/src/components/settings/ProviderSettingsForm.tsx
--- a/apps/web/src/components/settings/ProviderSettingsForm.tsx
+++ b/apps/web/src/components/settings/ProviderSettingsForm.tsx
@@ -109,7 +109,11 @@
config !== null && typeof config === "object" ? { ...(config as Record<string, unknown>) } : {};
if (typeof value === "boolean") {
- base[field.key] = value;
+ if (field.clearWhenEmpty === "omit" && value === false) {
+ delete base[field.key];
+ } else {
+ base[field.key] = value;
+ }
return Object.keys(base).length > 0 ? base : undefined;
}You can send follow-ups to the cloud agent here.
- Add schema-level ordering metadata for provider settings - Derive field order declaratively in the web form - Remove per-field order annotations from settings definitions
macroscopeapp
Bot
dismissed
their stale review
Dismissing prior approval to re-evaluate e5b9956
- drop false switch values when settings declare `clearWhenEmpty: omit` - add coverage for boolean field read/write behavior
macroscopeapp
Bot
dismissed
their stale review
Dismissing prior approval to re-evaluate 77e6873
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Dialog variant loses spacing between label, input, description
- Moved 'grid gap-1.5' from the FieldFrame wrapper div to the element itself for the dialog variant, so the CSS grid gap correctly spaces the label text, input, and description children.
Or push these changes by commenting:
@cursor push ef8ded46ad
Preview (ef8ded46ad)
diff --git a/apps/web/src/components/settings/ProviderSettingsForm.tsx b/apps/web/src/components/settings/ProviderSettingsForm.tsx
--- a/apps/web/src/components/settings/ProviderSettingsForm.tsx
+++ b/apps/web/src/components/settings/ProviderSettingsForm.tsx
@@ -202,7 +202,7 @@
if (field.control === "textarea") {
return (
<FieldFrame variant={variant}>
- <label htmlFor={inputId} className={cn(variant === "card" && "block")}>
+ <label htmlFor={inputId} className={cn(variant === "card" ? "block" : "grid gap-1.5")}>
{label}
<Textarea
id={inputId}
@@ -223,7 +223,7 @@
const type = field.control === "password" ? "password" : undefined;
return (
<FieldFrame variant={variant}>
- <label htmlFor={inputId} className={cn(variant === "card" && "block")}>
+ <label htmlFor={inputId} className={cn(variant === "card" ? "block" : "grid gap-1.5")}>
{label}
{variant === "card" ? (
<DraftInputYou can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit 77e6873. Configure here.
| return <div className="border-t border-border/60 px-4 py-3 sm:px-5">{props.children}</div>; | ||
| } | ||
| return <div className="grid gap-1.5">{props.children}</div>; | ||
| } |
There was a problem hiding this comment.
Dialog variant loses spacing between label, input, description
Medium Severity
FieldFrame for the dialog variant applies grid gap-1.5 on a wrapper div, but its only child is the <label> element. The gap has no effect because there's only one grid item. The label text, Input, and description are all inside the label with no layout class, so they lose the vertical spacing the old code provided (which had grid gap-1.5 directly on the <label> itself). This causes a visual regression where form fields in the add-provider dialog appear cramped with no gap between label, input, and description.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 77e6873. Configure here.
- preserve schema-defined boolean defaults in provider settings - omit default-valued switch fields when clearWhenEmpty is omit - add coverage for boolean read/write defaults
Co-authored-by: Julius Marminge <julius@macmini.local>
1 participant
Summary
ProviderSettingsFormthat renders text, password, textarea, and switch controls for both the add-instance dialog and provider cards.Testing
Not runNote
Medium Risk
Refactors how provider instance configs are rendered and normalized, and changes contract schemas/annotations that multiple surfaces may rely on. Risk is mainly UI/config persistence regressions (fields hidden/ordered differently, empty values omitted) rather than security.
Overview
Provider instance configuration is now rendered from declarative, schema-annotated metadata instead of hardcoded per-surface field arrays, via a new reusable
ProviderSettingsFormused in both the add-instance dialog and instance cards.Driver metadata in
providerDriverMeta.tsis updated to reference typed Effect Schemas (CodexSettings,ClaudeSettings,CursorSettings,OpenCodeSettings) and the contracts add schema annotations for field labels/descriptions/placeholders, ordering, hidden fields, and empty-value persistence rules. The add-instance flow switches to per-driver config drafts, and save/update logic now round-trips unknown config keys while omitting empty fields according toclearWhenEmpty, with unit tests covering derivation and normalization helpers.Reviewed by Cursor Bugbot for commit a4ac493. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Refactor provider settings to use schema-driven declarative metadata
fieldsarrays on driver definitions withsettingsSchemaannotations in packages/contracts/src/settings.ts, covering titles, descriptions, placeholders, control types (text, password, textarea, switch), andclearWhenEmptypolicies.cardanddialogvisual variants.AddProviderInstanceDialogandProviderInstanceCardto useProviderSettingsForminstead of manual field rendering; per-driver config drafts are preserved when switching tabs in the dialog.SettingsPanelsnow derives its provider list fromDRIVER_OPTIONSrather than a separate hard-coded array.clearWhenEmptypolicy rather than being stored as empty strings.Macroscope summarized a4ac493.