feat(app): de-hardcode connector type unions through validation/executor/storage (#1121)

[alfredo1996]

Part of v1.2 Connector SDK (epic #1093) — seam gap B4. Branches from release/1.2.

What

Several paths assumed only the built-in 'neo4j' | 'postgresql' union, so a registry-supplied connector type couldn't flow through the app. Now registry types are first-class.

• validation — schemas.ts accepts any registry-registered type via a runtime refine (isRegisteredConnectorType, routed through connection-adapter for test mockability) instead of z.enum(CONNECTOR_TYPES).
• execution — query-executor DbType widened to string; toConnectionTypeEnum maps unknown types to a new ConnectionTypes.UNKNOWN sentinel rather than mislabeling them PostgreSQL. pipeline-types.connectionType + prefetchSchema widened.
• storage — the connection.type column pgEnum → text (migration 0011: enum→text preserves values, then drops the type). The accepted set is enforced at the API layer, not the DB.

Acceptance

• A registry-only (fixture) type validates (schema test), executes (executor resolves its module), and stores (text column), with no core code change per type.

Tests

• schemas.test.ts: a registry-supplied fixture type validates; unregistered rejected.
• query-executor-core.test.ts: a registry type resolves its module via createConnectionModule.
• app tsc + lint clean; connections.spec.ts E2E 15/15 (migration applies + validation + text-column store).

Scope note — CLI intentionally left as-is

A registry/external connector already lists via neoboard-connectors.json. Coupling the published @neoboard/cli to the unpublished @neoboard/connector-sdk would break npm i -g @neoboard/cli. Adding a built-in is itself a core change, so the "no core change per registry type" acceptance holds without touching the CLI's built-in list.

Deferred (follow-up)

The CONNECTOR_LANGUAGES display map + the app/component duplicate DatabaseSchema 'neo4j'|'postgresql' unions remain (separate from validation/executor/storage).

Closes #1121

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Expanded connector support so additional registry-backed connector types can be used across connection setup, schema checks, and query execution.
  • Queries now handle non-built-in connector types more gracefully, improving compatibility with newly added connectors.

• Bug Fixes

  • Improved validation so connector types are checked dynamically, reducing invalid-type errors and making schema validation more reliable.
  • Updated database handling for connector type storage to better accommodate future connector additions.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e57ad368-dadb-40e5-86bc-dc3085b6f135

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Connector type handling is converted from hardcoded enums/unions to registry-backed strings. New adapter exports and helper functions check registry membership; validation schemas, query executor, pipeline types, schema-prefetch, and the DB schema column are widened to plain strings; ConnectionTypes gains an UNKNOWN member; tests updated accordingly.

Changes

Registry-backed connector types

Layer / File(s)	Summary
ConnectionTypes UNKNOWN member connector-sdk/src/ConnectionModuleConfig.ts	Adds ConnectionTypes.UNKNOWN = 0 with explanatory comment.
Registry lookup helpers app/src/lib/connector/connection-adapter.ts, app/src/lib/connector/registered-types.ts	Adapter re-exports getConnector/getAllConnectors; new module adds isRegisteredConnectorType and registeredConnectorTypes built on those exports.
Validation via registry check app/src/lib/shared/schemas.ts, app/src/lib/__tests__/shared/schemas.test.ts	connectorTypeSchema replaces z.enum(CONNECTOR_TYPES) in createConnectionSchema/testInlineSchema, refined via isRegisteredConnectorType; tests mock the registry and verify a registry-only type validates.
Executor and pipeline widening to string app/src/lib/query/query-executor.ts, app/src/lib/query/pipeline-types.ts, app/src/lib/connector/schema-prefetch.ts, app/src/lib/__tests__/query/query-executor-core.test.ts	DbType and QueryContext.connectionType become string; toConnectionTypeEnum maps only "neo4j"/"postgresql" explicitly and returns UNKNOWN otherwise; schema-prefetch functions take string type; tests mock ConnectionTypes.UNKNOWN and add a registry-type executor test.
DB schema type column widened app/src/lib/db/schema.ts	Removes connectionTypeEnum; connections.type becomes an unconstrained text column, with validation moved to the API layer.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant SchemaValidation as connectorTypeSchema
  participant RegisteredTypes as registered-types.ts
  participant Adapter as connection-adapter.ts
  participant Executor as query-executor.ts

  Client->>SchemaValidation: submit connection type (e.g. "mysql")
  SchemaValidation->>RegisteredTypes: isRegisteredConnectorType(type)
  RegisteredTypes->>Adapter: getConnector(type)
  Adapter-->>RegisteredTypes: connector found
  RegisteredTypes-->>SchemaValidation: true
  SchemaValidation-->>Client: validation success
  Client->>Executor: executeQuery(type)
  Executor->>Executor: toConnectionTypeEnum(type) -> UNKNOWN
  Executor->>Adapter: createConnectionModule(type, connectionType)
  Adapter-->>Executor: connection module
  Executor-->>Client: query result

Loading

Possibly related PRs

• alfredo1996/neoboard#393: Introduces the connector registry API (getConnector/getAllConnectors/createConnectionModule) consumed here for registry-supplied types.
• alfredo1996/neoboard#594: Removes the as DbType cast on connectionType when calling executeQuery, aligning with DbType becoming a plain string.
• alfredo1996/neoboard#1136: Also modifies fetchConnectionSchema/prefetchSchema in schema-prefetch.ts, switching dispatch to registry-based getSchemaManager.

Suggested labels: testing, refactor, area:query-exec

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Linked Issues check	⚠️ Warning	Validation and execution are covered, but the required CLI registry-list behavior is absent from the reviewed changes.	Add cli/src/commands/plugin.ts support and a unit test proving registry types are listed for #1121.,
Out of Scope Changes check	⚠️ Warning	The db schema change to store connection.type as text is outside the linked issue scope of validation, execution, and CLI listing.	Remove the storage-layer schema change or update the linked issue scope if storage is intended work.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title is concise and matches the connector-type de-hardcoding work, though it says storage instead of the missing CLI scope.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/issue-1121-dehardcode-type-unions

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

app/src/lib/__tests__/query/query-executor-core.test.ts (1)

114-130: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Assert the UNKNOWN config path in this regression test.

This only proves "mysql" reaches createConnectionModule. It does not verify the behavior changed in query-executor.ts: non-built-in types should hit runQuery with connectionType: ConnectionTypes.UNKNOWN. A regression back to PostgreSQL would still pass here.

Suggested assertion

   await executeQuery("mysql", pgCreds, { query: "SELECT 1" });

   expect(mockCreateConnectionModule).toHaveBeenCalledWith(
     "mysql",
     expect.objectContaining({ uri: pgCreds.uri }),
     expect.any(Object),
   );
+  expect(mockRunQuery).toHaveBeenCalledWith(
+    { query: "SELECT 1" },
+    expect.any(Object),
+    expect.objectContaining({ connectionType: 0 }),
+  );

🤖 Prompt for 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.

In `@app/src/lib/__tests__/query/query-executor-core.test.ts` around lines 114 -
130, Update the regression test in the executeQuery flow so it asserts the
non-built-in connector is passed to runQuery with connectionType set to
ConnectionTypes.UNKNOWN, not just that createConnectionModule receives "mysql".
Use the existing executeQuery, mockRunQuery, and mockCreateConnectionModule
setup to verify the args sent to runQuery include the UNKNOWN config path,
ensuring query-executor.ts cannot regress to PostgreSQL behavior unnoticed.

🤖 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.

Nitpick comments:
In `@app/src/lib/__tests__/query/query-executor-core.test.ts`:
- Around line 114-130: Update the regression test in the executeQuery flow so it
asserts the non-built-in connector is passed to runQuery with connectionType set
to ConnectionTypes.UNKNOWN, not just that createConnectionModule receives
"mysql". Use the existing executeQuery, mockRunQuery, and
mockCreateConnectionModule setup to verify the args sent to runQuery include the
UNKNOWN config path, ensuring query-executor.ts cannot regress to PostgreSQL
behavior unnoticed.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 2563e31d-18df-4782-837a-fef5e750b7ad

📥 Commits

Reviewing files that changed from the base of the PR and between f7d1f68 and f2c08e0.

⛔ Files ignored due to path filters (3)

• app/drizzle/migrations/0011_closed_captain_flint.sql is excluded by !app/drizzle/migrations/**
• app/drizzle/migrations/meta/0011_snapshot.json is excluded by !app/drizzle/migrations/**
• app/drizzle/migrations/meta/_journal.json is excluded by !app/drizzle/migrations/**

📒 Files selected for processing (10)

• app/src/lib/__tests__/query/query-executor-core.test.ts
• app/src/lib/__tests__/shared/schemas.test.ts
• app/src/lib/connector/connection-adapter.ts
• app/src/lib/connector/registered-types.ts
• app/src/lib/connector/schema-prefetch.ts
• app/src/lib/db/schema.ts
• app/src/lib/query/pipeline-types.ts
• app/src/lib/query/query-executor.ts
• app/src/lib/shared/schemas.ts
• connector-sdk/src/ConnectionModuleConfig.ts

[sonarqubecloud]

Quality Gate passed

Issues
3 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud