Shared Multi Step Form for Object Storage Connectors

[lovincyrus]

This PR implements a shared, config-driven multi-step form architecture for object storage connectors (GCS, S3,
Azure). It replaces a GCS-specific implementation with a generalized system that splits connector configuration
into two steps:

1. Connector step: Authentication configuration
2. Source step: Resource path and model creation

• Azure explicit credentials
• S3 explicit credentials
• Remove Skip and add a contextual messaging in right panel that acts the same
• Add public option that changes button to Continue
• Pin Help text to bottom of right panel, update messaging on Source model page
• Change Test and Import Data to, Just Import Data or Create Model
• https://github.com/rilldata/rill/pull/8395/files
• Connector validation for authentication method
• ⚠️ Centralize auth method state in connectorStepStore to avoid initialization races
• ⚠️ Extract isSubmitDisabled logic to testable functions
• ⚠️ Add constants for magic strings ("public", etc.)

Checklist:

• Covered by tests
• Ran it and it works as intended
• Reviewed the diff before requesting a review
• Checked for unhandled edge cases
• Linked the issues it closes
• Checked if the docs need to be updated. If so, create a separate Linear DOCS issue
• Intend to cherry-pick into the release branch
• I'm proud of this work!

[lovincyrus]

Code Review: Shared Multi Step Form for Object Storage Connectors

Overview

This PR implements a shared, config-driven multi-step form architecture for object storage connectors (GCS, S3, Azure). It replaces a GCS-specific implementation with a generalized system that splits connector configuration into two steps:

1. Connector step: Authentication configuration
2. Source step: Resource path and model creation

The refactoring demonstrates good abstraction design by extracting authentication patterns into reusable configurations.

---

Strengths

1. Excellent Abstraction & Reusability ✅

The introduction of multi-step-auth-configs.ts is a strong design decision. This configuration-driven approach allows adding new connectors without duplicating form logic. The config structure is well-designed with:

• authOptions: Defines available authentication methods
• authFieldGroups: Maps each auth method to its required fields
• clearFieldsByMethod: Ensures form state hygiene when switching auth methods
• excludedKeys: Prevents duplicate rendering of auth fields

2. Strong Type Safety ✅

The type definitions in types.ts are well-structured with discriminated unions for AuthField ensuring type-safe rendering based on field type.

3. Good Component Boundaries ✅

The component hierarchy is well-organized:

AddDataForm.svelte (orchestrator)
  ↓
MultiStepFormRenderer.svelte (config → component adapter)
  ↓
MultiStepAuthForm.svelte (auth UI logic)

Each component has a clear responsibility.

---

Areas for Improvement

1. State Management Complexity ⚠️

Issue: The selectedAuthMethod state is managed across multiple layers with initialization logic scattered in AddDataForm.svelte:313-327. Multiple reactive statements compete to set the value, passed through multiple components with bind:authMethod.

Problems:

• Initialization race conditions: Multiple reactive statements compete to set selectedAuthMethod
• Unclear ownership: Who owns this state? Parent or child?
• Difficult debugging: State changes happen reactively across files

Recommendation: Centralize auth method state in connectorStepStore:

// connectorStepStore.ts
export const connectorStepStore = writable<{
  step: ConnectorStep;
  connectorConfig: Record<string, unknown> | null;
  selectedAuthMethod: string | null;  // Add this
}>({
  step: "connector",
  connectorConfig: null,
  selectedAuthMethod: null,
});

export function setAuthMethod(method: string) {
  connectorStepStore.update((state) => ({ ...state, selectedAuthMethod: method }));
}

2. Dynamic Validation Logic Needs Improvement ⚠️

The validation in FormValidation.ts:50-86 uses test-based conditional validation with getter dependencies.

Problems:

• Fragile getter dependency: Relies on authMethodGetter function passed through constructor
• Validation state not obvious: Hard to understand which fields are validated when

Recommendation: Use yup.when() with explicit dependencies:

fieldValidations[field.id] = yup.string().when('$authMethod', {
  is: (authMethod: string) => authMethod === method,
  then: (schema) => schema.required(`${label} is required`),
  otherwise: (schema) => schema.optional(),
});

