Skip to content

Conversation

@mdesmet
Copy link
Contributor

@mdesmet mdesmet commented Oct 19, 2025

  • Update fusion detection to use 'dbtf --version' directly
  • Set dbtPath to 'dbtf' in DBTFusionCommandProjectIntegration
  • Remove unused getDBTPath import from fusion integration
  • Ensure dbt cloud integration remains unaffected

This ensures that dbt-fusion installed as 'dbtf' is properly detected
and used for all fusion operations.

🤖 Generated with Claude Code

Co-Authored-By: Claude [email protected]

Overview

Problem

Describe the problem you are solving. Mention the ticket/issue if applicable.

Solution

Describe the implemented solution. Add external references if needed.

Screenshot/Demo

A picture is worth a thousand words. Please highlight the changes if applicable.

How to test

  • Steps to be followed to verify the solution or code changes
  • Mention if there is any settings configuration added/changed/deleted

Checklist

  • I have run this code and it appears to resolve the stated issue
  • README.md updated and added information about my change

Important

Update dbt-fusion integration to detect and use 'dbtf' command, handling aliases and PATH resolution.

  • Behavior:
    • Update DBTFusionCommandDetection in dbtFusionCommandIntegration.ts to use dbtf --version for detection.
    • Set dbtPath to resolved dbtf path in DBTFusionCommandProjectIntegration.
    • Ensure dbt cloud integration remains unaffected.
  • Functions:
    • Add resolveDbtfAlias() and resolveDbtfPath() in dbtFusionCommandIntegration.ts to handle command resolution.
  • Imports:
    • Remove unused getDBTPath import from dbtFusionCommandIntegration.ts.
  • Dependency Injection:
    • Update inversify.config.ts to inject CommandProcessExecutionFactory into DBTFusionCommandProjectIntegration.

This description was created by Ellipsis for 231eac1. You can customize this summary. It will automatically update as commits are pushed.


Summary by CodeRabbit

Refactor

  • Improved DBT/Fusion command detection and initialization so the app resolves and logs the tool path more reliably across platforms, and propagates that path during project setup for more consistent runtime behavior.

- Update fusion detection to use 'dbtf --version' directly
- Set dbtPath to 'dbtf' in DBTFusionCommandProjectIntegration
- Remove unused getDBTPath import from fusion integration
- Ensure dbt cloud integration remains unaffected

This ensures that dbt-fusion installed as 'dbtf' is properly detected
and used for all fusion operations.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 19, 2025

Walkthrough

Detect and resolve the dbt/fusion command path via new helpers (alias resolution, which lookup, fallback), remove the prior exported getDBTPath, add an initializeProject() method to DBTFusionCommandProjectIntegration to resolve/store the dbt/fusion command path, and update DI wiring to pass a new CommandProcessExecutionFactory dependency.

Changes

Cohort / File(s) Summary
DBT Fusion Command Integration
src/dbt_client/dbtFusionCommandIntegration.ts
Added internal helpers resolveDbtfAlias and resolveDbtfPath (alias resolution, which lookup, fallback). detectDBT uses resolveDbtfPath. Added initializeProject(): Promise<void> to resolve and assign this.dbtPath after super.initializeProject(). Constructor signature extended to accept CommandProcessExecutionFactory and other injected infra; stores it as a private member. Removed import/usage of previous getDBTPath.
Inversify DI config
src/inversify.config.ts
Updated factory/constructor call for DBTFusionCommandProjectIntegration to fetch and pass CommandProcessExecutionFactory from the container alongside existing dependencies.
DBT Cloud Integration export removal
src/dbt_client/dbtCloudIntegration.ts
Removed exported function getDBTPath.
Dependencies / Declarations
package.json (implicit), various imports in changed files
Added dependency on which for PATH lookups and introduced imports for new public-like types (TelemetryService, AltimateRequest, DBTCommandExecutionInfrastructure, DBTCommandFactory, DBTCommandExecutionStrategy).

Sequence Diagram(s)

