feat(settings): add global dbt Core v1/v2 version management#339
Open
Nuri1977 wants to merge 4 commits into
Open
feat(settings): add global dbt Core v1/v2 version management#339Nuri1977 wants to merge 4 commits into
Nuri1977 wants to merge 4 commits into
Conversation
- support global dbt-core upgrades, downgrades, and rollback - add side-by-side Python v1 and Rust v2 runtime cards - show five stable v1 releases and available v2 previews - verify Apache dbt-core provenance after installation - add adapter compatibility and project safety checks - enable experimental Postgres support for scoped v2 commands - preserve settings when installation or verification fails - add confirmation, warning, progress, and result states - add unit tests for version management and runtime switching
📝 WalkthroughWalkthroughAdds managed dbt-core version and package management, adapter compatibility checks, environment-aware CLI execution with exit codes, failure-aware agent/tool reporting, expanded settings workflows, and toast layout refinements. ChangesDBT runtime management
Tool outcome handling
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
- add side-by-side dbt Core v1 Python and v2 Rust runtime cards - support verified global upgrades, downgrades, preview installs, and rollback - block unsupported v2 adapters and preserve experimental adapter warnings - propagate dbt exit codes and complete command output to the AI agent - render failed AI tool calls correctly instead of showing green success states - prevent partial dbt runs from being reported as fully successful - require a second run when validating incremental models - remove repetitive adapter warning toasts from dbt commands - fix long toast messages overflowing their containers - add regression tests for failed dbt commands and tool-result envelopes
- detect project adapters from connections and profiles.yml - separate v1 Python adapters from v2 built-in adapters - block unsupported v2 adapters before command execution - show v2 adapter compatibility in Settings - apply adapter checks to UI and AI dbt commands - consolidate adapter logic into dbtCoreVersion service
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
tests/unit/main/adapters/cli.adapter.test.ts (1)
49-81: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd coverage for the signal-killed (
code === null) case and the success path.Current tests cover env sanitization and a plain non-zero exit, but not: the streaming
runCommand's success (code === 0) path, or a signal-killed process (child.emit('close', null, 'SIGTERM')) — the latter directly exercises the null-code regression flagged incli.adapter.ts.🤖 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 `@tests/unit/main/adapters/cli.adapter.test.ts` around lines 49 - 81, Add tests alongside the existing nonzero-exit test for CliAdapter.runCommand covering both successful completion with close code 0 and signal termination via close(null, 'SIGTERM'). Assert the success path resolves and emits the expected cli:done behavior, and the signal-killed path exercises the null-code handling with the expected rejection/error and event ordering.src/renderer/components/settings/DbtSettings.tsx (1)
384-397: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winVerify
confirmDbtVersionChangehandles an emptysettings.pythonPathgracefully.
handleInstallDbtexplicitly guardsif (!settings.pythonPath) throw ...before installing, butconfirmDbtVersionChange/the "Available dbt runtimes" version buttons (reachable before any dbt-core setup finishes, since that panel isn't gated onsettings.dbtPath/pythonPath) sendpythonPath: settings.pythonPathunguarded. If the backend'sgetPythonExecutablefallback doesn't gracefully resolve a default whenpythonPathis empty, a first-time user could trigger a confusing backend error via this path instead of the clear "Install Python first" message used elsewhere.🤖 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 `@src/renderer/components/settings/DbtSettings.tsx` around lines 384 - 397, Update confirmDbtVersionChange to validate settings.pythonPath before calling installDbtVersionChange, matching handleInstallDbt’s existing “Install Python first” behavior. Return or throw the same clear user-facing error when the path is empty, and only pass pythonPath after validation.tests/unit/components/DbtSettings.versionChange.test.tsx (1)
156-163: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCheckbox click leaves "check current project after install" untested.
Clicking this checkbox actually unchecks it (defaults to checked since the mocked plan has
isMajorVersionChange: true), soconfirmDbtVersionChange'scheckCurrentProjectCompatibility()branch is skipped rather than exercised — anduseCheckCurrentProjectCompatibilityhas nomockResolvedValueto assert against. As written, this test never verifies the post-install project-check flow. Consider either asserting oncompatibilityResultrendering with the checkbox left checked, or removing the click if it's not meant to test anything.🤖 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 `@tests/unit/components/DbtSettings.versionChange.test.tsx` around lines 156 - 163, The test currently unchecks the default-enabled “Check current project after install” option, so it skips the compatibility-check branch. Update the test around confirmDbtVersionChange to leave the checkbox checked or explicitly configure it accordingly, mock useCheckCurrentProjectCompatibility with a resolved result, and assert the resulting compatibilityResult rendering to verify the post-install project-check flow.src/main/ipcHandlers/settings.ipcHandlers.ts (1)
148-198: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNew dbt IPC handlers leave request payloads untyped.
Unlike the pre-existing handlers in this file (e.g.
settings:savetypesbody: SettingsType), all 12 newdbt:*handlers accept an implicitly-anyrequest/req. The corresponding request types (DbtVersionListOptions,DbtVersionChangePlanRequest,DbtVersionChangeRequest,PythonPackageActionRequest,PythonPackageInstallVersionRequest,PythonPackageVersionListRequest) already exist insrc/types/backend.tsand are imported by the renderer controller — applying them here would restore compile-time safety at this IPC boundary.♻️ Example for one handler
- ipcMain.handle('dbt:versions:list', async (_event, request) => { + ipcMain.handle('dbt:versions:list', async (_event, request: DbtVersionListOptions) => { return DbtCoreVersionService.listDbtCoreVersions(request); });🤖 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 `@src/main/ipcHandlers/settings.ipcHandlers.ts` around lines 148 - 198, Type every request parameter in the new dbt IPC handlers in the registration block, importing and reusing the existing request types from backend.ts. Apply the appropriate types to version listing, version-change, package action, package-version listing, and package-version installation handlers, and type adapter requests consistently with their expected projectPath payload. Leave handlers without request payloads unchanged and ensure no request/req remains implicitly any.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main/services/dbtCoreVersion.service.ts`:
- Around line 775-789: The adapter compatibility check must use the live
dbt-core version detected by checkCurrentProjectCompatibility instead of
settings.dbtVersion. Thread that detected version through
checkProjectAdapterCompatibility and its callers, including the anchor flow at
src/main/services/dbtCoreVersion.service.ts:775-789 and sibling flow at
src/main/services/dbtCoreVersion.service.ts:1009-1037; update the corresponding
test setup and expectations at
tests/unit/services/dbtCoreVersion.service.test.ts:376-399 to verify stale
settings cannot misclassify v1/v2 or bypass the Postgres safety block.
- Around line 237-267: Update runProcess to enforce a finite execution timeout
for spawned commands, terminating the child process when the timeout elapses and
rejecting or otherwise returning a timeout failure so awaiting IPC calls cannot
remain blocked indefinitely. Preserve the existing stdout, stderr, exit-code,
and process-error handling for commands that complete normally.
In `@src/renderer/components/settings/DbtSettings.tsx`:
- Around line 261-296: Update the adapter-package branch in the install loop
around installLatestPackage so failures accumulate instead of replacing the
existing version-change result. Preserve the first or combined adapter failure
in setVersionChangeResult, and stop or otherwise prevent later failures from
clobbering it while retaining the existing dbt-core failure handling.
In `@src/renderer/utils/dbtCommandResult.ts`:
- Line 1: Update ANSI_ESCAPE_REGEX to require the ANSI escape character before
the bracketed formatting sequence, supporting the ESC representation used by
terminal output and preventing literal text such as “[1m” from matching.
---
Nitpick comments:
In `@src/main/ipcHandlers/settings.ipcHandlers.ts`:
- Around line 148-198: Type every request parameter in the new dbt IPC handlers
in the registration block, importing and reusing the existing request types from
backend.ts. Apply the appropriate types to version listing, version-change,
package action, package-version listing, and package-version installation
handlers, and type adapter requests consistently with their expected projectPath
payload. Leave handlers without request payloads unchanged and ensure no
request/req remains implicitly any.
In `@src/renderer/components/settings/DbtSettings.tsx`:
- Around line 384-397: Update confirmDbtVersionChange to validate
settings.pythonPath before calling installDbtVersionChange, matching
handleInstallDbt’s existing “Install Python first” behavior. Return or throw the
same clear user-facing error when the path is empty, and only pass pythonPath
after validation.
In `@tests/unit/components/DbtSettings.versionChange.test.tsx`:
- Around line 156-163: The test currently unchecks the default-enabled “Check
current project after install” option, so it skips the compatibility-check
branch. Update the test around confirmDbtVersionChange to leave the checkbox
checked or explicitly configure it accordingly, mock
useCheckCurrentProjectCompatibility with a resolved result, and assert the
resulting compatibilityResult rendering to verify the post-install project-check
flow.
In `@tests/unit/main/adapters/cli.adapter.test.ts`:
- Around line 49-81: Add tests alongside the existing nonzero-exit test for
CliAdapter.runCommand covering both successful completion with close code 0 and
signal termination via close(null, 'SIGTERM'). Assert the success path resolves
and emits the expected cli:done behavior, and the signal-killed path exercises
the null-code handling with the expected rejection/error and event ordering.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 466566e3-f983-4e0a-8278-032ceba605c9
📒 Files selected for processing (33)
src/main/adapters/cli.adapter.tssrc/main/ipcHandlers/cli.ipcHandlers.tssrc/main/ipcHandlers/settings.ipcHandlers.tssrc/main/services/agent.service.tssrc/main/services/ai/agents/projectAgent.tssrc/main/services/ai/tools/dbt.tools.tssrc/main/services/ai/tools/studio/cli.tools.tssrc/main/services/dbtCoreVersion.service.tssrc/main/services/dbtVersionManager.service.tssrc/renderer/components/chat/AgentStepBlock.tsxsrc/renderer/components/chat/MessageRenderer.tsxsrc/renderer/components/chat/ToolCallRow.tsxsrc/renderer/components/settings/DbtSettings.tsxsrc/renderer/controllers/dbtVersions.controller.tssrc/renderer/controllers/index.tssrc/renderer/hooks/useAgentStream.tssrc/renderer/hooks/useCli.tsxsrc/renderer/hooks/useDbt.tssrc/renderer/services/dbtVersions.service.tssrc/renderer/services/projects.service.tssrc/renderer/toastStyles.csssrc/renderer/utils/dbtCommandResult.tssrc/renderer/utils/dbtProcessEnvironment.tssrc/shared/toolResult.tssrc/types/backend.tssrc/types/ipc.tstests/unit/components/DbtSettings.versionChange.test.tsxtests/unit/hooks/useDbt.experimentalAdapters.test.tstests/unit/main/adapters/cli.adapter.test.tstests/unit/main/services/ai/dbt.tools.test.tstests/unit/services/dbtCoreVersion.service.test.tstests/unit/shared/toolResult.test.tstests/unit/utils/dbtCommandResult.test.ts
💤 Files with no reviewable changes (1)
- src/main/services/dbtVersionManager.service.ts
Comment on lines
+237
to
+267
| const runProcess = ( | ||
| command: string, | ||
| args: string[], | ||
| options?: ProcessOptions, | ||
| ): Promise<ProcessResult> => { | ||
| return new Promise((resolve, reject) => { | ||
| const child = spawn(command, args, { | ||
| shell: false, | ||
| ...(options?.cwd ? { cwd: options.cwd } : {}), | ||
| ...(options?.env ? { env: options.env } : {}), | ||
| }); | ||
| let stdout = ''; | ||
| let stderr = ''; | ||
|
|
||
| child.stdout.on('data', (data) => { | ||
| stdout += String(data); | ||
| }); | ||
|
|
||
| child.stderr.on('data', (data) => { | ||
| stderr += String(data); | ||
| }); | ||
|
|
||
| child.on('error', (error) => { | ||
| reject(error); | ||
| }); | ||
|
|
||
| child.on('close', (exitCode) => { | ||
| resolve({ exitCode, stdout, stderr }); | ||
| }); | ||
| }); | ||
| }; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
runProcess spawns without any timeout.
Every pip install/uninstall and every dbt parse/dbt compile invocation goes through runProcess, which has no timeout or kill mechanism (unlike fetchPypiProjectJson's axios.get(url, { timeout: 15000 }) in this same file). A hung network install or a stuck dbt process leaves the awaiting IPC call — and the renderer UI state waiting on it — blocked indefinitely.
⏱️ Suggested direction
const runProcess = (
command: string,
args: string[],
options?: ProcessOptions,
): Promise<ProcessResult> => {
return new Promise((resolve, reject) => {
const child = spawn(command, args, {
shell: false,
...(options?.cwd ? { cwd: options.cwd } : {}),
...(options?.env ? { env: options.env } : {}),
});
let stdout = '';
let stderr = '';
+ const timeoutMs = options?.timeoutMs ?? 5 * 60 * 1000;
+ const timer = setTimeout(() => {
+ child.kill();
+ reject(new Error(`Command timed out after ${timeoutMs}ms: ${command}`));
+ }, timeoutMs);
...
child.on('close', (exitCode) => {
+ clearTimeout(timer);
resolve({ exitCode, stdout, stderr });
});
});
};🤖 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 `@src/main/services/dbtCoreVersion.service.ts` around lines 237 - 267, Update
runProcess to enforce a finite execution timeout for spawned commands,
terminating the child process when the timeout elapses and rejecting or
otherwise returning a timeout failure so awaiting IPC calls cannot remain
blocked indefinitely. Preserve the existing stdout, stderr, exit-code, and
process-error handling for commands that complete normally.
Comment on lines
+775
to
+789
| const adapterCheck = await this.checkProjectAdapterCompatibility( | ||
| project.path, | ||
| ); | ||
| if (!adapterCheck.adapter.canExecute) { | ||
| return { | ||
| ok: false, | ||
| projectName: project.name, | ||
| projectPath: project.path, | ||
| diagnostics: [], | ||
| recommendations: [ | ||
| 'Switch the global dbt runtime to a stable v1 release in Settings before running this project.', | ||
| ], | ||
| error: `${adapterCheck.adapter.displayName}: ${adapterCheck.adapter.notes}`, | ||
| }; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Use the live dbt-core version for adapter gating checkProjectAdapterCompatibility still derives runtime from settings.dbtVersion; thread the already-detected version from checkCurrentProjectCompatibility through this path so a stale setting can’t misclassify v1/v2 and skip the Postgres safety block.
📍 Affects 2 files
src/main/services/dbtCoreVersion.service.ts#L775-L789(this comment)src/main/services/dbtCoreVersion.service.ts#L1009-L1037tests/unit/services/dbtCoreVersion.service.test.ts#L376-L399
🤖 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 `@src/main/services/dbtCoreVersion.service.ts` around lines 775 - 789, The
adapter compatibility check must use the live dbt-core version detected by
checkCurrentProjectCompatibility instead of settings.dbtVersion. Thread that
detected version through checkProjectAdapterCompatibility and its callers,
including the anchor flow at src/main/services/dbtCoreVersion.service.ts:775-789
and sibling flow at src/main/services/dbtCoreVersion.service.ts:1009-1037;
update the corresponding test setup and expectations at
tests/unit/services/dbtCoreVersion.service.test.ts:376-399 to verify stale
settings cannot misclassify v1/v2 or bypass the Postgres safety block.
Comment on lines
+261
to
296
| for (let i = 0; i < packages.length; i++) { | ||
| const pkg = packages[i]; | ||
| setCurrentPackage(pkg); | ||
| setLoadingMessage(`Installing ${pkg}...`); | ||
| setInstallProgress((i / packages.length) * 100); | ||
|
|
||
| if (pkg === 'dbt-core') { | ||
| const result = await installPackageVersion({ | ||
| pythonPath: settings.pythonPath, | ||
| packageName: pkg, | ||
| version: targetDbtVersion, | ||
| }); | ||
| if (!result.ok) { | ||
| setVersionChangeResult(result); | ||
| break; | ||
| } | ||
| } | ||
| } else { | ||
| try { | ||
| for (let i = 0; i < packages.length; i++) { | ||
| const pkg = packages[i]; | ||
| setCurrentPackage(pkg); | ||
| setInstallProgress((i / packages.length) * 100); | ||
|
|
||
| try { | ||
| await runCommand(`pip install ${pkg}`); | ||
| } catch { | ||
| /* Continue with next package */ | ||
| } | ||
| if (result.dbtPath) { | ||
| onInstallDbtSave('dbtPath', result.dbtPath); | ||
| } | ||
| if (result.installedVersion) { | ||
| onInstallDbtSave('dbtVersion', result.installedVersion); | ||
| } | ||
| } else { | ||
| const result = await installLatestPackage({ | ||
| pythonPath: settings.pythonPath, | ||
| packageName: pkg, | ||
| }); | ||
| if (!result.ok) { | ||
| setVersionChangeResult({ | ||
| ok: false, | ||
| error: result.error || `Unable to install ${pkg}.`, | ||
| }); | ||
| } | ||
| } catch { | ||
| /* empty */ | ||
| } | ||
| } | ||
|
|
||
| // Get and save the dbt path | ||
| setCurrentPackage('Locating dbt path...'); | ||
| setInstallProgress(100); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Adapter-package install failures overwrite each other in the loop.
When pkg !== 'dbt-core' fails, setVersionChangeResult clobbers whatever failure was recorded for a previously-failed adapter package in the same loop, and the loop keeps going. If e.g. dbt-postgres and dbt-bigquery both fail, only the dbt-bigquery error ends up visible; the dbt-postgres failure is silently dropped even though checkInstalledPackages() afterward won't show it as installed either.
🐛 Proposed fix: accumulate failures instead of overwriting
+ const failedPackages: string[] = [];
for (let i = 0; i < packages.length; i++) {
const pkg = packages[i];
setCurrentPackage(pkg);
setLoadingMessage(`Installing ${pkg}...`);
setInstallProgress((i / packages.length) * 100);
if (pkg === 'dbt-core') {
const result = await installPackageVersion({
pythonPath: settings.pythonPath,
packageName: pkg,
version: targetDbtVersion,
});
if (!result.ok) {
setVersionChangeResult(result);
break;
}
if (result.dbtPath) {
onInstallDbtSave('dbtPath', result.dbtPath);
}
if (result.installedVersion) {
onInstallDbtSave('dbtVersion', result.installedVersion);
}
} else {
const result = await installLatestPackage({
pythonPath: settings.pythonPath,
packageName: pkg,
});
if (!result.ok) {
- setVersionChangeResult({
- ok: false,
- error: result.error || `Unable to install ${pkg}.`,
- });
+ failedPackages.push(pkg);
}
}
}
+ if (failedPackages.length > 0) {
+ setVersionChangeResult({
+ ok: false,
+ error: `Unable to install: ${failedPackages.join(', ')}.`,
+ });
+ }📝 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
| for (let i = 0; i < packages.length; i++) { | |
| const pkg = packages[i]; | |
| setCurrentPackage(pkg); | |
| setLoadingMessage(`Installing ${pkg}...`); | |
| setInstallProgress((i / packages.length) * 100); | |
| if (pkg === 'dbt-core') { | |
| const result = await installPackageVersion({ | |
| pythonPath: settings.pythonPath, | |
| packageName: pkg, | |
| version: targetDbtVersion, | |
| }); | |
| if (!result.ok) { | |
| setVersionChangeResult(result); | |
| break; | |
| } | |
| } | |
| } else { | |
| try { | |
| for (let i = 0; i < packages.length; i++) { | |
| const pkg = packages[i]; | |
| setCurrentPackage(pkg); | |
| setInstallProgress((i / packages.length) * 100); | |
| try { | |
| await runCommand(`pip install ${pkg}`); | |
| } catch { | |
| /* Continue with next package */ | |
| } | |
| if (result.dbtPath) { | |
| onInstallDbtSave('dbtPath', result.dbtPath); | |
| } | |
| if (result.installedVersion) { | |
| onInstallDbtSave('dbtVersion', result.installedVersion); | |
| } | |
| } else { | |
| const result = await installLatestPackage({ | |
| pythonPath: settings.pythonPath, | |
| packageName: pkg, | |
| }); | |
| if (!result.ok) { | |
| setVersionChangeResult({ | |
| ok: false, | |
| error: result.error || `Unable to install ${pkg}.`, | |
| }); | |
| } | |
| } catch { | |
| /* empty */ | |
| } | |
| } | |
| // Get and save the dbt path | |
| setCurrentPackage('Locating dbt path...'); | |
| setInstallProgress(100); | |
| const failedPackages: string[] = []; | |
| for (let i = 0; i < packages.length; i++) { | |
| const pkg = packages[i]; | |
| setCurrentPackage(pkg); | |
| setLoadingMessage(`Installing ${pkg}...`); | |
| setInstallProgress((i / packages.length) * 100); | |
| if (pkg === 'dbt-core') { | |
| const result = await installPackageVersion({ | |
| pythonPath: settings.pythonPath, | |
| packageName: pkg, | |
| version: targetDbtVersion, | |
| }); | |
| if (!result.ok) { | |
| setVersionChangeResult(result); | |
| break; | |
| } | |
| if (result.dbtPath) { | |
| onInstallDbtSave('dbtPath', result.dbtPath); | |
| } | |
| if (result.installedVersion) { | |
| onInstallDbtSave('dbtVersion', result.installedVersion); | |
| } | |
| } else { | |
| const result = await installLatestPackage({ | |
| pythonPath: settings.pythonPath, | |
| packageName: pkg, | |
| }); | |
| if (!result.ok) { | |
| failedPackages.push(pkg); | |
| } | |
| } | |
| } | |
| if (failedPackages.length > 0) { | |
| setVersionChangeResult({ | |
| ok: false, | |
| error: `Unable to install: ${failedPackages.join(', ')}.`, | |
| }); | |
| } | |
| setInstallProgress(100); |
🤖 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 `@src/renderer/components/settings/DbtSettings.tsx` around lines 261 - 296,
Update the adapter-package branch in the install loop around
installLatestPackage so failures accumulate instead of replacing the existing
version-change result. Preserve the first or combined adapter failure in
setVersionChangeResult, and stop or otherwise prevent later failures from
clobbering it while retaining the existing dbt-core failure handling.
| @@ -0,0 +1,46 @@ | |||
| const ANSI_ESCAPE_REGEX = /\[[0-9;]*m/g; | |||
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Include the ESC character in ANSI_ESCAPE_REGEX.
The current regex /\[[0-9;]*m/g misses the escape character (\x1b or \u001b) and will incorrectly match and strip literal text like [1m from strings (e.g., Array[1m]).
🐛 Proposed fix for the ANSI escape regex
-const ANSI_ESCAPE_REGEX = /\[[0-9;]*m/g;
+const ANSI_ESCAPE_REGEX = /\x1b\[[0-9;]*m/g;📝 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
| const ANSI_ESCAPE_REGEX = /\[[0-9;]*m/g; | |
| const ANSI_ESCAPE_REGEX = /\x1b\[[0-9;]*m/g; |
🤖 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 `@src/renderer/utils/dbtCommandResult.ts` at line 1, Update ANSI_ESCAPE_REGEX
to require the ANSI escape character before the bracketed formatting sequence,
supporting the ESC representation used by terminal output and preventing literal
text such as “[1m” from matching.
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
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Bug Fixes
Tests