3. Submit Button Logic Too Complex ⚠️

The isSubmitDisabled calculation in AddDataForm.svelte:137-210 is ~74 lines with nested conditionals.

Problems:

• Hard to test: Giant reactive statement with multiple branches
• Hard to maintain: Adding new conditions requires understanding all existing logic
• Duplicates validation logic: Re-implements field requirement checks

Recommendation: Extract to well-named functions:

function isMultiStepConnectorDisabled(config, selectedAuthMethod, paramsForm, paramsErrors) {
  const method = selectedAuthMethod || config.defaultAuthMethod || config.authOptions?.[0]?.value;
  if (!method) return true;
  
  const fields = config.authFieldGroups?.[method] || [];
  if (!fields.length && method === 'public') return false;
  
  return !fields.every(field => {
    if (field.optional ?? false) return true;
    const value = paramsForm[field.id];
    return !isEmpty(value) && !paramsErrors[field.id]?.length;
  });
}

$: isSubmitDisabled = (() => {
  if (isMultiStepConnector && stepState.step === "connector") {
    return isMultiStepConnectorDisabled(activeMultiStepConfig, selectedAuthMethod, $paramsForm, $paramsErrors);
  }
  // ... other cases
})();

4. Form State Clearing Logic Scattered ⚠️

Field clearing happens reactively in MultiStepAuthForm.svelte:28-39. If someone forgets to update clearFieldsByMethod, stale data persists.

Recommendation: Make clearing explicit and comprehensive:

function clearUnusedAuthFields(
  form: Record<string, any>,
  config: MultiStepFormConfig,
  selectedMethod: string
): Record<string, any> {
  const allAuthFields = new Set(
    Object.values(config.authFieldGroups).flat().map(f => f.id)
  );
  const activeFields = new Set(
    config.authFieldGroups[selectedMethod]?.map(f => f.id) || []
  );
  
  const clearedForm = { ...form };
  for (const field of allAuthFields) {
    if (!activeFields.has(field)) {
      clearedForm[field] = "";
    }
  }
  return clearedForm;
}

This approach clears all non-active auth fields automatically and doesn't require maintaining clearFieldsByMethod separately.

5. Submit Flow Has Hidden Public Auth Shortcut ⚠️

In AddDataFormManager.ts:317-321, public auth skips connection testing:

if (selectedAuthMethod === "public") {
  setConnectorConfig(values);
  setStep("source");
  return;
}

Problems:

• Hidden logic: User clicks "Continue" but validation still runs
• Hard to discover: This shortcut is buried in the submit handler

Recommendation: Extract to named functions:

private handlePublicAuthSubmit(values: Record<string, unknown>) {
  // Public auth doesn't require connection testing
  setConnectorConfig(values);
  setStep("source");
}

private async handlePrivateAuthSubmit(queryClient: any, values: Record<string, unknown>) {
  await submitAddConnectorForm(queryClient, this.connector, values, false);
  setConnectorConfig(values);
  setStep("source");
}

6. "Save Anyway" Logic is Confusing ⚠️

The conditions for showing "Save Anyway" button in AddDataFormManager.ts:287-307 have 6 conditions ANDed together.

Recommendation: Extract to a well-named predicate:

private shouldShowSaveAnywayButton(args: {
  isConnectorForm: boolean;
  event: any;
  stepState: ConnectorStepState;
  selectedAuthMethod?: string;
}): boolean {
  const { isConnectorForm, event, stepState, selectedAuthMethod } = args;
  
  // Only show for connector forms (not sources)
  if (!isConnectorForm) return false;
  
  // ClickHouse has its own error handling
  if (this.connector.name === "clickhouse") return false;
  
  // Need a submission result to show the button
  if (!event?.result) return false;
  
  // Multi-step connectors: don't show on source step (final step)
  if (stepState?.step === "source") return false;
  
  // Public auth bypasses connection test, so no "Save Anyway" needed
  if (stepState?.step === "connector" && selectedAuthMethod === "public") return false;
  
  return true;
}

7. Backend Connector Property Changes Not Well Explained ⚠️

The backend changes in runtime/drivers/s3/s3.go remove role assumption properties (aws_role_arn, aws_role_session_name, aws_external_id).