sequenceDiagram
  participant Init as Project Init
  participant Integration as DBTFusionCommandProjectIntegration
  participant Resolver as resolveDbtfPath
  participant Shell as Shell (alias/which)
  participant DBT as dbt/dbtf

  Note right of Integration: initializeProject()
  Init->>Integration: initializeProject()
  Integration->>Integration: super.initializeProject()
  Integration->>Resolver: resolveDbtfPath()
  Resolver->>Shell: check shell aliases (platform-specific)
  Shell-->>Resolver: alias path | null
  alt alias found
    Resolver-->>Integration: resolved path (alias)
  else alias not found
    Resolver->>Shell: which lookup on PATH
    Shell-->>Resolver: which result | null
    alt which found
      Resolver-->>Integration: resolved path (which)
    else none found
      Resolver-->>Integration: fallback "dbt" (or "dbtf")
    end
  end
  Integration->>DBT: use resolved path for version check / commands
  DBT-->>Integration: version/response
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested reviewers

  • anandgupta42
  • AdiGajbhiye

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The pull request description includes the template structure but fails to properly complete the required sections. The "Problem," "Solution," and "How to test" sections contain only the template placeholder text without actual content being filled in, and the checklist items remain unchecked with no indication of completion. While supplemental information is provided in the IMPORTANT blockquote describing the behavior, functions, and dependency injection changes, this does not fulfill the template's structural requirement that key information be placed in the designated sections. The lack of specific testing steps and an incomplete problem statement are particularly concerning for reviewability.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title "fix: detect and use dbtf command for fusion integration" clearly and concisely summarizes the main change in the pull request. It captures the core objective: updating the fusion integration to properly detect and use the 'dbtf' command. The title is specific enough that a reviewer scanning the commit history would immediately understand the primary purpose of the change, and it avoids vague language or noise.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/dbt-fusion-detection-dbtf-path

Warning

Review ran into problems

🔥 Problems

Errors were encountered while retrieving linked issues.

Errors (1)
  • JIRA integration encountered authorization issues. Please disconnect and reconnect the integration in the CodeRabbit UI.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@mdesmet mdesmet requested a review from saravmajestic October 19, 2025 07:51
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

Looks good to me! 👍

Reviewed everything up to 87d9b80 in 1 minute and 4 seconds. Click for details.
  • Reviewed 39 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. src/dbt_client/dbtFusionCommandIntegration.ts:41
  • Draft comment:
    Consider extracting the hardcoded 'dbtf' string into a named constant (e.g. DBT_FUSION_CLI) to avoid duplication and improve maintainability.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50% None
2. src/dbt_client/dbtFusionCommandIntegration.ts:37
  • Draft comment:
    Consider refactoring detectDBT() to use early returns for failure cases to simplify the control flow and reduce nesting.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50% None

Workflow ID: wflow_554ao2vIIBWzjm9y

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
src/dbt_client/dbtFusionCommandIntegration.ts (1)

109-112: Based on the verification information gathered, I can now generate the final response. The web search on TypeScript best practices confirms that the pattern of calling super.initializeProject() followed by specializing the behavior is correct and standard. The script output showed that the parent's initializeProject() method ends at line 333 (right after the dbtPath assignment at line 332), indicating it primarily does dbtPath initialization.

Add override keyword to match TypeScript best practices.

The override keyword should be added to catch potential errors at compile time. While the method correctly calls super.initializeProject() before specializing with dbtPath = "dbtf", the implementation should be:

async override initializeProject(): Promise<void> {
  await super.initializeProject();
  this.dbtPath = "dbtf";
}

This pattern is safe—the parent's initialization completes before the child's override takes effect, ensuring DBT Fusion always uses the correct executable.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 158fab7 and 87d9b80.

