fix: detect and use dbtf command for fusion integration #1760
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -225,6 +225,7 @@ container | |
| container.get(DeferToProdService), | ||
| projectRoot, | ||
| container.get(AltimateRequest), | ||
| container.get(CommandProcessExecutionFactory), | ||
| ); | ||
|
Comment on lines
+228
to
229
Contributor
There was a problem hiding this comment. 🧩 Analysis chainDI 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.tsLength 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*\{' srcLength 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*\{' srcLength 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.tsLength 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 -50Length 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.tsLength 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.tsLength of output: 1402 Fix DI wiring: Correct factory type and parameter tuple mismatch. Lines 209–213 have two critical issues:
These mismatches will cause type/runtime errors when the factory is resolved and invoked. 🤖 Prompt for AI Agents |
||
| }; | ||
| }); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 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:
🏁 Script executed:
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.
Length of output: 366
🏁 Script executed:
Length of output: 20906
🏁 Script executed:
Length of output: 97
🏁 Script executed:
Length of output: 4325
I need to examine the
CommandProcessExecutionFactoryandCommandProcessExecutionimplementations to verify timeout capabilities.Length of output: 4465
🏁 Script executed:
Length of output: 2432
🏁 Script executed:
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:
Interactive shell risk (confirmed): Line 83 uses
-iflag which sources shell RC files (.bashrc, .zshrc, etc.), potentially executing arbitrary user code and adding startup overhead.No timeout enforcement (confirmed):
CommandProcessExecutionFactoryhas no timeout parameter or mechanism. TheCommandProcessExecution.complete()method can hang indefinitely with no timeout handling.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:
CommandProcessExecutionFactory.createCommandProcessExecution()and apply it in thespawn()method-i -cto-lc(login shell, non-interactive)resolveDbtfPath(), trywhich("dbtf")before alias probing to avoid RC file sourcing when unnecessary🤖 Prompt for AI Agents