Problems:

• Breaking change: Existing connectors using role assumption will break
• No migration path: PR description doesn't mention how to handle existing configs

Recommendation:

• Add migration guide in PR description
• Consider deprecation path
• Add comment explaining why removed

8. Magic String "public" Used Throughout ⚠️

The string "public" appears in multiple places without constants.

Recommendation: Define constants:

// constants.ts
export const AUTH_METHOD_PUBLIC = "public" as const;
export const AUTH_METHOD_CREDENTIALS = "credentials" as const;

// Usage:
if (selectedAuthMethod === AUTH_METHOD_PUBLIC) { ... }

9. UI Copy Should Be Extracted to Constants ⚠️

Button labels are hardcoded in AddDataFormManager.ts:229-247.

Recommendation: Extract to a config:

export const BUTTON_LABELS = {
  public: { idle: "Continue", submitting: "Continuing..." },
  connector: { idle: "Test and Connect", submitting: "Testing connection..." },
  source: { idle: "Import Data", submitting: "Importing data..." },
} as const;

10. Adding New Connectors Requires Updates in Multiple Files ⚠️

To add a new multi-step connector, you need to update 4 different files.

Recommendation: Enforce via types:

// Ensure every MULTI_STEP_CONNECTOR has a config
type MultiStepConnectorName = typeof MULTI_STEP_CONNECTORS[number];
const _typeCheck: Record<MultiStepConnectorName, MultiStepFormConfig> = multiStepFormConfigs;

This creates a compile-time error if someone adds a connector without a config.

---

Testing Concerns

11. Complex Logic Needs More Tests ⚠️

Key areas needing tests:

1. Auth method switching: Does field clearing work correctly?
2. Validation logic: Are required fields enforced per auth method?
3. Public auth shortcut: Does it skip connection testing correctly?
4. Multi-step navigation: Can users go back? Is state preserved?

---

Summary & Recommendations

Priority Fixes (Before Merge)

1. ⚠️ Centralize auth method state in connectorStepStore to avoid initialization races
2. ⚠️ Extract isSubmitDisabled logic to testable functions
3. ⚠️ Add constants for magic strings ("public", etc.)
4. ⚠️ Document backend breaking changes and provide migration path

Medium Priority (Can Be Follow-up)

1. Improve validation with yup.when() instead of test-based validation
2. Simplify "Save Anyway" logic with named predicates
3. Extract field clearing logic to be config-driven
4. Add comprehensive tests for auth switching and validation

Low Priority (Nice to Have)

1. Extract button labels to constants
2. Standardize error handling utilities
3. Add type-level enforcement for multi-step connector configs

---

Final Verdict

Overall Assessment: Good abstraction work with some state management complexity

• ✅ Readability: Good component boundaries, but some overly complex reactive statements
• ✅ Maintainability: Config-driven design is excellent, but state management needs simplification
• ⚠️ Abstractions: Strong config system, but validation and state management could be cleaner
• ⚠️ Function Boundaries: Good separation, but some functions are too large (isSubmitDisabled)
• ⚠️ State Management: Needs improvement - auth method state scattered across components

The PR successfully generalizes the multi-step form pattern, making it reusable for all object storage connectors. The main concern is state management complexity around selectedAuthMethod, which should be centralized in the store before merging.

[royendo]

GCS: https://jam.dev/c/74536bbf-e99d-4381-b4ad-f9afd6b007a4

• when switching between JSON and HMAC, there's a split sec where HMAC shows missing creds in red
• when skipping using "already connected?" I cannot add a model, the button flickers but does nothing (see end of recording)

S3: https://jam.dev/c/1f2b5a0d-2ba3-46c4-95fb-73ce3d93b459

• connectors are iterating s3, s4, s5 intead of s3, s3_1, s3_2
• same as above: when skipping using "already connected?" I cannot add a model, the button flickers but does nothing

Azure: https://jam.dev/c/a9dfe375-4ed9-4c8a-9168-0451e2894ab2

• same as above: when skipping using "already connected?" I cannot add a model, the button flickers but does nothing
  Overall good! Thanks for adding the additional changes that I messaged you with!