📒 Files selected for processing (1)
  • src/dbt_client/dbtFusionCommandIntegration.ts (3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/dbt_client/*Integration.ts

📄 CodeRabbit inference engine (CLAUDE.md)

Extend the appropriate dbt client file (e.g., dbtCoreIntegration.ts, dbtCloudIntegration.ts, dbtFusionCommandIntegration.ts) when adding new dbt integration features.

Files:

  • src/dbt_client/dbtFusionCommandIntegration.ts
🧠 Learnings (1)
📚 Learning: 2025-07-30T17:27:13.149Z
Learnt from: CR
PR: AltimateAI/vscode-dbt-power-user#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-30T17:27:13.149Z
Learning: Applies to src/dbt_client/*Integration.ts : Extend the appropriate dbt client file (e.g., `dbtCoreIntegration.ts`, `dbtCloudIntegration.ts`, `dbtFusionCommandIntegration.ts`) when adding new dbt integration features.

Applied to files:

  • src/dbt_client/dbtFusionCommandIntegration.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
  • GitHub Check: build (windows-latest)
  • GitHub Check: build (macos-latest)
  • GitHub Check: test (macos-latest)
  • GitHub Check: test (windows-latest)
  • GitHub Check: test (ubuntu-latest)
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (2)
src/dbt_client/dbtFusionCommandIntegration.ts (2)

26-26: LGTM! Import cleaned up correctly.

The removal of getDBTPath import aligns with the PR objective to eliminate this dependency and use a hardcoded "dbtf" command instead.


42-42: Good simplification for fusion detection.

Using "dbtf" directly is cleaner and aligns with how dbt-fusion is installed as an executable. The detection logic properly validates the command exists before the integration is used.

- Add cross-platform shell alias resolution for dbtf command
- Support Windows (doskey), bash, and zsh alias formats
- Implement fallback chain: alias -> which dbt -> "dbt"
- Handles cases where dbtf is aliased to a specific dbt path
- Update DBTFusionCommandProjectIntegration with CommandProcessExecutionFactory

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

Looks good to me! 👍

Reviewed 231eac1 in 59 seconds. Click for details.
  • Reviewed 234 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 5 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. src/dbt_client/dbtFusionCommandIntegration.ts:90
  • Draft comment:
    Consider enhancing the regex patterns in resolveDbtfAlias to handle alias outputs with extra quotes or different formatting to improve robustness.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50% None
2. src/dbt_client/dbtFusionCommandIntegration.ts:150
  • Draft comment:
    When neither an alias nor a PATH lookup succeeds in resolveDbtfPath, consider logging a warning before falling back to 'dbt'.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50% None
3. src/dbt_client/dbtFusionCommandIntegration.ts:187
  • Draft comment:
    Using stdout.includes('dbt-fusion') for CLI detection may be brittle. Consider a more robust version parsing strategy.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
4. src/dbt_client/dbtFusionCommandIntegration.ts:276
  • Draft comment:
    In initializeProject, the use of resolveDbtfPath to set dbtPath is good; ensure appropriate error handling if resolution unexpectedly fails.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50% None
5. src/inversify.config.ts:216
  • Draft comment:
    The factory binding for DBTFusionCommandProjectIntegration now includes CommandProcessExecutionFactory. Verify that the constructor parameter order and dependency injection are correct.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50% None

Workflow ID: wflow_HlYYbZgY2uC9Dlc4

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 87d9b80 and 231eac1.

📒 Files selected for processing (2)
  • src/dbt_client/dbtFusionCommandIntegration.ts (4 hunks)
  • src/inversify.config.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
src/inversify.config.ts

📄 CodeRabbit inference engine (CLAUDE.md)

Register all new providers in src/inversify.config.ts when adding language features.

Files:

  • src/inversify.config.ts
src/dbt_client/*Integration.ts

📄 CodeRabbit inference engine (CLAUDE.md)

Extend the appropriate dbt client file (e.g., dbtCoreIntegration.ts, dbtCloudIntegration.ts, dbtFusionCommandIntegration.ts) when adding new dbt integration features.

Files:

  • src/dbt_client/dbtFusionCommandIntegration.ts
🧠 Learnings (1)
📚 Learning: 2025-07-30T17:27:13.149Z
Learnt from: CR
PR: AltimateAI/vscode-dbt-power-user#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-30T17:27:13.149Z
Learning: Applies to src/dbt_client/*Integration.ts : Extend the appropriate dbt client file (e.g., `dbtCoreIntegration.ts`, `dbtCloudIntegration.ts`, `dbtFusionCommandIntegration.ts`) when adding new dbt integration features.

Applied to files:

  • src/dbt_client/dbtFusionCommandIntegration.ts
🧬 Code graph analysis (2)
src/inversify.config.ts (1)
src/modules.ts (1)
  • CommandProcessExecutionFactory (9-9)
src/dbt_client/dbtFusionCommandIntegration.ts (2)
src/utils.ts (1)
  • getFirstWorkspacePath (120-131)
src/dbt_client/dbtIntegration.ts (1)
  • DBTCommandExecutionStrategy (42-47)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: test (windows-latest)
  • GitHub Check: test (ubuntu-latest)
  • GitHub Check: build (windows-latest)
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
src/dbt_client/dbtFusionCommandIntegration.ts (1)

35-36: All dependency declarations verified. The import of which in dbtFusionCommandIntegration.ts is properly backed: which is listed in dependencies (v^4.0.0) and @types/which is in devDependencies (v^3.0.4).

Comment on lines +41 to +118
async function resolveDbtfAlias(
commandProcessExecutionFactory: CommandProcessExecutionFactory,
terminal: DBTTerminal,
): Promise<string | null> {
const platform = process.platform;

// Windows: Use doskey to check for aliases/macros
if (platform === "win32") {
try {
const aliasProcess =
commandProcessExecutionFactory.createCommandProcessExecution({
command: "doskey",
args: ["/macros"],
cwd: getFirstWorkspacePath(),
});

const { stdout } = await aliasProcess.complete();

// Parse output like: "dbtf=path\to\dbt $*"
const match = stdout.match(/dbtf=(.+?)(?:\s+\$\*)?$/m);
if (match) {
const resolved = match[1].trim();
terminal.debug("resolveDbtfAlias", `Found Windows macro: ${resolved}`);
return resolved;
}
} catch (error) {
terminal.debug(
"resolveDbtfAlias",
"Windows doskey resolution failed",
error,
);
}
return null;
}

// Unix-like systems: Use shell alias command
const shell = process.env.SHELL || "/bin/bash";

try {
const aliasProcess =
commandProcessExecutionFactory.createCommandProcessExecution({
command: shell,
args: ["-i", "-c", "alias dbtf 2>/dev/null || echo ''"],
cwd: getFirstWorkspacePath(),
});

const { stdout } = await aliasProcess.complete();
terminal.debug("resolveDbtfAlias", `alias command output: ${stdout}`);

// Parse different alias formats
// Bash: alias dbtf='path' or alias dbtf="path"
// Zsh: dbtf: aliased to path
// Direct: dbtf=path
const bashMatch = stdout.match(/alias dbtf=['"](.+?)['"]/);
const zshMatch = stdout.match(/dbtf.*aliased to (.+?)$/m);
const directMatch = stdout.match(/dbtf=(.+?)$/m);

if (bashMatch) {
const resolved = bashMatch[1].trim();
terminal.debug("resolveDbtfAlias", `Found bash alias: ${resolved}`);
return resolved;
}
if (zshMatch) {
const resolved = zshMatch[1].trim();
terminal.debug("resolveDbtfAlias", `Found zsh alias: ${resolved}`);
return resolved;
}
if (directMatch) {
const resolved = directMatch[1].trim();
terminal.debug("resolveDbtfAlias", `Found direct alias: ${resolved}`);
return resolved;
}
} catch (error) {
terminal.debug("resolveDbtfAlias", "Shell alias resolution failed", error);
}

return null;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Alias probing via interactive shells can have side‑effects; consider safer approach.

Spawning an interactive shell (-i) sources user RC files, which may run arbitrary code and slow detection. Prefer:

  • Try PATH first (which("dbtf")) before alias probing.
  • If alias probing is required, add a short timeout and use a non‑interactive login shell (e.g., bash -lc) or parse known RC files directly.

🏁 Script executed:

#!/bin/bash
# Do we enforce timeouts on CommandProcessExecutionFactory executions?
rg -nP -C2 'createCommandProcessExecution\([^)]*\)' src | sed -n '1,120p'

Length of output: 0


I need to refine the search strategy since the initial command produced no output. Let me gather the necessary information with broader searches.


---

🏁 Script executed:

```shell
#!/bin/bash
# Find CommandProcessExecutionFactory definition and interface
fd -t f -e ts -e tsx | xargs grep -l 'CommandProcessExecutionFactory' | head -10

Length of output: 366


🏁 Script executed:

#!/bin/bash
# Look for createCommandProcessExecution usage with broader context
rg -n 'createCommandProcessExecution' src -A3 -B3

Length of output: 20906


🏁 Script executed:

#!/bin/bash
# Search for timeout-related configurations in the codebase
rg -n 'timeout|Timeout|duration|Duration' src/dbt_client/ | grep -iE '(timeout|duration)' | head -20

Length of output: 97


🏁 Script executed:

#!/bin/bash
# Examine the dbtFusionCommandIntegration file structure
cat src/dbt_client/dbtFusionCommandIntegration.ts | head -150

Length of output: 4325


I need to examine the CommandProcessExecutionFactory and CommandProcessExecution implementations to verify timeout capabilities.


---

🏁 Script executed:

```shell
#!/bin/bash
# Get the full CommandProcessExecutionFactory class definition
cat src/commandProcessExecution.ts

Length of output: 4465


🏁 Script executed:

#!/bin/bash
# Search for CommandProcessExecution interface or class
rg -n 'class CommandProcessExecution|interface CommandProcessExecution' src -A20

Length of output: 2432


🏁 Script executed:

#!/bin/bash
# Check if there are any timeout-related parameters in process execution
rg -n 'timeout|Timeout' src/commandProcessExecution.ts

Length of output: 0


Interactive shell with -i flag sources user RC files; add timeout and reverse lookup order.

The concerns in the review are valid:

  1. Interactive shell risk (confirmed): Line 83 uses -i flag which sources shell RC files (.bashrc, .zshrc, etc.), potentially executing arbitrary user code and adding startup overhead.

  2. No timeout enforcement (confirmed): CommandProcessExecutionFactory has no timeout parameter or mechanism. The CommandProcessExecution.complete() method can hang indefinitely with no timeout handling.

  3. Suboptimal lookup order (confirmed): Alias probing runs before the PATH check via which("dbt"). Reversing this order would avoid unnecessary interactive shell spawning in most cases.

Recommended fixes:

  • Add a timeout parameter to CommandProcessExecutionFactory.createCommandProcessExecution() and apply it in the spawn() method
  • Use non-interactive shell: change -i -c to -lc (login shell, non-interactive)
  • In resolveDbtfPath(), try which("dbtf") before alias probing to avoid RC file sourcing when unnecessary
🤖 Prompt for AI Agents
In src/dbt_client/dbtFusionCommandIntegration.ts around lines 41 to 118, the
alias probing uses an interactive shell (-i) with no timeout and runs before a
PATH lookup; update resolveDbtfAlias to spawn a non-interactive login shell by
changing args from ["-i", "-c", "alias dbtf 2>/dev/null || echo ''"] to ["-lc",
"alias dbtf 2>/dev/null || echo ''"], pass a timeout option into
commandProcessExecutionFactory.createCommandProcessExecution(...) (e.g. { ...,
timeout: SOME_MS }) so the underlying spawn/complete call cannot hang
indefinitely, and then flip the probing order in resolveDbtfPath (call
which("dbt") first and only call resolveDbtfAlias if which fails) so you avoid
sourcing user RC files unless necessary.

Comment on lines +121 to +153
* Resolves the dbtf command path with fallback chain:
* 1. Try to resolve shell alias for dbtf
* 2. Try to find dbt in PATH
* 3. Fallback to "dbt" as command name
* @returns The resolved path to the dbt/dbtf command
*/
async function resolveDbtfPath(
commandProcessExecutionFactory: CommandProcessExecutionFactory,
terminal: DBTTerminal,
): Promise<string> {
// Try to resolve alias via shell
const aliasResolved = await resolveDbtfAlias(
commandProcessExecutionFactory,
terminal,
);
if (aliasResolved) {
terminal.debug("resolveDbtfPath", `Resolved via alias: ${aliasResolved}`);
return aliasResolved;
}

// Try which/where for dbt
try {
const dbtPath = await which("dbt");
terminal.debug("resolveDbtfPath", `Found dbt in PATH: ${dbtPath}`);
return dbtPath;
} catch {
// dbt not in PATH either
}

// Fallback to just "dbt" command name
terminal.debug("resolveDbtfPath", "Falling back to 'dbt' command");
return "dbt";
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Resolver prefers “dbt” over “dbtf” — breaks stated goal; fix resolution order and fallback.

Current flow: alias(dbtf) → which("dbt") → "dbt". This can run dbt core instead of dbtf, making fusion detection fail. Prefer dbtf everywhere, with optional last‑chance dbt only if you explicitly validate it’s fusion.

Apply this diff to align with “use dbtf”:

- * 2. Try to find dbt in PATH
- * 3. Fallback to "dbt" as command name
+ * 2. Try to find dbtf in PATH
+ * 3. Fallback to "dbtf" as command name
@@
-  // Try which/where for dbt
+  // Try which/where for dbtf
   try {
-    const dbtPath = await which("dbt");
-    terminal.debug("resolveDbtfPath", `Found dbt in PATH: ${dbtPath}`);
-    return dbtPath;
-  } catch {
-    // dbt not in PATH either
+    const dbtfPath = await which("dbtf", { nothrow: true } as any);
+    if (dbtfPath) {
+      terminal.debug("resolveDbtfPath", `Found dbtf in PATH: ${dbtfPath}`);
+      return dbtfPath;
+    }
+  } catch {
+    // dbtf not in PATH either
   }
 
-  // Fallback to just "dbt" command name
-  terminal.debug("resolveDbtfPath", "Falling back to 'dbt' command");
-  return "dbt";
+  // Fallback to just "dbtf" command name
+  terminal.debug("resolveDbtfPath", "Falling back to 'dbtf' command");
+  return "dbtf";

Optional: If you keep a dbt fallback, only accept it when dbt --version contains “dbt‑fusion”, otherwise ignore it.

📝 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
* Resolves the dbtf command path with fallback chain:
* 1. Try to resolve shell alias for dbtf
* 2. Try to find dbt in PATH
* 3. Fallback to "dbt" as command name
* @returns The resolved path to the dbt/dbtf command
*/
async function resolveDbtfPath(
commandProcessExecutionFactory: CommandProcessExecutionFactory,
terminal: DBTTerminal,
): Promise<string> {
// Try to resolve alias via shell
const aliasResolved = await resolveDbtfAlias(
commandProcessExecutionFactory,
terminal,
);
if (aliasResolved) {
terminal.debug("resolveDbtfPath", `Resolved via alias: ${aliasResolved}`);
return aliasResolved;
}
// Try which/where for dbt
try {
const dbtPath = await which("dbt");
terminal.debug("resolveDbtfPath", `Found dbt in PATH: ${dbtPath}`);
return dbtPath;
} catch {
// dbt not in PATH either
}
// Fallback to just "dbt" command name
terminal.debug("resolveDbtfPath", "Falling back to 'dbt' command");
return "dbt";
}
* Resolves the dbtf command path with fallback chain:
* 1. Try to resolve shell alias for dbtf
* 2. Try to find dbtf in PATH
* 3. Fallback to "dbtf" as command name
* @returns The resolved path to the dbt/dbtf command
*/
async function resolveDbtfPath(
commandProcessExecutionFactory: CommandProcessExecutionFactory,
terminal: DBTTerminal,
): Promise<string> {
// Try to resolve alias via shell
const aliasResolved = await resolveDbtfAlias(
commandProcessExecutionFactory,
terminal,
);
if (aliasResolved) {
terminal.debug("resolveDbtfPath", `Resolved via alias: ${aliasResolved}`);
return aliasResolved;
}
// Try which/where for dbtf
try {
const dbtfPath = await which("dbtf", { nothrow: true } as any);
if (dbtfPath) {
terminal.debug("resolveDbtfPath", `Found dbtf in PATH: ${dbtfPath}`);
return dbtfPath;
}
} catch {
// dbtf not in PATH either
}
// Fallback to just "dbtf" command name
terminal.debug("resolveDbtfPath", "Falling back to 'dbtf' command");
return "dbtf";
}
🤖 Prompt for AI Agents
In src/dbt_client/dbtFusionCommandIntegration.ts around lines 121-153, the
resolver currently tries alias(dbtf) → which("dbt") → "dbt", which can cause
core dbt to be chosen; change the resolution order to prefer dbtf everywhere:
after the existing aliasResolved check, attempt which("dbtf") next and return it
if found; only then attempt which("dbt") as a last-resort fallback but validate
it by running `dbt --version` (or equivalent via the provided
commandProcessExecutionFactory/terminal) and accept it only if the version
output contains "dbt-fusion"; if that validation fails, do not return plain
"dbt" — instead fall back to the literal "dbtf" command name and log the
decision via terminal.debug.

Comment on lines +167 to 180
// Resolve dbtf command (handles aliases, symlinks, PATH)
const dbtfPath = await resolveDbtfPath(
this.commandProcessExecutionFactory,
this.terminal,
);
this.terminal.debug(
"DBTCLIDetection",
`Resolved dbtf command to: ${dbtfPath}`,
);

const checkDBTInstalledProcess =
this.commandProcessExecutionFactory.createCommandProcessExecution({
command: dbtPath,
command: dbtfPath,
args: ["--version"],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Potential break if alias resolves to multi‑word command.

If resolveDbtfAlias returns something like “pipx run dbtf”, passing it as command plus args=["--version"] may fail unless the process runner uses shell=true or supports a command+preArgs split.

If shell=true is not used, consider:

  • Returning { command, preArgs } from resolveDbtfPath, or
  • Splitting on whitespace respecting quotes, or
  • Always executing through a shell when the resolved string contains whitespace.

🏁 Script executed:

#!/bin/bash
# Inspect the process runner to confirm shell mode and arg handling
rg -nP -C3 'class\s+CommandProcessExecutionFactory|createCommandProcessExecution\(' src

# Look for child_process.spawn usage and shell:true
rg -nP -C2 'spawn\s*\(|shell\s*:\s*true' src

Length of output: 16237


Comment on lines +276 to +287
async initializeProject(): Promise<void> {
await super.initializeProject();
// Resolve dbtf command (handles aliases, symlinks, PATH)
this.dbtPath = await resolveDbtfPath(
this.commandProcessExecutionFactory,
this.terminal,
);
this.terminal.debug(
"DBTFusionInitialization",
`Resolved dbtf command to: ${this.dbtPath}`,
);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Initialize with dbtf path; ensure final fallback is “dbtf”.

Given the resolver fix above, this.dbtPath will reliably point to dbtf. If you add a dbt fallback, gate it by verifying it’s the fusion CLI to avoid running core by mistake.


🏁 Script executed:

#!/bin/bash
# Sanity: where is initializeProject called for Fusion projects?
rg -nP -C2 'initializeProject\s*\(' src | rg -nP 'DBTFusionCommandProjectIntegration' -n -C2

Length of output: 0


I need to search more broadly to understand the codebase structure. Let me try different approaches to verify the initialization and any fallback logic.


🏁 Script executed:

#!/bin/bash
# Search for initializeProject calls more broadly
echo "=== initializeProject calls in src ==="
rg -n 'initializeProject\s*\(' src --type ts

echo ""
echo "=== DBTFusionCommandIntegration class definition ==="
rg -n 'class\s+DBTFusionCommand.*Integration' src --type ts -A 5

echo ""
echo "=== resolveDbtfPath function ==="
rg -n 'resolveDbtfPath' src --type ts -B 2 -A 10

Length of output: 6351



🏁 Script executed:

#!/bin/bash
# Check how dbtPath is used after initialization
echo "=== dbtPath usage in file ==="
rg -n 'this\.dbtPath' src/dbt_client/dbtFusionCommandIntegration.ts -A 2 -B 2

echo ""
echo "=== Check if there's any CLI verification logic ==="
rg -n 'dbtf\|fusion.*cli\|verify.*cli' src/dbt_client/dbtFusionCommandIntegration.ts -i

echo ""
echo "=== Look at parent class initializeProject (DBTCloudProjectIntegration) ==="
rg -n 'class DBTCloudProjectIntegration' src/dbt_client/dbtCloudIntegration.ts -A 20

Length of output: 1590


Fallback command should be "dbtf" or gated with CLI verification, not bare "dbt".

The resolveDbtfPath() function (lines 127-153) currently falls back to "dbt" on line 152. For a Fusion integration, this risks invoking dbt core instead of the fusion CLI. Either:

  1. Change fallback to "dbtf", or
  2. Add verification/gating to confirm it's the fusion CLI before using the fallback

Currently there is no verification logic to distinguish between core and fusion CLI invocations.

🤖 Prompt for AI Agents
In src/dbt_client/dbtFusionCommandIntegration.ts around lines 276 to 287, the
code uses resolveDbtfPath which currently falls back to "dbt" and can mistakenly
invoke dbt-core; update resolveDbtfPath so its fallback is "dbtf" (not "dbt")
and add a verification step: after resolving a path, check the executable
name/version to confirm it is the Fusion CLI (e.g., executable basename equals
"dbtf" or run a lightweight version/identify command and validate output); if
verification fails, continue searching for "dbtf" on PATH and only fall back to
the literal "dbtf" when a verified Fusion CLI cannot be found, otherwise throw a
clear error.

Comment on lines +228 to 229
container.get(CommandProcessExecutionFactory),
);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

DI wiring looks correct; confirm factory signature/arity.

Passing CommandProcessExecutionFactory as the last ctor arg matches the updated DBTFusionCommandProjectIntegration constructor. Please also verify the factory’s declared parameter tuple and return type align with the actual lambda (it currently appears typed as [Uri, DiagnosticCollection] while only (projectRoot: Uri) is accepted, and the bound generic uses DBTCoreProjectIntegration). Prevents subtle DI/type errors later.

Run to double-check DI signatures:


🏁 Script executed:

#!/bin/bash
# Check factory symbols, arities, and ctor params
rg -nP -C2 'Factory<DBTFusionCommandProjectIntegration>|DBTFusionCommandProjectIntegration\s*constructor\(' src

# Ensure all "toFactory<...,[...]>" entries match the returned lambda arity
rg -nP -C1 'toFactory<\s*DBTFusionCommandProjectIntegration\s*,\s*\[.*\]\s*>' src/inversify.config.ts

Length of output: 729


🏁 Script executed:

#!/bin/bash
# Get full factory binding definition including parameter tuple
sed -n '208,225p' src/inversify.config.ts

echo "---"

# Find DBTFusionCommandProjectIntegration class/interface definition and constructor
rg -nP -C5 'class\s+DBTFusionCommandProjectIntegration|interface\s+DBTFusionCommandProjectIntegration' src

echo "---"

# Search for constructor parameters in DBTFusionCommandProjectIntegration
rg -nP -C8 'DBTFusionCommandProjectIntegration\s*\{' src

Length of output: 162


🏁 Script executed:

#!/bin/bash
# Get full factory binding definition including parameter tuple
sed -n '208,225p' src/inversify.config.ts

echo "---"

# Find DBTFusionCommandProjectIntegration class/interface definition and constructor
rg -nP -C5 'class\s+DBTFusionCommandProjectIntegration|interface\s+DBTFusionCommandProjectIntegration' src

echo "---"

# Search for constructor parameters in DBTFusionCommandProjectIntegration
rg -nP -C8 'DBTFusionCommandProjectIntegration\s*\{' src

Length of output: 1650


🏁 Script executed:

#!/bin/bash
# Get full DBTFusionCommandProjectIntegration constructor
sed -n '235,260p' src/dbt_client/dbtFusionCommandIntegration.ts

echo "---"

# Check if DBTCoreProjectIntegration is used elsewhere in similar factory patterns to understand the bind type
rg -nP -C3 'bind<\s*interfaces\.Factory<DBTCoreProjectIntegration>' src/inversify.config.ts

echo "---"

# Check what interface/type Factory is bound to for other similar integrations
rg -nP -C3 'toFactory<\s*DBT.*CommandProjectIntegration' src/inversify.config.ts

Length of output: 890


🏁 Script executed:

#!/bin/bash
# Look for other CommandProjectIntegration factory patterns to understand bind type pattern
rg -nP -B3 -A10 'toFactory<\s*DBTCloudProjectIntegration|toFactory<\s*DBTCoreCommandProjectIntegration' src/inversify.config.ts | head -50

Length of output: 0


🏁 Script executed:

#!/bin/bash
# Find all toFactory patterns to understand bind/toFactory type relationship
rg -nP -B2 'toFactory<' src/inversify.config.ts | head -80

echo "---"

# Search for all Factory string bindings to see naming patterns
rg -nP '"Factory<' src/inversify.config.ts

Length of output: 2663


🏁 Script executed:

#!/bin/bash
# Get parameter tuples for DBTCoreCommandProjectIntegration and DBTFusionCommandProjectIntegration
sed -n '179,195p' src/inversify.config.ts

echo "---"

sed -n '209,225p' src/inversify.config.ts

Length of output: 1402


Fix DI wiring: Correct factory type and parameter tuple mismatch.

Lines 209–213 have two critical issues:

  1. Line 209: bind<interfaces.Factory<DBTCoreProjectIntegration>> should be bind<interfaces.Factory<DBTFusionCommandProjectIntegration>> to match the string key and toFactory first type parameter.

  2. Line 212–213: Parameter tuple [Uri, DiagnosticCollection] should be [Uri] because the returned lambda only accepts (projectRoot: Uri), not two parameters. Compare to DBTCoreCommandProjectIntegration (lines 182–186), which correctly declares [Uri, DiagnosticCollection] and accepts both parameters in its lambda.

These mismatches will cause type/runtime errors when the factory is resolved and invoked.

🤖 Prompt for AI Agents
In src/inversify.config.ts around lines 228 to 229, the DI binding has a wrong
factory generic and an incorrect parameter tuple: change the bind generic from
interfaces.Factory<DBTCoreProjectIntegration> to
interfaces.Factory<DBTFusionCommandProjectIntegration> to match the string
key/toFactory target, and update the factory parameter tuple from [Uri,
DiagnosticCollection] to [Uri] so the returned lambda signature accepts only
(projectRoot: Uri); adjust the toFactory registration types accordingly so the
factory resolves and invokes with the correct types.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants