Introduce support to override connector dependencies in descriptor.yml#2130
Open
Introduce support to override connector dependencies in descriptor.yml#2130
Conversation
Contributor
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (14)
📝 WalkthroughWalkthroughAdds RPC APIs, types, client/server bindings, and UI changes to support connector-effective dependency resolution, overrides, omit/reset flags, and connector-scoped/global flag updates; moves driver download and form wiring from Maven coordinates to connectorName/connectionType inputs. Changes
Sequence DiagramsequenceDiagram
actor User
participant FormUI as Form UI
participant ExtClient as Extended Language Client
participant Extension as MI Extension (RPC Manager)
participant Config as Connector Config Store
participant FS as File System
User->>FormUI: Open connector form
FormUI->>ExtClient: getConnectorDependencies({connectorName, connectionType})
ExtClient->>Extension: sendRequest(getConnectorDependencies)
Extension->>FS: read connector-config.json
FS-->>Extension: effective dependencies
Extension-->>ExtClient: GetConnectorDependenciesResponse
ExtClient-->>FormUI: dependency data
User->>FormUI: Update override / omit / localPath
FormUI->>ExtClient: updateConnectorDependencyOverride(...) / updateConnectorFlags(...)
ExtClient->>Extension: sendRequest(update...)
Extension->>FS: persist updated connector-config.json
FS-->>Extension: success
Extension-->>ExtClient: boolean (true)
ExtClient-->>FormUI: confirmation -> refresh UI
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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. Review rate limit: 0/1 reviews remaining, refill in 23 minutes and 48 seconds.Comment |
![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: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@workspaces/mi/mi-diagram/src/components/Form/FormGenerator.tsx`:
- Around line 1397-1414: The UI ignores effectiveDriverDep?.omit and still
renders the normal driver info; update the render logic in FormGenerator (the
block computing isLocalJar, isOverridden, artifactLabel, versionLabel,
driverLabel) to detect an omitted dependency (e.g., const isOmitted =
!!effectiveDriverDep?.omit) and when isOmitted render an explicit omitted state
(a clear badge or message like "omitted" instead of the driverLabel and any
overridden/local badges). Ensure the omitted branch takes precedence over
isLocalJar/isOverridden so the component shows the omitted badge/message
whenever effectiveDriverDep.omit is true.
In `@workspaces/mi/mi-extension/src/project-explorer/activate.ts`:
- Around line 403-417: The command handlers are using the captured projectUri
instead of resolving the target workspace for the clicked item, causing actions
to run against the wrong project in multi-root workspaces. Update the handlers
registered for COMMANDS.MANAGE_CONNECTOR_DEPENDENCIES,
COMMANDS.OVERRIDE_CONNECTOR_DEPENDENCY and COMMANDS.RESET_CONNECTOR_DEPENDENCY
to derive the correct project/workspace folder from the passed-in
ProjectExplorerEntry (use the item's URI/resource or workspace folder lookup) at
invocation time, then pass that derived projectUri into
manageConnectorDependencies, overrideConnectorDependency and
resetConnectorDependency instead of the captured projectUri.
In `@workspaces/mi/mi-extension/src/rpc-managers/mi-diagram/rpc-manager.ts`:
- Around line 3937-3949: The connector config initialization is missing after
copying a connector and several functions use the async promise-executor
anti-pattern. Create a small helper (e.g., ensureConnectorConfig(projectUri)
that does: const langClient = await MILanguageClient.getInstance(projectUri);
await langClient.initConnectorConfig(projectUri); then invoke that helper from
both copyConnectorZip(...) and downloadConnector(...). In downloadConnector,
replace the async promise executor with a non-async executor (new
Promise((resolve, reject) => { ... })) and wrap the finish handler body in
try/catch so any error calls reject(err) (and reject if initConnectorConfig
throws). Finally, locate the five RPC connector dependency/flag methods that
currently use new Promise(async (resolve) => ...) and refactor them to use new
Promise((resolve, reject) => { try { /* await work via async helper and then
resolve */ } catch(e) { reject(e) } }) so rejections are propagated.
- Around line 6600-6638: The listed RPC methods (getConnectorDependencies,
updateConnectorDependencyOverride, resetConnectorDependencyOverrides,
updateConnectorFlags, updateGlobalConnectorFlags) wrap an async executor in new
Promise which swallows rejections; remove the unnecessary new Promise(async
(...) => { ... }) wrappers, call MILanguageClient.getInstance(this.projectUri)
and then directly return/await the delegated langClient methods (e.g. return
await langClient.getConnectorDependencies(params) or return
langClient.getConnectorDependencies(params)) so errors from
MILanguageClient.getInstance or the delegated calls propagate instead of leaving
the outer promise pending.
In
`@workspaces/mi/mi-visualizer/src/views/Overview/ProjectInformation/DependencyItem.tsx`:
- Around line 225-231: The props onEdit and onDelete are declared optional but
are invoked unguarded, causing runtime errors; either make them required in the
DependencyItem props (remove the ? from onEdit and onDelete) or add null checks
where they are used and only render the Edit/Delete buttons when the callbacks
are present. Specifically update the prop signature lines declaring onEdit and
onDelete, and modify the handlers or JSX that call them (the edit/update/delete
flows and corresponding button render logic) to guard calls with if (onEdit) /
if (onDelete) or conditional rendering. Apply the same change to the other
occurrences mentioned that call these callbacks unconditionally.
- Around line 296-310: The handler starts with setIsSaving(true) but clears it
only on the success path; wrap the RPC call and subsequent updates in a
try/finally so setIsSaving(false) always executes (e.g., in
handleDriverVersionSave move the await rpcClient... and
setDriverEditState/onDriverUpdated calls into a try block and put
setIsSaving(false) in finally); also apply the same try/finally pattern to the
other mutation/save handlers in this file that call setIsSaving(true) so the UI
is never left stuck if the RPC rejects.
- Around line 456-467: The comparison latestVersion > dependency.version in
DependencyItem does string (lexicographic) comparison; change it to use semantic
version comparison (e.g., use the semver package or a small comparator) to
determine if latestVersion is a newer semver than dependency.version, handling
null/undefined and prerelease/build metadata; update the conditional that
renders the update badge and the onClick payload (onEdit call) to rely on the
semver check (use symbols latestVersion, dependency.version, DependencyItem
component, and onEdit) so "Update available" and quick-update only appear when
semver.compare(latestVersion, dependency.version) indicates a newer version.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0c4e19a3-fe8c-4d78-b909-6aa25d2b1ade
📒 Files selected for processing (17)
workspaces/mi/mi-core/src/rpc-types/mi-diagram/index.tsworkspaces/mi/mi-core/src/rpc-types/mi-diagram/rpc-type.tsworkspaces/mi/mi-core/src/rpc-types/mi-diagram/types.tsworkspaces/mi/mi-diagram/src/components/Form/DBConnector/DriverConfiguration.tsxworkspaces/mi/mi-diagram/src/components/Form/DynamicFields/DynamicFieldsHandler.tsxworkspaces/mi/mi-diagram/src/components/Form/FormGenerator.tsxworkspaces/mi/mi-extension/package.jsonworkspaces/mi/mi-extension/schemas/connector-config-schema.jsonworkspaces/mi/mi-extension/src/constants/index.tsworkspaces/mi/mi-extension/src/lang-client/ExtendedLanguageClient.tsworkspaces/mi/mi-extension/src/project-explorer/activate.tsworkspaces/mi/mi-extension/src/rpc-managers/mi-diagram/rpc-handler.tsworkspaces/mi/mi-extension/src/rpc-managers/mi-diagram/rpc-manager.tsworkspaces/mi/mi-rpc-client/src/rpc-clients/mi-diagram/rpc-client.tsworkspaces/mi/mi-visualizer/src/views/Forms/ConnectionForm/ConnectionFormGenerator.tsxworkspaces/mi/mi-visualizer/src/views/Overview/ProjectInformation/DependencyItem.tsxworkspaces/mi/mi-visualizer/src/views/Overview/ProjectInformation/DependencyManager.tsx
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a connector-driver dependency override/omit workflow backed by a new connector-config.json, exposed via new MiDiagram RPCs and surfaced in both the Project Overview dependency UI and connection forms (read-only driver display + persistence of overrides).
Changes:
- Introduces new RPC types + MiDiagram API endpoints to read/update effective connector dependency overrides and omit flags.
- Updates the MI Visualizer dependency UI to manage per-connector driver overrides (version/local JAR/omit) and a global “omit all drivers” flag (runtime >= 4.4.0 for zip deps).
- Adds
connector-config.jsonJSON schema validation and uses it to pre-populate/persist driver coordinates from connection forms.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| workspaces/mi/mi-visualizer/src/views/Overview/ProjectInformation/DependencyManager.tsx | Enables driver-management UI (global omit flag + per-connector driver data wiring) gated by runtime version. |
| workspaces/mi/mi-visualizer/src/views/Overview/ProjectInformation/DependencyItem.tsx | Adds expandable driver panel with override/omit/local JAR actions and connector-level omit toggles. |
| workspaces/mi/mi-visualizer/src/views/Forms/ConnectionForm/ConnectionFormGenerator.tsx | Pre-populates driver fields from connector-config and persists overrides there instead of local entries. |
| workspaces/mi/mi-rpc-client/src/rpc-clients/mi-diagram/rpc-client.ts | Exposes new MiDiagram RPC client methods for connector dependency management. |
| workspaces/mi/mi-extension/src/rpc-managers/mi-diagram/rpc-manager.ts | Ensures connector-config initialization when downloading connectors; proxies new dependency-management RPCs to LS. |
| workspaces/mi/mi-extension/src/rpc-managers/mi-diagram/rpc-handler.ts | Registers new messenger request handlers for connector dependency RPCs. |
| workspaces/mi/mi-extension/src/project-explorer/activate.ts | Adds command handlers + quick-pick UX to override/omit/reset connector dependencies. |
| workspaces/mi/mi-extension/src/lang-client/ExtendedLanguageClient.ts | Adds LS request/notification wrappers for connector dependency operations and config initialization. |
| workspaces/mi/mi-extension/src/constants/index.ts | Adds command IDs for connector dependency management actions. |
| workspaces/mi/mi-extension/schemas/connector-config-schema.json | New JSON schema for connector-config override/omit structure. |
| workspaces/mi/mi-extension/package.json | Registers JSON validation for connector-config.json using the new schema. |
| workspaces/mi/mi-diagram/src/components/Form/FormGenerator.tsx | Replaces in-form driver configuration UI with effective-driver display + pointer to dependency manager. |
| workspaces/mi/mi-diagram/src/components/Form/DynamicFields/DynamicFieldsHandler.tsx | Updates driver download request shape to use connectorName/connectionType. |
| workspaces/mi/mi-diagram/src/components/Form/DBConnector/DriverConfiguration.tsx | Tweaks driver configuration button labels/availability (legacy UI pieces). |
| workspaces/mi/mi-core/src/rpc-types/mi-diagram/types.ts | Adds connector dependency override/flag types and updates DriverDownloadRequest shape. |
| workspaces/mi/mi-core/src/rpc-types/mi-diagram/rpc-type.ts | Declares new RPC request types for connector dependency operations. |
| workspaces/mi/mi-core/src/rpc-types/mi-diagram/index.ts | Extends MiDiagramAPI interface with connector dependency operations. |
Comments suppressed due to low confidence (1)
workspaces/mi/mi-diagram/src/components/Form/DynamicFields/DynamicFieldsHandler.tsx:669
downloadDriverForConnectorreturns aDriverDownloadResponseobject (e.g.,{ driverPath }), but the code assigns it directly to thedriverPathstring variable. This will causedriverPathto become an object and later be passed intoloadDriverAndTestConnection/ stored in connection params incorrectly. Extract and useres.driverPathinstead (and consider handling an empty/undefined driverPath response).
const args = {
connectorName: connectorName,
connectionType: connectionType,
};
this.setCustomError(getNameForController(FIELD_NAMES.CONFIG_KEY), "Checking DB Driver...");
driverPath = await this.rpcClient.getMiDiagramRpcClient().downloadDriverForConnector(args);
if (driverPath) {
isDriverDownloaded = true;
}
retryCount++;
}
if (!isDriverDownloaded) {
this.setCustomError(getNameForController(FIELD_NAMES.CONFIG_KEY), "Failed to download the DB driver after 5 attempts.");
}
}
connection.parameters.push({
name: FIELD_NAMES.DRIVER_PATH,
value: driverPath,
});
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
arunans23
force-pushed
the
driver
branch
2 times, most recently
from
731f3de to
e478a73
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
workspaces/mi/mi-extension/src/rpc-managers/mi-diagram/rpc-manager.ts (2)
6598-6636:⚠️ Potential issue | 🟠 MajorRemove
new Promise(async ...)in the new connector dependency RPC methods.At Lines 6599, 6607, 6615, 6623, and 6631, async promise executors can swallow rejections and leave callers waiting indefinitely. These methods are already
async; return delegated calls directly.💡 Suggested refactor
async getConnectorDependencies(params: GetConnectorDependenciesRequest): Promise<GetConnectorDependenciesResponse> { - return new Promise(async (resolve) => { - const langClient = await MILanguageClient.getInstance(this.projectUri); - const res = await langClient.getConnectorDependencies(params); - resolve(res); - }); + const langClient = await MILanguageClient.getInstance(this.projectUri); + return langClient.getConnectorDependencies(params); } async updateConnectorDependencyOverride(params: UpdateConnectorDependencyOverrideRequest): Promise<boolean> { - return new Promise(async (resolve) => { - const langClient = await MILanguageClient.getInstance(this.projectUri); - const res = await langClient.updateConnectorDependencyOverride(params); - resolve(res); - }); + const langClient = await MILanguageClient.getInstance(this.projectUri); + return langClient.updateConnectorDependencyOverride(params); } async resetConnectorDependencyOverrides(params: ResetConnectorDependencyOverridesRequest): Promise<boolean> { - return new Promise(async (resolve) => { - const langClient = await MILanguageClient.getInstance(this.projectUri); - const res = await langClient.resetConnectorDependencyOverrides(params); - resolve(res); - }); + const langClient = await MILanguageClient.getInstance(this.projectUri); + return langClient.resetConnectorDependencyOverrides(params); } async updateConnectorFlags(params: UpdateConnectorFlagsRequest): Promise<boolean> { - return new Promise(async (resolve) => { - const langClient = await MILanguageClient.getInstance(this.projectUri); - const res = await langClient.updateConnectorFlags(params); - resolve(res); - }); + const langClient = await MILanguageClient.getInstance(this.projectUri); + return langClient.updateConnectorFlags(params); } async updateGlobalConnectorFlags(params: UpdateGlobalConnectorFlagsRequest): Promise<boolean> { - return new Promise(async (resolve) => { - const langClient = await MILanguageClient.getInstance(this.projectUri); - const res = await langClient.updateGlobalConnectorFlags(params); - resolve(res); - }); + const langClient = await MILanguageClient.getInstance(this.projectUri); + return langClient.updateGlobalConnectorFlags(params); }#!/bin/bash # Verify async Promise executor anti-pattern in the new methods rg -nP "new Promise\(async\s*\(resolve" workspaces/mi/mi-extension/src/rpc-managers/mi-diagram/rpc-manager.ts sed -n '6598,6636p' workspaces/mi/mi-extension/src/rpc-managers/mi-diagram/rpc-manager.ts🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workspaces/mi/mi-extension/src/rpc-managers/mi-diagram/rpc-manager.ts` around lines 6598 - 6636, The five methods getConnectorDependencies, updateConnectorDependencyOverride, resetConnectorDependencyOverrides, updateConnectorFlags, and updateGlobalConnectorFlags wrap async work in "new Promise(async (resolve) => { ... })", which can swallow rejections; remove the unnecessary Promise wrapper and instead directly await (or return) the delegated call to MILanguageClient.getInstance(this.projectUri) and then call the corresponding langClient method (e.g., langClient.getConnectorDependencies(params)) returning its result so errors propagate normally.
3931-3940:⚠️ Potential issue | 🔴 CriticalHandle async
finishcallback failures to avoid a stuck promise.At Line 3931,
writer.on('finish', async () => ...)can leave the outer promise pending if Line 3934 or Line 3939 throws/rejects, becausereject(...)is never called in that path.💡 Suggested fix
- return new Promise((resolve, reject) => { - writer.on('finish', async () => { - writer.close(); - // Copy the file from the temp location to the connectorPath - await copy(tmpobj.name, connectorPath); - // Remove the temporary file - tmpobj.removeCallback(); - // Ensure connector-config.json exists for this project - const langClient = await MILanguageClient.getInstance(this.projectUri); - await langClient.initConnectorConfig(this.projectUri); - resolve({ path: connectorPath }); - }); - writer.on('error', reject); - }); + return new Promise((resolve, reject) => { + writer.on('finish', () => { + (async () => { + try { + writer.close(); + await copy(tmpobj.name, connectorPath); + tmpobj.removeCallback(); + const langClient = await MILanguageClient.getInstance(this.projectUri); + await langClient.initConnectorConfig(this.projectUri); + resolve({ path: connectorPath }); + } catch (err) { + reject(err); + } + })(); + }); + writer.on('error', reject); + });#!/bin/bash # Verify async finish-handler usage in promise context rg -nP "writer\.on\('finish',\s*async" workspaces/mi/mi-extension/src/rpc-managers/mi-diagram/rpc-manager.ts sed -n '3928,3944p' workspaces/mi/mi-extension/src/rpc-managers/mi-diagram/rpc-manager.ts🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workspaces/mi/mi-extension/src/rpc-managers/mi-diagram/rpc-manager.ts` around lines 3931 - 3940, The async finish handler registered with writer.on('finish', async () => { ... }) can throw and leave the outer Promise unresolved; wrap the body (calls to copy(tmpobj.name, connectorPath), tmpobj.removeCallback(), MILanguageClient.getInstance(this.projectUri), and langClient.initConnectorConfig(...)) in a try/catch and call the Promise's reject(error) on error, and use a finally block to always call tmpobj.removeCallback() so temporary cleanup always runs; update the handler around writer.on('finish', async () => { ... }) to ensure resolve({ path: connectorPath }) is only called on success and reject(err) is invoked on failures.
🧹 Nitpick comments (1)
workspaces/mi/mi-visualizer/src/views/Overview/ProjectInformation/DependencyItem.tsx (1)
297-434: Consider extracting a shared driver-mutation wrapper.Line 297-Line 434 repeats the same
isSaving/RPC/refresh/finally pattern in many handlers. A single helper would reduce drift and future regressions.Refactor sketch
+ const runDriverMutation = async (mutation: () => Promise<void>) => { + setIsSaving(true); + try { + await mutation(); + await onDriverUpdated?.(); + } finally { + setIsSaving(false); + } + }; ... - setIsSaving(true); - try { - await rpcClient.getMiDiagramRpcClient().updateConnectorDependencyOverride({ ... }); - setDriverEditState(null); - await onDriverUpdated?.(); - } finally { - setIsSaving(false); - } + await runDriverMutation(async () => { + await rpcClient.getMiDiagramRpcClient().updateConnectorDependencyOverride({ ... }); + setDriverEditState(null); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workspaces/mi/mi-visualizer/src/views/Overview/ProjectInformation/DependencyItem.tsx` around lines 297 - 434, The handlers (handleDriverVersionSave, handleBrowseLocalJar, handleClearLocalJar, handleDriverOmitConfirm, handleDriverReset, handleOmitAllDriversConfirm, handleResetAllDrivers, handleConnectorOmitToggle) repeat the same setIsSaving/try-await-rpc/await onDriverUpdated?.()/finally setIsSaving(false) pattern; extract a helper like runDriverMutation(asyncMutator: () => Promise<void>) that does setIsSaving(true); try { await asyncMutator(); await onDriverUpdated?.(); } finally { setIsSaving(false); } and then replace each handler body with a call to runDriverMutation(() => rpcClient.getMiDiagramRpcClient().<...>) (preserve any pre-checks like confirmOmitDriver or askDriverPath and any post-actions like setConfirmOmitDriver/setConfirmOmitAllDrivers) so the mutation and refresh logic is centralized.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@workspaces/mi/mi-diagram/src/components/Form/FormGenerator.tsx`:
- Around line 295-308: When connectorArtifactId (cn) or connectionName (ct) are
missing or when getConnectorDependencies throws, reset effectiveDriverDep to
null so stale driver state isn't shown; update the early-return path and the
catch block around
rpcClient.getMiDiagramRpcClient().getConnectorDependencies(...) to call
setEffectiveDriverDep(null), referencing the existing variables cn, ct and the
setter setEffectiveDriverDep to ensure the component clears any previous
dependency state on missing prerequisites or lookup failure.
In
`@workspaces/mi/mi-visualizer/src/views/Forms/ConnectionForm/ConnectionFormGenerator.tsx`:
- Around line 293-307: The current flow commits the connection via
createConnection before calling updateConnectorDependencyOverride, which can
leave a partially-saved state if the override call fails; change the sequence or
add compensating logic: either call updateConnectorDependencyOverride before
invoking createConnection (preferred), or wrap updateConnectorDependencyOverride
in a try/catch and on failure perform a rollback of the just-created connection
by calling the appropriate RPC undo/delete using the createConnection response
(or at minimum convert the override error into a non-fatal warning and log it
via process/logger instead of throwing), and apply the same fix for the other
occurrence around updateConnectorDependencyOverride in the 352-367 block; use
the unique symbols updateConnectorDependencyOverride and createConnection to
find where to reorder or add rollback/try-catch logic.
In
`@workspaces/mi/mi-visualizer/src/views/Overview/ProjectInformation/DependencyItem.tsx`:
- Around line 717-719: The confirm buttons in the DialogActions should be
disabled while the async mutation is in-flight to prevent duplicate RPC writes:
update the component to track the mutation loading state (use the existing
mutation hook's isLoading/isMutating flag or add one) and pass
disabled={isLoading} (or similar) to both the Cancel and Omit Buttons rendered
in the block that calls handleDriverOmitConfirm, and repeat the same change for
the other dialog instance found around the handleDriverOmitConfirm usage at the
other occurrence (lines referenced in the review). Additionally, add an
early-return guard inside handleDriverOmitConfirm to ignore calls when the
mutation is already running (check the same isLoading flag) so rapid re-clicks
are ignored even if the UI state lags.
---
Duplicate comments:
In `@workspaces/mi/mi-extension/src/rpc-managers/mi-diagram/rpc-manager.ts`:
- Around line 6598-6636: The five methods getConnectorDependencies,
updateConnectorDependencyOverride, resetConnectorDependencyOverrides,
updateConnectorFlags, and updateGlobalConnectorFlags wrap async work in "new
Promise(async (resolve) => { ... })", which can swallow rejections; remove the
unnecessary Promise wrapper and instead directly await (or return) the delegated
call to MILanguageClient.getInstance(this.projectUri) and then call the
corresponding langClient method (e.g.,
langClient.getConnectorDependencies(params)) returning its result so errors
propagate normally.
- Around line 3931-3940: The async finish handler registered with
writer.on('finish', async () => { ... }) can throw and leave the outer Promise
unresolved; wrap the body (calls to copy(tmpobj.name, connectorPath),
tmpobj.removeCallback(), MILanguageClient.getInstance(this.projectUri), and
langClient.initConnectorConfig(...)) in a try/catch and call the Promise's
reject(error) on error, and use a finally block to always call
tmpobj.removeCallback() so temporary cleanup always runs; update the handler
around writer.on('finish', async () => { ... }) to ensure resolve({ path:
connectorPath }) is only called on success and reject(err) is invoked on
failures.
---
Nitpick comments:
In
`@workspaces/mi/mi-visualizer/src/views/Overview/ProjectInformation/DependencyItem.tsx`:
- Around line 297-434: The handlers (handleDriverVersionSave,
handleBrowseLocalJar, handleClearLocalJar, handleDriverOmitConfirm,
handleDriverReset, handleOmitAllDriversConfirm, handleResetAllDrivers,
handleConnectorOmitToggle) repeat the same setIsSaving/try-await-rpc/await
onDriverUpdated?.()/finally setIsSaving(false) pattern; extract a helper like
runDriverMutation(asyncMutator: () => Promise<void>) that does
setIsSaving(true); try { await asyncMutator(); await onDriverUpdated?.(); }
finally { setIsSaving(false); } and then replace each handler body with a call
to runDriverMutation(() => rpcClient.getMiDiagramRpcClient().<...>) (preserve
any pre-checks like confirmOmitDriver or askDriverPath and any post-actions like
setConfirmOmitDriver/setConfirmOmitAllDrivers) so the mutation and refresh logic
is centralized.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 46247d7d-c252-4176-a9ab-246a3d8ecefa
📒 Files selected for processing (6)
workspaces/mi/mi-diagram/src/components/Form/FormGenerator.tsxworkspaces/mi/mi-extension/src/constants/index.tsworkspaces/mi/mi-extension/src/project-explorer/activate.tsworkspaces/mi/mi-extension/src/rpc-managers/mi-diagram/rpc-manager.tsworkspaces/mi/mi-visualizer/src/views/Forms/ConnectionForm/ConnectionFormGenerator.tsxworkspaces/mi/mi-visualizer/src/views/Overview/ProjectInformation/DependencyItem.tsx
💤 Files with no reviewable changes (1)
- workspaces/mi/mi-extension/src/constants/index.ts
✅ Files skipped from review due to trivial changes (1)
- workspaces/mi/mi-extension/src/project-explorer/activate.ts
arunans23
force-pushed
the
driver
branch
3 times, most recently
from
32275fc to
4f3ef54
Compare
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (4)
workspaces/mi/mi-visualizer/src/views/Overview/ProjectInformation/DependencyItem.tsx (1)
351-368:⚠️ Potential issue | 🟡 MinorBlock dialog re-entry while omit mutations are running.
These confirm dialogs are still clickable while
isSavingis true, and the handlers do not guard against re-entry. Rapid re-clicks can queue duplicate omit RPCs before the dialog closes.Suggested fix
const handleDriverOmitConfirm = async (confirmed: boolean) => { + if (isSaving) return; if (confirmed && confirmOmitDriver) { setIsSaving(true); try { await rpcClient.getMiDiagramRpcClient().updateConnectorDependencyOverride({ connectorArtifactId, @@ const handleOmitAllDriversConfirm = async (confirmed: boolean) => { + if (isSaving) return; if (confirmed) { setIsSaving(true); try { // Reset individual overrides first, then set the connector-level flag. // Order matters: resetConnectorDependencyOverrides (no connectionType) removes the @@ - <Button appearance="secondary" onClick={() => handleDriverOmitConfirm(false)}>Cancel</Button> - <Button appearance="primary" onClick={() => handleDriverOmitConfirm(true)}>Omit</Button> + <Button appearance="secondary" onClick={() => handleDriverOmitConfirm(false)} disabled={isSaving}>Cancel</Button> + <Button appearance="primary" onClick={() => handleDriverOmitConfirm(true)} disabled={isSaving}>Omit</Button> @@ - <Button appearance="secondary" onClick={() => handleOmitAllDriversConfirm(false)}>Cancel</Button> - <Button appearance="primary" onClick={() => handleOmitAllDriversConfirm(true)}>Omit All</Button> + <Button appearance="secondary" onClick={() => handleOmitAllDriversConfirm(false)} disabled={isSaving}>Cancel</Button> + <Button appearance="primary" onClick={() => handleOmitAllDriversConfirm(true)} disabled={isSaving}>Omit All</Button>Also applies to: 385-405, 717-719, 734-736
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workspaces/mi/mi-visualizer/src/views/Overview/ProjectInformation/DependencyItem.tsx` around lines 351 - 368, The confirm handler handleDriverOmitConfirm can be re-entered while an RPC is in flight; add a re-entry guard and close the dialog immediately to prevent duplicate RPCs: at the top of handleDriverOmitConfirm return early if isSaving is true, and as soon as a confirmed action is detected call setConfirmOmitDriver(null) before awaiting the RPC; keep setIsSaving(true) before the RPC and the existing try/finally to setIsSaving(false) after, and apply the same pattern to the other confirm handlers referenced (the handlers around lines 385-405, 717-719, 734-736) so dialogs are closed immediately and further clicks are ignored while isSaving is true.workspaces/mi/mi-diagram/src/components/Form/FormGenerator.tsx (1)
294-311:⚠️ Potential issue | 🟠 MajorClear stale driver state and ignore out-of-order lookups.
Line 297 and Line 306 still leave
effectiveDriverDepunchanged when prerequisites are missing or the RPC fails, so the UI can keep showing the previous connector's driver status. This effect also has no stale-request guard, so a slower earlier lookup can overwrite a newer selection.Suggested fix
useEffect(() => { + let cancelled = false; const loadEffectiveDriver = async () => { const cn = connectorArtifactIdProp ?? formData?.connectorName ?? connectorName?.replace(/\s/g, ''); const ct = connectionName; - if (!cn || !ct) return; + if (!cn || !ct) { + setEffectiveDriverDep(null); + return; + } try { const resp = await rpcClient.getMiDiagramRpcClient().getConnectorDependencies({ connectorArtifactId: cn, }); const dep = resp?.dependencies?.find( d => d.connectionType?.toLowerCase() === ct.toLowerCase() ); - setEffectiveDriverDep(dep ?? null); + if (!cancelled) { + setEffectiveDriverDep(dep ?? null); + } } catch { // non-DB connector or deps not available — silently ignore + if (!cancelled) { + setEffectiveDriverDep(null); + } } }; loadEffectiveDriver(); + return () => { + cancelled = true; + }; }, [connectorArtifactIdProp, formData?.connectorName, connectorName, connectionName]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workspaces/mi/mi-diagram/src/components/Form/FormGenerator.tsx` around lines 294 - 311, The effect's loadEffectiveDriver must clear stale state and guard against out-of-order responses: when connectorArtifactIdProp/formData?.connectorName/connectorName or connectionName are missing, call setEffectiveDriverDep(null) before returning; inside loadEffectiveDriver create a local request token (e.g., incrementing requestId or a cancelled boolean) and capture it so that after awaiting rpcClient.getMiDiagramRpcClient().getConnectorDependencies(...) you only call setEffectiveDriverDep(dep ?? null) if the token is still current; also catch errors and call setEffectiveDriverDep(null) in the catch to clear state on failure. Ensure references to loadEffectiveDriver, setEffectiveDriverDep, and getConnectorDependencies are used to locate the changes.workspaces/mi/mi-extension/src/rpc-managers/mi-diagram/rpc-manager.ts (2)
3931-3940:⚠️ Potential issue | 🔴 CriticalPropagate failures from the
finishhandler.Line 3931 still uses an async
'finish'listener. Ifcopy(...),MILanguageClient.getInstance(...), orinitConnectorConfig(...)rejects, this outer promise never settles and the connector flow hangs.Suggested fix
- return new Promise((resolve, reject) => { - writer.on('finish', async () => { - writer.close(); - // Copy the file from the temp location to the connectorPath - await copy(tmpobj.name, connectorPath); - // Remove the temporary file - tmpobj.removeCallback(); - // Ensure connector-config.json exists for this project - const langClient = await MILanguageClient.getInstance(this.projectUri); - await langClient.initConnectorConfig(this.projectUri); - resolve({ path: connectorPath }); - }); + return new Promise((resolve, reject) => { + writer.on('finish', () => { + void (async () => { + try { + writer.close(); + await copy(tmpobj.name, connectorPath); + tmpobj.removeCallback(); + const langClient = await MILanguageClient.getInstance(this.projectUri); + await langClient.initConnectorConfig(this.projectUri); + resolve({ path: connectorPath }); + } catch (err) { + reject(err); + } + })(); + }); writer.on('error', reject); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workspaces/mi/mi-extension/src/rpc-managers/mi-diagram/rpc-manager.ts` around lines 3931 - 3940, The 'finish' listener on writer is async so any rejection from copy(...), MILanguageClient.getInstance(this.projectUri), or initConnectorConfig(...) will be unhandled and leave the outer promise unresolved; change the handler to catch errors and propagate them by calling the outer promise's reject (or otherwise resolving with an error) — e.g., in the writer.on('finish', ...) callback wrap the await calls to copy, MILanguageClient.getInstance, and initConnectorConfig in a try/catch and call reject(error) on failure (ensure you reference the same resolve/reject from the surrounding promise), and still perform tmpobj.removeCallback() in a finally block so temporary files are always cleaned up.
6598-6636:⚠️ Potential issue | 🟠 MajorRemove the
new Promise(async ...)wrappers from these RPC delegates.Lines 6599, 6607, 6615, 6623, and 6631 still use async promise executors. Any rejection from
MILanguageClient.getInstance(...)or the delegated request is detached from the returned promise, so the caller can wait forever instead of receiving an error.Suggested fix
async getConnectorDependencies(params: GetConnectorDependenciesRequest): Promise<GetConnectorDependenciesResponse> { - return new Promise(async (resolve) => { - const langClient = await MILanguageClient.getInstance(this.projectUri); - const res = await langClient.getConnectorDependencies(params); - resolve(res); - }); + const langClient = await MILanguageClient.getInstance(this.projectUri); + return langClient.getConnectorDependencies(params); } async updateConnectorDependencyOverride(params: UpdateConnectorDependencyOverrideRequest): Promise<boolean> { - return new Promise(async (resolve) => { - const langClient = await MILanguageClient.getInstance(this.projectUri); - const res = await langClient.updateConnectorDependencyOverride(params); - resolve(res); - }); + const langClient = await MILanguageClient.getInstance(this.projectUri); + return langClient.updateConnectorDependencyOverride(params); } async resetConnectorDependencyOverrides(params: ResetConnectorDependencyOverridesRequest): Promise<boolean> { - return new Promise(async (resolve) => { - const langClient = await MILanguageClient.getInstance(this.projectUri); - const res = await langClient.resetConnectorDependencyOverrides(params); - resolve(res); - }); + const langClient = await MILanguageClient.getInstance(this.projectUri); + return langClient.resetConnectorDependencyOverrides(params); } async updateConnectorFlags(params: UpdateConnectorFlagsRequest): Promise<boolean> { - return new Promise(async (resolve) => { - const langClient = await MILanguageClient.getInstance(this.projectUri); - const res = await langClient.updateConnectorFlags(params); - resolve(res); - }); + const langClient = await MILanguageClient.getInstance(this.projectUri); + return langClient.updateConnectorFlags(params); } async updateGlobalConnectorFlags(params: UpdateGlobalConnectorFlagsRequest): Promise<boolean> { - return new Promise(async (resolve) => { - const langClient = await MILanguageClient.getInstance(this.projectUri); - const res = await langClient.updateGlobalConnectorFlags(params); - resolve(res); - }); + const langClient = await MILanguageClient.getInstance(this.projectUri); + return langClient.updateGlobalConnectorFlags(params); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workspaces/mi/mi-extension/src/rpc-managers/mi-diagram/rpc-manager.ts` around lines 6598 - 6636, Several RPC delegate methods (getConnectorDependencies, updateConnectorDependencyOverride, resetConnectorDependencyOverrides, updateConnectorFlags, updateGlobalConnectorFlags) wrap calls in new Promise(async ...) which detaches rejections; remove the Promise wrappers and simply await/return the underlying call: get the language client via await MILanguageClient.getInstance(this.projectUri) and then return await langClient.<correspondingMethod>(params) (or directly return langClient.<correspondingMethod>(params)) so errors propagate to the caller.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@workspaces/mi/mi-diagram/src/components/Form/FormGenerator.tsx`:
- Around line 294-311: The effect's loadEffectiveDriver must clear stale state
and guard against out-of-order responses: when
connectorArtifactIdProp/formData?.connectorName/connectorName or connectionName
are missing, call setEffectiveDriverDep(null) before returning; inside
loadEffectiveDriver create a local request token (e.g., incrementing requestId
or a cancelled boolean) and capture it so that after awaiting
rpcClient.getMiDiagramRpcClient().getConnectorDependencies(...) you only call
setEffectiveDriverDep(dep ?? null) if the token is still current; also catch
errors and call setEffectiveDriverDep(null) in the catch to clear state on
failure. Ensure references to loadEffectiveDriver, setEffectiveDriverDep, and
getConnectorDependencies are used to locate the changes.
In `@workspaces/mi/mi-extension/src/rpc-managers/mi-diagram/rpc-manager.ts`:
- Around line 3931-3940: The 'finish' listener on writer is async so any
rejection from copy(...), MILanguageClient.getInstance(this.projectUri), or
initConnectorConfig(...) will be unhandled and leave the outer promise
unresolved; change the handler to catch errors and propagate them by calling the
outer promise's reject (or otherwise resolving with an error) — e.g., in the
writer.on('finish', ...) callback wrap the await calls to copy,
MILanguageClient.getInstance, and initConnectorConfig in a try/catch and call
reject(error) on failure (ensure you reference the same resolve/reject from the
surrounding promise), and still perform tmpobj.removeCallback() in a finally
block so temporary files are always cleaned up.
- Around line 6598-6636: Several RPC delegate methods (getConnectorDependencies,
updateConnectorDependencyOverride, resetConnectorDependencyOverrides,
updateConnectorFlags, updateGlobalConnectorFlags) wrap calls in new
Promise(async ...) which detaches rejections; remove the Promise wrappers and
simply await/return the underlying call: get the language client via await
MILanguageClient.getInstance(this.projectUri) and then return await
langClient.<correspondingMethod>(params) (or directly return
langClient.<correspondingMethod>(params)) so errors propagate to the caller.
In
`@workspaces/mi/mi-visualizer/src/views/Overview/ProjectInformation/DependencyItem.tsx`:
- Around line 351-368: The confirm handler handleDriverOmitConfirm can be
re-entered while an RPC is in flight; add a re-entry guard and close the dialog
immediately to prevent duplicate RPCs: at the top of handleDriverOmitConfirm
return early if isSaving is true, and as soon as a confirmed action is detected
call setConfirmOmitDriver(null) before awaiting the RPC; keep setIsSaving(true)
before the RPC and the existing try/finally to setIsSaving(false) after, and
apply the same pattern to the other confirm handlers referenced (the handlers
around lines 385-405, 717-719, 734-736) so dialogs are closed immediately and
further clicks are ignored while isSaving is true.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 58c62797-9e28-49a6-9d9f-0a99010ecc93
📒 Files selected for processing (13)
workspaces/mi/mi-core/src/rpc-types/mi-diagram/index.tsworkspaces/mi/mi-core/src/rpc-types/mi-diagram/rpc-type.tsworkspaces/mi/mi-core/src/rpc-types/mi-diagram/types.tsworkspaces/mi/mi-diagram/src/components/Form/DBConnector/DriverConfiguration.tsxworkspaces/mi/mi-diagram/src/components/Form/DynamicFields/DynamicFieldsHandler.tsxworkspaces/mi/mi-diagram/src/components/Form/FormGenerator.tsxworkspaces/mi/mi-extension/src/lang-client/ExtendedLanguageClient.tsworkspaces/mi/mi-extension/src/rpc-managers/mi-diagram/rpc-handler.tsworkspaces/mi/mi-extension/src/rpc-managers/mi-diagram/rpc-manager.tsworkspaces/mi/mi-rpc-client/src/rpc-clients/mi-diagram/rpc-client.tsworkspaces/mi/mi-visualizer/src/views/Forms/ConnectionForm/ConnectionFormGenerator.tsxworkspaces/mi/mi-visualizer/src/views/Overview/ProjectInformation/DependencyItem.tsxworkspaces/mi/mi-visualizer/src/views/Overview/ProjectInformation/DependencyManager.tsx
🚧 Files skipped from review as they are similar to previous changes (7)
- workspaces/mi/mi-diagram/src/components/Form/DynamicFields/DynamicFieldsHandler.tsx
- workspaces/mi/mi-diagram/src/components/Form/DBConnector/DriverConfiguration.tsx
- workspaces/mi/mi-extension/src/rpc-managers/mi-diagram/rpc-handler.ts
- workspaces/mi/mi-rpc-client/src/rpc-clients/mi-diagram/rpc-client.ts
- workspaces/mi/mi-visualizer/src/views/Overview/ProjectInformation/DependencyManager.tsx
- workspaces/mi/mi-extension/src/lang-client/ExtendedLanguageClient.ts
- workspaces/mi/mi-core/src/rpc-types/mi-diagram/index.ts
ChinthakaJ98
previously approved these changes
Apr 30, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
workspaces/mi/mi-diagram/src/components/Form/DynamicFields/DynamicFieldsHandler.tsx (1)
651-657:⚠️ Potential issue | 🟠 Major | ⚡ Quick winExtract
driverPathfrom the RPC response.Line 656 assigns the whole
downloadDriverForConnector(...)result intodriverPath, but this RPC now returns aDriverDownloadResponse, not a raw string. That makes the retry loop succeed on any truthy object and can pass an object where later code expects a filesystem path.Suggested fix
while (!isDriverDownloaded && retryCount < maxRetries) { const args = { connectorName: connectorName, connectionType: connectionType, }; this.setCustomError(getNameForController(FIELD_NAMES.CONFIG_KEY), "Checking DB Driver..."); - driverPath = await this.rpcClient.getMiDiagramRpcClient().downloadDriverForConnector(args); + const downloadResponse = await this.rpcClient.getMiDiagramRpcClient().downloadDriverForConnector(args); + driverPath = downloadResponse?.driverPath; if (driverPath) { isDriverDownloaded = true; } retryCount++; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workspaces/mi/mi-diagram/src/components/Form/DynamicFields/DynamicFieldsHandler.tsx` around lines 651 - 657, The RPC now returns a DriverDownloadResponse object, so change the code around downloadDriverForConnector to first await into a response variable (e.g., downloadResp = await this.rpcClient.getMiDiagramRpcClient().downloadDriverForConnector(args)), then extract the actual filesystem path property from that response (e.g., downloadResp.path or downloadResp.filePath) and assign that string to driverPath used later; also update the truthy check to ensure driverPath is a non-empty string before proceeding. Ensure you modify usages that expect a string path (driverPath variable) and keep getNameForController(FIELD_NAMES.CONFIG_KEY) / setCustomError calls intact.
♻️ Duplicate comments (2)
workspaces/mi/mi-diagram/src/components/Form/FormGenerator.tsx (1)
294-311:⚠️ Potential issue | 🟠 Major | ⚡ Quick winClear stale driver state when the lookup is skipped or fails.
If
cn/ctare missing, orgetConnectorDependenciesthrows,effectiveDriverDepkeeps the previous connector's value. ThedriverSelectOptionUI below then shows stale omitted/overridden/local-JAR status for the wrong connector.Suggested fix
const loadEffectiveDriver = async () => { const cn = connectorArtifactIdProp ?? formData?.connectorName ?? connectorName?.replace(/\s/g, ''); const ct = connectionName; - if (!cn || !ct) return; + if (!cn || !ct) { + setEffectiveDriverDep(null); + return; + } try { const resp = await rpcClient.getMiDiagramRpcClient().getConnectorDependencies({ connectorArtifactId: cn, }); const dep = resp?.dependencies?.find( d => d.connectionType?.toLowerCase() === ct.toLowerCase() ); setEffectiveDriverDep(dep ?? null); } catch { // non-DB connector or deps not available — silently ignore + setEffectiveDriverDep(null); } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workspaces/mi/mi-diagram/src/components/Form/FormGenerator.tsx` around lines 294 - 311, The loadEffectiveDriver function can leave a stale effectiveDriverDep when lookup is skipped or fails; ensure you clear that state immediately when cn or ct are missing and also in the catch block: call setEffectiveDriverDep(null) before returning when (!cn || !ct) and inside the catch after the failed rpcClient.getMiDiagramRpcClient().getConnectorDependencies call so the driverSelectOption UI doesn't show the previous connector's status.workspaces/mi/mi-visualizer/src/views/Overview/ProjectInformation/DependencyItem.tsx (1)
351-368:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winPrevent duplicate omit mutations while a save is in flight.
Line 717/Line 718 and Line 734/Line 735 keep confirm actions clickable, and the handlers also lack an
isSavingguard. Rapid clicks can send duplicate RPC writes for omit operations.Suggested patch
const handleDriverOmitConfirm = async (confirmed: boolean) => { + if (isSaving) return; if (confirmed && confirmOmitDriver) { setIsSaving(true); try { await rpcClient.getMiDiagramRpcClient().updateConnectorDependencyOverride({ connectorArtifactId, @@ const handleOmitAllDriversConfirm = async (confirmed: boolean) => { + if (isSaving) return; if (confirmed) { setIsSaving(true); try { await rpcClient.getMiDiagramRpcClient().resetConnectorDependencyOverrides({ connectorArtifactId, @@ - <Button appearance="secondary" onClick={() => handleDriverOmitConfirm(false)}>Cancel</Button> - <Button appearance="primary" onClick={() => handleDriverOmitConfirm(true)}>Omit</Button> + <Button appearance="secondary" onClick={() => handleDriverOmitConfirm(false)} disabled={isSaving}>Cancel</Button> + <Button appearance="primary" onClick={() => handleDriverOmitConfirm(true)} disabled={isSaving}>Omit</Button> @@ - <Button appearance="secondary" onClick={() => handleOmitAllDriversConfirm(false)}>Cancel</Button> - <Button appearance="primary" onClick={() => handleOmitAllDriversConfirm(true)}>Omit All</Button> + <Button appearance="secondary" onClick={() => handleOmitAllDriversConfirm(false)} disabled={isSaving}>Cancel</Button> + <Button appearance="primary" onClick={() => handleOmitAllDriversConfirm(true)} disabled={isSaving}>Omit All</Button>Also applies to: 385-405, 717-719, 734-736
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workspaces/mi/mi-visualizer/src/views/Overview/ProjectInformation/DependencyItem.tsx` around lines 351 - 368, The confirm handlers (e.g., handleDriverOmitConfirm) currently allow repeated clicks and lack an isSaving guard; add an early return when isSaving is true, and set isSaving to true before the RPC call and false in finally as already done to prevent duplicate mutations; also ensure the UI disables/hides the confirm buttons while isSaving (e.g., respect isSaving when rendering confirm controls tied to confirmOmitDriver) so rapid clicks cannot trigger rpcClient.getMiDiagramRpcClient().updateConnectorDependencyOverride or similar update calls, and apply the same fix to the other confirm handlers following this pattern (the ones that call updateConnectorDependencyOverride and invoke onDriverUpdated).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@workspaces/mi/mi-visualizer/src/views/Forms/ConnectionForm/ConnectionFormGenerator.tsx`:
- Around line 293-309: The current save logic only calls
updateConnectorDependencyOverride when any of
driverGroupId/driverArtifactId/driverVersion are non-empty, so clearing all
three leaves an old override in connector-config.json; modify the block that
builds driverGroupId, driverArtifactId, driverVersion and checks
connectionType/connectorArtifactId so that if all three are empty you call
rpcClient.getMiDiagramRpcClient().resetConnectorDependencyOverrides({
connectorArtifactId, connectionType }) instead of
updateConnectorDependencyOverride, otherwise call
updateConnectorDependencyOverride as before; apply the same change to the other
identical save path that currently uses updateConnectorDependencyOverride.
- Around line 136-157: The fetched driver override values (driverParamValues)
are stored into params.paramValues but the FormGenerator reads schema-backed
fields from params.parameters (or the connectionSchema defaults), so the edit
form still shows schema defaults; update the code after fetching matches to
merge these overrides into the schema-backed parameters used by FormGenerator:
map driverParamValues into params.parameters (e.g., set params.parameters[name]
= value) or construct a new parameters object and call setParams({ ...params,
paramValues: generateParams(driverParamValues), parameters: mergedParameters })
so that connectionSchema-backed fields (groupId/artifactId/version) are
initialized from the fetched overrides used by FormGenerator when rendering the
form.
---
Outside diff comments:
In
`@workspaces/mi/mi-diagram/src/components/Form/DynamicFields/DynamicFieldsHandler.tsx`:
- Around line 651-657: The RPC now returns a DriverDownloadResponse object, so
change the code around downloadDriverForConnector to first await into a response
variable (e.g., downloadResp = await
this.rpcClient.getMiDiagramRpcClient().downloadDriverForConnector(args)), then
extract the actual filesystem path property from that response (e.g.,
downloadResp.path or downloadResp.filePath) and assign that string to driverPath
used later; also update the truthy check to ensure driverPath is a non-empty
string before proceeding. Ensure you modify usages that expect a string path
(driverPath variable) and keep getNameForController(FIELD_NAMES.CONFIG_KEY) /
setCustomError calls intact.
---
Duplicate comments:
In `@workspaces/mi/mi-diagram/src/components/Form/FormGenerator.tsx`:
- Around line 294-311: The loadEffectiveDriver function can leave a stale
effectiveDriverDep when lookup is skipped or fails; ensure you clear that state
immediately when cn or ct are missing and also in the catch block: call
setEffectiveDriverDep(null) before returning when (!cn || !ct) and inside the
catch after the failed
rpcClient.getMiDiagramRpcClient().getConnectorDependencies call so the
driverSelectOption UI doesn't show the previous connector's status.
In
`@workspaces/mi/mi-visualizer/src/views/Overview/ProjectInformation/DependencyItem.tsx`:
- Around line 351-368: The confirm handlers (e.g., handleDriverOmitConfirm)
currently allow repeated clicks and lack an isSaving guard; add an early return
when isSaving is true, and set isSaving to true before the RPC call and false in
finally as already done to prevent duplicate mutations; also ensure the UI
disables/hides the confirm buttons while isSaving (e.g., respect isSaving when
rendering confirm controls tied to confirmOmitDriver) so rapid clicks cannot
trigger rpcClient.getMiDiagramRpcClient().updateConnectorDependencyOverride or
similar update calls, and apply the same fix to the other confirm handlers
following this pattern (the ones that call updateConnectorDependencyOverride and
invoke onDriverUpdated).
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4bcd5afb-3785-492d-9ea2-115aaa1fd0b5
📒 Files selected for processing (14)
workspaces/mi/mi-core/src/rpc-types/mi-diagram/index.tsworkspaces/mi/mi-core/src/rpc-types/mi-diagram/rpc-type.tsworkspaces/mi/mi-core/src/rpc-types/mi-diagram/types.tsworkspaces/mi/mi-diagram/src/components/Form/DBConnector/DriverConfiguration.tsxworkspaces/mi/mi-diagram/src/components/Form/DynamicFields/DynamicFieldsHandler.tsxworkspaces/mi/mi-diagram/src/components/Form/FormGenerator.tsxworkspaces/mi/mi-extension/src/lang-client/ExtendedLanguageClient.tsworkspaces/mi/mi-extension/src/rpc-managers/mi-diagram/rpc-handler.tsworkspaces/mi/mi-extension/src/rpc-managers/mi-diagram/rpc-manager.tsworkspaces/mi/mi-extension/src/util/templates.tsworkspaces/mi/mi-rpc-client/src/rpc-clients/mi-diagram/rpc-client.tsworkspaces/mi/mi-visualizer/src/views/Forms/ConnectionForm/ConnectionFormGenerator.tsxworkspaces/mi/mi-visualizer/src/views/Overview/ProjectInformation/DependencyItem.tsxworkspaces/mi/mi-visualizer/src/views/Overview/ProjectInformation/DependencyManager.tsx
✅ Files skipped from review due to trivial changes (3)
- workspaces/mi/mi-extension/src/util/templates.ts
- workspaces/mi/mi-extension/src/lang-client/ExtendedLanguageClient.ts
- workspaces/mi/mi-core/src/rpc-types/mi-diagram/rpc-type.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- workspaces/mi/mi-diagram/src/components/Form/DBConnector/DriverConfiguration.tsx
- workspaces/mi/mi-rpc-client/src/rpc-clients/mi-diagram/rpc-client.ts
Comment on lines
+136
to
+157
| // Load driver overrides from connector-config.json to pre-populate driver fields | ||
| const connectorArtifactId = connector?.artifactId ?? connector?.name ?? props.connector?.name ?? ''; | ||
| const ct = connectionFound.connectionType; | ||
| let driverParamValues: Array<{ name: string; value: string }> = []; | ||
| if (connectorArtifactId && ct) { | ||
| try { | ||
| const depResponse = await rpcClient.getMiDiagramRpcClient().getConnectorDependencies({ | ||
| connectorArtifactId, | ||
| }); | ||
| const match = depResponse?.dependencies?.find(d => | ||
| d.connectionType?.toUpperCase() === ct.toUpperCase()); | ||
| if (match) { | ||
| if (match.groupId) driverParamValues.push({ name: 'groupId', value: match.groupId }); | ||
| if (match.artifactId) driverParamValues.push({ name: 'artifactId', value: match.artifactId }); | ||
| const version = match.overriddenVersion ?? match.defaultVersion ?? ''; | ||
| if (version) driverParamValues.push({ name: 'version', value: version }); | ||
| } | ||
| } catch (_) { | ||
| // connector-config.json may not exist; fall back to empty driver fields | ||
| } | ||
| } | ||
| setParams({ ...params, paramValues: generateParams(driverParamValues) }); |
Contributor
There was a problem hiding this comment.
This does not actually prefill the schema-backed driver fields.
In the edit flow, the loaded driver override values are written into params, but the <FormGenerator> path does not use parameters to initialize regular form controls. When connectionSchema exists, groupId / artifactId / version will still render with their schema defaults instead of the fetched override.
Suggested fix
- let driverParamValues: Array<{ name: string; value: string }> = [];
+ let driverGroupId = '';
+ let driverArtifactId = '';
+ let driverVersion = '';
if (connectorArtifactId && ct) {
try {
const depResponse = await rpcClient.getMiDiagramRpcClient().getConnectorDependencies({
connectorArtifactId,
});
const match = depResponse?.dependencies?.find(d =>
d.connectionType?.toUpperCase() === ct.toUpperCase());
if (match) {
- if (match.groupId) driverParamValues.push({ name: 'groupId', value: match.groupId });
- if (match.artifactId) driverParamValues.push({ name: 'artifactId', value: match.artifactId });
- const version = match.overriddenVersion ?? match.defaultVersion ?? '';
- if (version) driverParamValues.push({ name: 'version', value: version });
+ driverGroupId = match.groupId ?? '';
+ driverArtifactId = match.artifactId ?? '';
+ driverVersion = match.overriddenVersion ?? match.defaultVersion ?? '';
}
} catch (_) {
// connector-config.json may not exist; fall back to empty driver fields
}
}
- setParams({ ...params, paramValues: generateParams(driverParamValues) });
reset({
name: props.connectionName,
- connectionType: connectionType
+ connectionType: connectionFound.connectionType
});
+ setValue('groupId', driverGroupId);
+ setValue('artifactId', driverArtifactId);
+ setValue('version', driverVersion);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Load driver overrides from connector-config.json to pre-populate driver fields | |
| const connectorArtifactId = connector?.artifactId ?? connector?.name ?? props.connector?.name ?? ''; | |
| const ct = connectionFound.connectionType; | |
| let driverParamValues: Array<{ name: string; value: string }> = []; | |
| if (connectorArtifactId && ct) { | |
| try { | |
| const depResponse = await rpcClient.getMiDiagramRpcClient().getConnectorDependencies({ | |
| connectorArtifactId, | |
| }); | |
| const match = depResponse?.dependencies?.find(d => | |
| d.connectionType?.toUpperCase() === ct.toUpperCase()); | |
| if (match) { | |
| if (match.groupId) driverParamValues.push({ name: 'groupId', value: match.groupId }); | |
| if (match.artifactId) driverParamValues.push({ name: 'artifactId', value: match.artifactId }); | |
| const version = match.overriddenVersion ?? match.defaultVersion ?? ''; | |
| if (version) driverParamValues.push({ name: 'version', value: version }); | |
| } | |
| } catch (_) { | |
| // connector-config.json may not exist; fall back to empty driver fields | |
| } | |
| } | |
| setParams({ ...params, paramValues: generateParams(driverParamValues) }); | |
| // Load driver overrides from connector-config.json to pre-populate driver fields | |
| const connectorArtifactId = connector?.artifactId ?? connector?.name ?? props.connector?.name ?? ''; | |
| const ct = connectionFound.connectionType; | |
| let driverGroupId = ''; | |
| let driverArtifactId = ''; | |
| let driverVersion = ''; | |
| if (connectorArtifactId && ct) { | |
| try { | |
| const depResponse = await rpcClient.getMiDiagramRpcClient().getConnectorDependencies({ | |
| connectorArtifactId, | |
| }); | |
| const match = depResponse?.dependencies?.find(d => | |
| d.connectionType?.toUpperCase() === ct.toUpperCase()); | |
| if (match) { | |
| driverGroupId = match.groupId ?? ''; | |
| driverArtifactId = match.artifactId ?? ''; | |
| driverVersion = match.overriddenVersion ?? match.defaultVersion ?? ''; | |
| } | |
| } catch (_) { | |
| // connector-config.json may not exist; fall back to empty driver fields | |
| } | |
| } | |
| reset({ | |
| name: props.connectionName, | |
| connectionType: connectionFound.connectionType | |
| }); | |
| setValue('groupId', driverGroupId); | |
| setValue('artifactId', driverArtifactId); | |
| setValue('version', driverVersion); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@workspaces/mi/mi-visualizer/src/views/Forms/ConnectionForm/ConnectionFormGenerator.tsx`
around lines 136 - 157, The fetched driver override values (driverParamValues)
are stored into params.paramValues but the FormGenerator reads schema-backed
fields from params.parameters (or the connectionSchema defaults), so the edit
form still shows schema defaults; update the code after fetching matches to
merge these overrides into the schema-backed parameters used by FormGenerator:
map driverParamValues into params.parameters (e.g., set params.parameters[name]
= value) or construct a new parameters object and call setParams({ ...params,
paramValues: generateParams(driverParamValues), parameters: mergedParameters })
so that connectionSchema-backed fields (groupId/artifactId/version) are
initialized from the fetched overrides used by FormGenerator when rendering the
form.
Comment on lines
+293
to
+309
| // If the user specified driver coordinates, persist them to connector-config.json. | ||
| const driverGroupId: string = values.groupId?.trim?.() ?? ''; | ||
| const driverArtifactId: string = values.artifactId?.trim?.() ?? ''; | ||
| const driverVersion: string = values.version?.trim?.() ?? ''; | ||
| const connectorArtifactId: string = props.connector?.artifactId ?? props.connector?.name ?? ''; | ||
| if (connectionType && connectorArtifactId && (driverGroupId || driverArtifactId || driverVersion)) { | ||
| try { | ||
| await rpcClient.getMiDiagramRpcClient().updateConnectorDependencyOverride({ | ||
| connectorArtifactId, | ||
| connectionType, | ||
| groupId: driverGroupId || undefined, | ||
| artifactId: driverArtifactId || undefined, | ||
| version: driverVersion || undefined, | ||
| }); | ||
| } catch (e) { | ||
| console.warn('Failed to persist driver coordinates to connector-config.json:', e); | ||
| } |
Contributor
There was a problem hiding this comment.
Clearing the driver fields never removes an existing override.
Both save paths only persist when at least one of groupId / artifactId / version is non-empty. If a user clears all three fields to go back to the connector default, the old override stays in connector-config.json silently. This PR already adds resetConnectorDependencyOverrides, which is the missing path here.
Suggested fix
- if (connectionType && connectorArtifactId && (driverGroupId || driverArtifactId || driverVersion)) {
+ if (connectionType && connectorArtifactId) {
try {
- await rpcClient.getMiDiagramRpcClient().updateConnectorDependencyOverride({
- connectorArtifactId,
- connectionType,
- groupId: driverGroupId || undefined,
- artifactId: driverArtifactId || undefined,
- version: driverVersion || undefined,
- });
+ if (driverGroupId || driverArtifactId || driverVersion) {
+ await rpcClient.getMiDiagramRpcClient().updateConnectorDependencyOverride({
+ connectorArtifactId,
+ connectionType,
+ groupId: driverGroupId || undefined,
+ artifactId: driverArtifactId || undefined,
+ version: driverVersion || undefined,
+ });
+ } else {
+ await rpcClient.getMiDiagramRpcClient().resetConnectorDependencyOverrides({
+ connectorArtifactId,
+ connectionType,
+ });
+ }
} catch (e) {
console.warn('Failed to persist driver coordinates to connector-config.json:', e);
}
}Also applies to: 355-375
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@workspaces/mi/mi-visualizer/src/views/Forms/ConnectionForm/ConnectionFormGenerator.tsx`
around lines 293 - 309, The current save logic only calls
updateConnectorDependencyOverride when any of
driverGroupId/driverArtifactId/driverVersion are non-empty, so clearing all
three leaves an old override in connector-config.json; modify the block that
builds driverGroupId, driverArtifactId, driverVersion and checks
connectionType/connectorArtifactId so that if all three are empty you call
rpcClient.getMiDiagramRpcClient().resetConnectorDependencyOverrides({
connectorArtifactId, connectionType }) instead of
updateConnectorDependencyOverride, otherwise call
updateConnectorDependencyOverride as before; apply the same change to the other
identical save path that currently uses updateConnectorDependencyOverride.
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.
Purpose
$subject
video1785716989.mp4
Summary by CodeRabbit
New Features
Improvements