[lovincyrus]

GCS: https://jam.dev/c/74536bbf-e99d-4381-b4ad-f9afd6b007a4

• when switching between JSON and HMAC, there's a split sec where HMAC shows missing creds in red
• when skipping using "already connected?" I cannot add a model, the button flickers but does nothing (see end of recording)

S3: https://jam.dev/c/1f2b5a0d-2ba3-46c4-95fb-73ce3d93b459

• connectors are iterating s3, s4, s5 intead of s3, s3_1, s3_2
• same as above: when skipping using "already connected?" I cannot add a model, the button flickers but does nothing

Azure: https://jam.dev/c/a9dfe375-4ed9-4c8a-9168-0451e2894ab2

• same as above: when skipping using "already connected?" I cannot add a model, the button flickers but does nothing
  Overall good! Thanks for adding the additional changes that I messaged you with!

Fixed! @royendo

Resolved

[royendo]

still creating s3, s4 file names

[lovincyrus]

still creating s3, s4 file names

Have migrated the fix from #8354

[royendo]

Looks good from Product QA.

[ericpgreen2]

Hey Cyrus,

A while back we wrote a design doc for connector forms that aligned on using JSON Schema as the form definition format: https://www.notion.so/rilldata/Snippets-API-design-doc-1f5ba33c8f5780798a0cfc819a406b42

The idea is that these schemas eventually live on the backend and can be used by both UI and AI/CLI.

Defining a config-driven form system that works across multiple connectors is definitely the right direction! But the custom TypeScript format (authFieldGroups, clearFieldsByMethod, etc.) diverges from that design. Instead, let's define these as JSON Schema in the frontend. This gets us closer to the design doc without requiring backend work now, and when we move the schemas to the backend later, it's just moving files.

Here's a rough sketch of what S3 might look like:

{
  "$schema": "http://json-schema.org/draft-07/schema#",
  "type": "object",
  "properties": {
    "auth_method": {
      "type": "string",
      "title": "Authentication method",
      "enum": ["access_keys", "public"],
      "default": "access_keys",
      "x-display": "radio"
    },
    "aws_access_key_id": {
      "type": "string",
      "title": "Access Key ID",
      "description": "AWS access key ID for the bucket",
      "x-secret": true,
      "x-step": "connector",
      "x-visible-if": { "auth_method": "access_keys" }
    },
    "aws_secret_access_key": {
      "type": "string",
      "title": "Secret Access Key",
      "description": "AWS secret access key for the bucket",
      "x-secret": true,
      "x-step": "connector",
      "x-visible-if": { "auth_method": "access_keys" }
    },
    "path": {
      "type": "string",
      "title": "S3 URI",
      "pattern": "^s3://",
      "x-step": "source"
    },
    "name": {
      "type": "string",
      "title": "Model name",
      "pattern": "^[a-zA-Z_][a-zA-Z0-9_]*$",
      "x-step": "source"
    }
  },
  "allOf": [
    {
      "if": { "properties": { "auth_method": { "const": "access_keys" } } },
      "then": { "required": ["aws_access_key_id", "aws_secret_access_key"] }
    }
  ]
}

This is illustrative—you'll need to think through the right JSON Schema properties. Try to stay as close to native JSON Schema as possible before reaching for custom x-* extensions. The ones I used above:

• x-secret: mask the input field
• x-step: which step of the multi-step flow this field belongs to
• x-display: UI hint for how to render (radio, select, etc.)
• x-visible-if: conditional visibility based on other field values

There may be better ways to model some of these. Let me know if you want to talk through it.

---

Developed in collaboration with Claude Code

[ericpgreen2]

See the comment above. A declarative config language to define these forms is definitely the right direction! It's so close to the JSON Schema approach we had discussed previously. This seems like a good time to take your code here one step closer to that design doc.

[lovincyrus]

Hey Cyrus,

A while back we wrote a design doc for connector forms that aligned on using JSON Schema as the form definition format: https://www.notion.so/rilldata/Snippets-API-design-doc-1f5ba33c8f5780798a0cfc819a406b42

The idea is that these schemas eventually live on the backend and can be used by both UI and AI/CLI.

Defining a config-driven form system that works across multiple connectors is definitely the right direction! But the custom TypeScript format (authFieldGroups, clearFieldsByMethod, etc.) diverges from that design. Instead, let's define these as JSON Schema in the frontend. This gets us closer to the design doc without requiring backend work now, and when we move the schemas to the backend later, it's just moving files.

Here's a rough sketch of what S3 might look like:

{
  "$schema": "http://json-schema.org/draft-07/schema#",
  "type": "object",
  "properties": {
    "auth_method": {
      "type": "string",
      "title": "Authentication method",
      "enum": ["access_keys", "public"],
      "default": "access_keys",
      "x-display": "radio"
    },
    "aws_access_key_id": {
      "type": "string",
      "title": "Access Key ID",
      "description": "AWS access key ID for the bucket",
      "x-secret": true,
      "x-step": "connector",
      "x-visible-if": { "auth_method": "access_keys" }
    },
    "aws_secret_access_key": {
      "type": "string",
      "title": "Secret Access Key",
      "description": "AWS secret access key for the bucket",
      "x-secret": true,
      "x-step": "connector",
      "x-visible-if": { "auth_method": "access_keys" }
    },
    "path": {
      "type": "string",
      "title": "S3 URI",
      "pattern": "^s3://",
      "x-step": "source"
    },
    "name": {
      "type": "string",
      "title": "Model name",
      "pattern": "^[a-zA-Z_][a-zA-Z0-9_]*$",
      "x-step": "source"
    }
  },
  "allOf": [
    {
      "if": { "properties": { "auth_method": { "const": "access_keys" } } },
      "then": { "required": ["aws_access_key_id", "aws_secret_access_key"] }
    }
  ]
}

This is illustrative—you'll need to think through the right JSON Schema properties. Try to stay as close to native JSON Schema as possible before reaching for custom x-* extensions. The ones I used above:

• x-secret: mask the input field
• x-step: which step of the multi-step flow this field belongs to
• x-display: UI hint for how to render (radio, select, etc.)
• x-visible-if: conditional visibility based on other field values

There may be better ways to model some of these. Let me know if you want to talk through it.

Developed in collaboration with Claude Code

Added the json schema changes in 83bfcdf

resolved

[ericpgreen2]

Nice work adopting JSON Schema—this is the right direction.

One structural concern: the current flow is JSON Schema → getMultiStepFormConfig() → custom config → MultiStepFormRenderer. The ~200-line getMultiStepFormConfig() function transforms the schema into a different format that the renderer consumes. This defeats the purpose of using JSON Schema—the value is that consumers can interpret the schema directly, without needing a translation layer that duplicates the structure in a different format.

A cleaner approach would be a JSONSchemaFormRenderer that consumes the schema directly—interpreting properties, x-visible-if, allOf/if/then, etc. without the intermediate transformation. This is how libraries like react-jsonschema-form work. The renderer would emit form values via a callback (e.g., onSubmit={(values) => ...}), which the parent can use to call the existing submitAddSourceForm/submitAddConnectorForm functions.

Keep the renderer generic—use slots for context-specific UI outside the form fields themselves: the "Already connected? Import your data" link, connector-specific error messages, etc. That way the same renderer can be reused for non-connector templates later.

For incremental migration, isolate the new pattern in its own top-level feature directory to align with the design doc:

web-common/src/features/templates/
  schemas/
    s3.ts
    gcs.ts
    azure.ts
  JSONSchemaFormRenderer.svelte
  types.ts
  utils.ts

This keeps the new approach cleanly separated and positions it as a general pattern for template-based code generation, not just source connectors.

Optionally, you could also extract the file generation logic from submitAddDataForm.ts into this directory with an interface that mirrors the future backend API:

// Today: implemented client-side
// Tomorrow: calls RuntimeService.GenerateFile
async function generateFile(args: {
  template: string;
  properties: Record<string, unknown>;
  preview?: boolean;
}): Promise<{
  addedPaths: string[];
  modifiedPaths: string[];
  previews: Record<string, string>; // path → YAML
}>

This would make the backend migration a simple swap—same interface, different implementation.

Let me know if you want to talk through any of this.

---

Developed in collaboration with Claude Code

[ericpgreen2]

See above feedback on the JSON Schema implementation

resolved

[ericpgreen2]

Good progress on the JSONSchemaFormRenderer—it's much better now that the intermediate transformation layer is gone.

One more structural issue: the renderer still has connector-specific logic baked in:

• Hardcoded step: "connector" | "source" types
• Auth-specific handling (authMethod, authInfo, authMethodKey)
• Special "Authentication method" label
• Logic that assumes the auth-method-with-nested-fields pattern

This makes it a "connector form renderer" rather than a generic "JSON Schema form renderer."

The principle: The renderer should be a pure function of the schema. It shouldn't know about connectors, sources, or auth methods. It should just:

1. Render fields based on schema properties
2. Filter by x-step if a step prop is passed (but step is just a string, not specifically "connector"/"source")
3. Show/hide fields based on x-visible-if evaluated against current form values
4. Handle x-display: radio for any enum field—not just auth methods

The auth method pattern (radio with nested fields) is just one instance of a general pattern: "enum field with conditionally visible related fields." The renderer should handle that generically.

Connector-specific logic belongs in a parent component. Some naming options:

• ObjectStorageConnectorFlow.svelte - describes what it's for
• MultiStepConnectorForm.svelte - describes the pattern
• ConnectorFormWizard.svelte - wizard pattern

This parent component handles:

• The multi-step flow (connector → source)
• Button labels that change based on auth method
• "Already connected? Import your data" skip link
• Connection testing before advancing steps

The renderer just renders fields; the parent orchestrates the workflow.

Directory structure: The sources/modal/ directory is getting unwieldy. I'd suggest splitting things up:

web-common/src/features/templates/
  JSONSchemaFormRenderer.svelte   # Generic, reusable
  types.ts
  utils.ts
  schemas/
    s3.ts
    gcs.ts
    azure.ts

web-common/src/features/sources/modal/
  ObjectStorageConnectorFlow.svelte   # Connector-specific workflow
  AddDataForm.svelte                   # Existing, uses the flow component
  ...

The generic rendering infrastructure lives in templates/. The connector-specific workflow stays in sources/modal/ and imports from templates/. The schemas live in templates/ since they'll eventually be served from the backend per the design doc.

Let me know if you want to talk through any of this.

---

Developed in collaboration with Claude Code

resolved

[lovincyrus]

The schemasafe implementation looks good - the validation architecture is now clean with JSON Schema as the single source of truth.

Bug: Validation Error Persists After Fixing Input

When testing the source step: after submitting with an invalid path and then correcting it to a valid GCS URI, the validation error persists and I'm unable to submit the form.

The error "Must be a GS URI (e.g. gs://bucket/path)" continues to display even though the input now contains a valid `gs://` URI.

Component Boundaries

Before we merge, I'd like to address the component boundaries. Currently the responsibilities are muddled:

Current state:

• AddDataForm creates AddDataFormManager and SuperForms instances
• Passes formManager, paramsForm, paramsErrors, etc. down to MultiStepConnectorFlow
• AddDataForm subscribes to connectorStepStore and has step-aware logic (YAML preview title, Save Anyway visibility)
• MultiStepConnectorFlow also subscribes to connectorStepStore and has its own auth syncing logic
• The boundary is unclear - parent knows too much about child's internals

Cleaner hierarchy using container/presentational pattern:

The idea is to separate components into two types:

• Container components (smart): Own state, handle side effects, make API calls
• Presentational components (pure): Receive data via props, render UI, no side effects

ConnectorForm (container - owns SuperForms setup, submission, step state)
  └── JSONSchemaFormRenderer (presentational - renders form + fields from schema)
        └── SchemaField (presentational - renders individual field control)

JSONSchemaFormRenderer should probably encompass the <form> element (absorbing AddDataFormSection), making it a complete form renderer that takes schema + form state as props.

For this PR, MultiStepConnectorFlow should become the container component:

1. Own its SuperForms setup internally (with schemasafe) rather than receiving formManager
2. Own step state entirely - AddDataForm should not subscribe to connectorStepStore
3. Expose only what the parent needs: isSubmitDisabled, primaryButtonLabel, yamlPreview, showSaveAnyway
4. Accept minimal props: connector, onClose, onBack

This makes MultiStepConnectorFlow a self-contained component with a clear boundary. The parent becomes a thin orchestrator that just renders the right component based on connector type.

Consider renaming MultiStepConnectorFlow → ConnectorForm since this will become the standard for all connectors once we migrate them to JSON Schema.

Developed in collaboration with Claude Code

Addressed the latest review!

.

[ericpgreen2]

Great work on this, Cyrus! Thank you for iterating through all the feedback rounds - the architecture has evolved into something much cleaner. The JSON Schema approach with schemasafe validation, the clear container/presentational split between ConnectorForm and JSONSchemaFormRenderer, and having AddDataForm no longer coupled to step state internals - this is solid foundation work.

Migration Path Forward

Now that we have this clean JSON Schema-driven architecture, we're well positioned to migrate the remaining connectors and finally clean up the legacy AddDataForm complexity that's been accumulating. Here's what that looks like:

1. DSN Tab Support - Define DSN as a schema-driven option (e.g., x-display: "tabs" or x-input-mode) so JSONSchemaFormRenderer can render Parameters/DSN tabs based on the schema. This keeps ConnectorForm as a thin container that doesn't know about DSN vs parameters.

2. Single-Step Connectors - Database connectors don't have a source step. Schemas with only x-step: "connector" fields should submit and close directly.

3. Change Gating Logic - Replace MULTI_STEP_CONNECTORS.includes(...) with schema existence check: getConnectorSchema(connector.name) !== null

4. ClickHouse Migration - Move to JSON Schema like other connectors to eliminate the forked AddClickHouseForm code path.

Once complete, all connectors flow through ConnectorForm → JSONSchemaFormRenderer and the legacy paths can be removed.

---

Developed in collaboration with Claude Code

[ericpgreen2]

Note that I did not do a final UXQA pass. @royendo, it looks like you may have. From the product perspective, are we good to merge?

[royendo]

bug finds:

• model name error, not sure we added a new rule here. strange behavior only clears after removing numbers and typing something. dont think we need to be strict here.

• save anyways staying visible in model page

• returning to previous page doesnt clear previous credentials

• auth_method: xxxx being added to model

• creating two unique credentials confuses Rill, create_secrets_from_connectors: xxx fixes issue, id say we have this added by default in models to avoid confusion.
  https://www.loom.com/share/24c629ec53dd4137aff427139725cf24?from_recorder=1&focus_title=1
  not sure if what Naman demoed a while back is in this branch, (adding all connectors to DuckDB implicitly) but if it is, it isn't working. If it's not then might be fixed with that PR.

[lovincyrus]

• Change Gating Logic - Replace MULTI_STEP_CONNECTORS.includes(...) with schema existence check: getConnectorSchema(connector.name) !== null
• ClickHouse Migration - Move to JSON Schema like other connectors to eliminate the forked AddClickHouseForm code path.

These are addressed in #8576

[lovincyrus]

1. DSN Tab Support - Define DSN as a schema-driven option (e.g., x-display: "tabs" or x-input-mode) so JSONSchemaFormRenderer can render Parameters/DSN tabs based on the schema. This keeps ConnectorForm as a thin container that doesn't know about DSN vs parameters.

The object storage connectors do not currently have tabs support. This review should be handled in #8576

[lovincyrus]

bug finds:

• model name error, not sure we added a new rule here. strange behavior only clears after removing numbers and typing something. dont think we need to be strict here.

* [x] save anyways staying visible in model page * [x] returning to previous page doesnt clear previous credentials * [x] `auth_method: xxxx` being added to model * [x] creating two unique credentials confuses Rill, `create_secrets_from_connectors: xxx` fixes issue, id say we have this added by default in models to avoid confusion. https://www.loom.com/share/24c629ec53dd4137aff427139725cf24?from_recorder=1&focus_title=1 not sure if what Naman demoed a while back is in this branch, (adding all connectors to DuckDB implicitly) but if it is, it isn't working. If it's not then might be fixed with that PR.

Addressed and added e2e! @royendo

resolved