feat(dbt-tools): auto-discover config with Windows compatibility#464
Merged
anandgupta42 merged 6 commits intomainfrom Mar 26, 2026
Merged
feat(dbt-tools): auto-discover config with Windows compatibility#464anandgupta42 merged 6 commits intomainfrom
anandgupta42 merged 6 commits intomainfrom
Conversation
…s for vscode integration - config.ts: add findProjectRoot() and discoverPython() (exported), update read() to auto-discover projectRoot and pythonPath at runtime when no config file exists — removes requirement to run `altimate-dbt init` before using any command - discoverPython() prioritises ALTIMATE_CODE_VIRTUAL_ENV (injected by vscode-altimate-mcp-server) over project-local venvs; tries python3 before python in each candidate location - dbt-resolve.ts: add tier 2 (ALTIMATE_CODE_PYTHON_PATH sibling) and tier 6 (ALTIMATE_CODE_VIRTUAL_ENV) so the dbt binary is found when altimate serve is spawned with a vscode-activated Python environment - init.ts: remove duplicated find()/python() helpers; import from config.ts - tests: add config.test.ts coverage for auto-discovery, findProjectRoot, discoverPython; add dbt-resolve.test.ts scenarios for ALTIMATE_CODE_PYTHON_PATH and ALTIMATE_CODE_VIRTUAL_ENV priority Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review.
Tip: disable this comment in your organization's Code Review settings.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
Disabled knowledge base sources:
📝 WalkthroughWalkthroughReplaced local init discovery with shared Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI (init)
participant Config as config module
participant FS as Filesystem
participant Env as Environment
CLI->>Config: init() / read()
Config->>FS: findProjectRoot(start=CWD) — check for dbt_project.yml up tree
alt projectRoot found
Config->>Env: inspect env vars (VIRTUAL_ENV, CONDA_PREFIX, ALTIMATE_CODE_PYTHON_PATH)
Config->>FS: probe project-local venvs (.venv, venv, env) for platform bin/Scripts
Config->>FS: fallback to which/where python3|python
Config-->>CLI: return synthesized config with projectRoot and pythonPath
else
Config-->>CLI: return null
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/dbt-tools/src/dbt-resolve.ts (2)
165-177: Duplicate comment numbering for tiers 8 and 9.Lines 132 and 165 both say
// 8., and lines 150 and 177 both say// 9.. After inserting the new tiers, the comment numbers need to be updated to reflect the actual priority order from the docstring (which goes up to 10).✏️ Suggested fix
- // 8. `which dbt` on current PATH (catches pipx ~/.local/bin, system pip, homebrew, etc.) + // 9. `which dbt` on current PATH (catches pipx ~/.local/bin, system pip, homebrew, etc.) try { const whichDbt = execFileSync("which", ["dbt"], { ... } catch {} - // 9. Common known locations (last resort) + // 10. Common known locations (last resort)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/dbt-tools/src/dbt-resolve.ts` around lines 165 - 177, The inline tier comment numbers are duplicated after adding new detection tiers; update the sequential comment markers to match the docstring priority (ensure comments for the blocks that currently show "// 8." and "// 9." are renumbered correctly up through "// 10.") so the ordering around the execFileSync("which", ["dbt"], ...) block and the subsequent "Common known locations" block reflects their true priority; locate these comments near the candidates.push calls in dbt-resolve.ts and adjust the numeric labels to the correct tier numbers.
91-97: Comment numbering is stale after insertion of new tiers.The comment on line 91 says
// 3.but based on the priority list in the docstring (lines 44-54), this should be// 4.since tiers 2 and 3 are nowALTIMATE_CODE_PYTHON_PATHsibling andpythonPathsibling respectively.✏️ Suggested fix
- // 3. Project-local .venv/bin/dbt (uv, pdm, venv, poetry in-project, rye) + // 4. Project-local .venv/bin/dbt (uv, pdm, venv, poetry in-project, rye)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/dbt-tools/src/dbt-resolve.ts` around lines 91 - 97, Update the inline comment above the project-local venv loop from "// 3." to "// 4." so the numbering matches the priority list in the docstring; locate the block that checks "if (projectRoot) { for (const venvDir of [".venv", "venv", "env"]) { ... } }" (references: projectRoot, venvDir, join, candidates) and change the numeric comment label accordingly.packages/dbt-tools/test/config.test.ts (1)
56-90: Consider test isolation withprocess.chdir()and module caching.Using
process.chdir()can cause issues if tests run in parallel, as the current working directory is process-global. While you correctly restore it in thefinallyblock, any test running concurrently would be affected.Additionally, the dynamic
await import("../src/config")pattern may not provide fresh module state due to module caching—subsequent imports return the same cached module instance.If you observe flaky tests, consider:
- Using
jest.isolateModules()equivalent in Bun, or- Refactoring the functions to accept
cwdas a parameter for testability🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/dbt-tools/test/config.test.ts` around lines 56 - 90, The tests mutate process.cwd() and rely on a dynamic import of the module-level read function, which can cause cross-test interference and stale module state; update the tests (or the implementation) so they don't rely on a global chdir and cached module state: either (A) refactor the read function in src/config to accept an optional cwd/projectRoot parameter and call read(dir) from the tests instead of using process.chdir, or (B) wrap the dynamic import and invocation in an isolation mechanism (e.g., Bun/Jest isolateModules or explicitly clear the module cache before importing) so each test gets a fresh module instance; locate the read export in src/config and the two tests here to apply the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/dbt-tools/src/dbt-resolve.ts`:
- Around line 44-54: Priority mismatch: resolveDbt() currently checks
project-local .venv before ALTIMATE_CODE_VIRTUAL_ENV, which can cause dbt to be
resolved to a different Python than discoverPython(). Update resolveDbt() so its
lookup order aligns with discoverPython(): keep ALTIMATE_DBT_PATH highest, then
check ALTIMATE_CODE_PYTHON_PATH sibling and pythonPath sibling, then honor
ALTIMATE_CODE_VIRTUAL_ENV (env var) before falling back to project-local .venv,
then VIRTUAL_ENV and CONDA_PREFIX; adjust the code paths in resolveDbt() and its
comment block accordingly (references: resolveDbt(), discoverPython(),
ALTIMATE_CODE_VIRTUAL_ENV, ALTIMATE_CODE_PYTHON_PATH, project-local .venv).
---
Nitpick comments:
In `@packages/dbt-tools/src/dbt-resolve.ts`:
- Around line 165-177: The inline tier comment numbers are duplicated after
adding new detection tiers; update the sequential comment markers to match the
docstring priority (ensure comments for the blocks that currently show "// 8."
and "// 9." are renumbered correctly up through "// 10.") so the ordering around
the execFileSync("which", ["dbt"], ...) block and the subsequent "Common known
locations" block reflects their true priority; locate these comments near the
candidates.push calls in dbt-resolve.ts and adjust the numeric labels to the
correct tier numbers.
- Around line 91-97: Update the inline comment above the project-local venv loop
from "// 3." to "// 4." so the numbering matches the priority list in the
docstring; locate the block that checks "if (projectRoot) { for (const venvDir
of [".venv", "venv", "env"]) { ... } }" (references: projectRoot, venvDir, join,
candidates) and change the numeric comment label accordingly.
In `@packages/dbt-tools/test/config.test.ts`:
- Around line 56-90: The tests mutate process.cwd() and rely on a dynamic import
of the module-level read function, which can cause cross-test interference and
stale module state; update the tests (or the implementation) so they don't rely
on a global chdir and cached module state: either (A) refactor the read function
in src/config to accept an optional cwd/projectRoot parameter and call read(dir)
from the tests instead of using process.chdir, or (B) wrap the dynamic import
and invocation in an isolation mechanism (e.g., Bun/Jest isolateModules or
explicitly clear the module cache before importing) so each test gets a fresh
module instance; locate the read export in src/config and the two tests here to
apply the change.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 183bb31a-8db2-403b-b865-335d8db67044
📒 Files selected for processing (5)
packages/dbt-tools/src/commands/init.tspackages/dbt-tools/src/config.tspackages/dbt-tools/src/dbt-resolve.tspackages/dbt-tools/test/config.test.tspackages/dbt-tools/test/dbt-resolve.test.ts
- Add `ALTIMATE_CODE_PYTHON_PATH` as highest priority in `discoverPython()` so VS Code's explicit Python selection is not silently ignored - Align `ALTIMATE_CODE_VIRTUAL_ENV` priority: move to tier 4 in `resolveDbt()` (before project-local `.venv`) to match `discoverPython()` priority and prevent Python/dbt resolving from different environments - Add `JSON.parse` error handling in `read()` — malformed config file now falls back to auto-discovery instead of crashing - Fix stale inline comment numbering in `dbt-resolve.ts` (1-11) - Fix environment-dependent `read returns null` test by setting CWD to temp dir - Add tests: `ALTIMATE_CODE_PYTHON_PATH` priority, malformed config fallback - Run prettier on `dbt-resolve.ts` and `dbt-resolve.test.ts` Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/dbt-tools/src/dbt-resolve.ts (1)
114-131:⚠️ Potential issue | 🟠 MajorAlign
VIRTUAL_ENVvsCONDA_PREFIXorder withdiscoverPython().Line 114-131 still prefers
CONDA_PREFIXbeforeVIRTUAL_ENV, whilediscoverPython()does the opposite. This can resolve dbt from a different environment than the selected Python.🔧 Proposed fix
- // 6. CONDA_PREFIX (conda/mamba/micromamba — set after `conda activate`) - const condaPrefix = process.env.CONDA_PREFIX - if (condaPrefix) { - candidates.push({ - path: join(condaPrefix, "bin", "dbt"), - source: `CONDA_PREFIX (${condaPrefix})`, - binDir: join(condaPrefix, "bin"), - }) - } - - // 7. VIRTUAL_ENV (set by venv/virtualenv activate scripts) + // 6. VIRTUAL_ENV (set by venv/virtualenv activate scripts) const virtualEnv = process.env.VIRTUAL_ENV if (virtualEnv) { candidates.push({ path: join(virtualEnv, "bin", "dbt"), source: `VIRTUAL_ENV (${virtualEnv})`, binDir: join(virtualEnv, "bin"), }) } + + // 7. CONDA_PREFIX (conda/mamba/micromamba — set after `conda activate`) + const condaPrefix = process.env.CONDA_PREFIX + if (condaPrefix) { + candidates.push({ + path: join(condaPrefix, "bin", "dbt"), + source: `CONDA_PREFIX (${condaPrefix})`, + binDir: join(condaPrefix, "bin"), + }) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/dbt-tools/src/dbt-resolve.ts` around lines 114 - 131, The current resolution prefers CONDA_PREFIX before VIRTUAL_ENV (variables condaPrefix and virtualEnv), which is inconsistent with discoverPython(); swap the order so VIRTUAL_ENV is checked and pushed into candidates before CONDA_PREFIX to ensure the virtualenv-discovered Python and dbt are resolved from the same environment (update the block that constructs candidates so the virtualEnv block runs prior to the condaPrefix block).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/dbt-tools/src/config.ts`:
- Around line 94-95: The code currently returns JSON.parse(raw) as Config
without runtime validation; after parsing the raw string, validate the resulting
object’s shape (required fields like projectRoot and dbtIntegration and their
expected types/enum values) before returning it from the function that contains
"JSON.parse(raw) as Config"; if validation fails, throw a clear error in the
catch/validation path instead of returning the unchecked object. Use a runtime
schema validator (e.g., zod) or explicit property/type checks against the Config
interface, and replace the blind cast with the validated Config instance so
downstream consumers receive a correct shape.
- Line 43: The Windows path/command assumptions must be removed: update
configDir() to use os.homedir() (or homedir()) instead of process.env.HOME, and
in discoverPython() replace POSIX-only checks (e.g., "bin/..." and `which`) with
platform-aware logic that also checks Windows locations ("Scripts\\...") and
uses `where` on Windows; additionally, when handling altPython (the variable
referenced in the line with `if (altPython && existsSync(altPython)) return
altPython`) validate that the path is an executable file (not just exists)
before returning it, and update other discovery steps (the calls that check
`bin/...`, `which`, and similar paths) to perform the same executable validation
and platform branching so discovery works on both POSIX and Windows.
In `@packages/dbt-tools/src/dbt-resolve.ts`:
- Line 210: The PATH concatenation on the env creation line can produce
"PATH=<bin>:undefined" when process.env.PATH is missing; update the construction
around resolved.binDir (the const env assignment that currently uses
`${resolved.binDir}:${process.env.PATH}`) to guard against a missing PATH by
using the same nullish coalescing approach as buildDbtEnv (e.g., use
process.env.PATH ?? '' or equivalent) so the PATH becomes
`${resolved.binDir}:${process.env.PATH ?? ''}` when resolved.binDir is present,
otherwise keep process.env unchanged.
---
Outside diff comments:
In `@packages/dbt-tools/src/dbt-resolve.ts`:
- Around line 114-131: The current resolution prefers CONDA_PREFIX before
VIRTUAL_ENV (variables condaPrefix and virtualEnv), which is inconsistent with
discoverPython(); swap the order so VIRTUAL_ENV is checked and pushed into
candidates before CONDA_PREFIX to ensure the virtualenv-discovered Python and
dbt are resolved from the same environment (update the block that constructs
candidates so the virtualEnv block runs prior to the condaPrefix block).
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 4eb605ba-2a98-4296-8d9a-64f3fbabc0ee
📒 Files selected for processing (4)
packages/dbt-tools/src/config.tspackages/dbt-tools/src/dbt-resolve.tspackages/dbt-tools/test/config.test.tspackages/dbt-tools/test/dbt-resolve.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/dbt-tools/test/config.test.ts
packages/dbt-tools/src/config.ts
Outdated
| export function discoverPython(projectRoot: string): string { | ||
| // ALTIMATE_CODE_PYTHON_PATH (explicit selection from vscode-altimate-mcp-server — highest priority) | ||
| const altPython = process.env.ALTIMATE_CODE_PYTHON_PATH | ||
| if (altPython && existsSync(altPython)) return altPython |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether the repo expects Windows/cross-platform behavior and where POSIX-only assumptions exist.
rg -n -C2 '(Windows|win32|cross[- ]platform|VS Code)' README.md packages/dbt-tools
rg -n -C2 'which\\(|bin/python3|bin/python|Scripts/python.exe|process\\.platform|win32' packages/dbt-tools/src packages/dbt-tools/testRepository: AltimateAI/altimate-code
Length of output: 808
🏁 Script executed:
cat packages/dbt-tools/src/config.tsRepository: AltimateAI/altimate-code
Length of output: 3447
🏁 Script executed:
# Check project structure and platform-specific considerations
fd -t f "(package.json|tsconfig|README)" packages/dbt-tools --max-depth 2Repository: AltimateAI/altimate-code
Length of output: 132
🏁 Script executed:
# Search for any existing platform-specific code in the project
rg -n "process\.platform|win32|Scripts" packages/dbt-tools/src --max-count 10Repository: AltimateAI/altimate-code
Length of output: 50
🏁 Script executed:
# Check if the project has any Windows/cross-platform references
rg -n "(Windows|cross.platform|Cursor|Claude Code)" README.md --max-count 5Repository: AltimateAI/altimate-code
Length of output: 579
Fix Windows path assumptions in Python discovery and config directory handling.
The discoverPython() function hardcodes POSIX-specific paths (bin/...) and commands (which), breaking on Windows which uses Scripts/... and where. Additionally, configDir() (line 18) relies on process.env.HOME, which is not set on Windows—use homedir() directly instead. Line 43 should also validate that the path is an executable file, not just that it exists.
Affected lines: 18, 43, 49, 57, 66, 75, 83.
Suggested patch
function configDir() {
- return join(process.env.HOME || homedir(), ".altimate-code")
+ return join(homedir(), ".altimate-code")
}
export function discoverPython(projectRoot: string): string {
+ const isWin = process.platform === "win32"
+ const binDir = isWin ? "Scripts" : "bin"
+ const candidates = isWin ? ["python.exe", "python3.exe", "python", "python3"] : ["python3", "python"]
// ALTIMATE_CODE_PYTHON_PATH (explicit selection from vscode-altimate-mcp-server — highest priority)
const altPython = process.env.ALTIMATE_CODE_PYTHON_PATH
if (altPython && existsSync(altPython)) return altPython
// ALTIMATE_CODE_VIRTUAL_ENV (injected by vscode-altimate-mcp-server — explicit user selection wins)
const altVenv = process.env.ALTIMATE_CODE_VIRTUAL_ENV
if (altVenv) {
- for (const bin of ["python3", "python"]) {
- const py = join(altVenv, "bin", bin)
+ for (const bin of candidates) {
+ const py = join(altVenv, binDir, bin)
if (existsSync(py)) return py
}
}
// Project-local venvs (uv, pdm, venv, poetry in-project, rye)
for (const venvDir of [".venv", "venv", "env"]) {
- for (const bin of ["python3", "python"]) {
- const py = join(projectRoot, venvDir, "bin", bin)
+ for (const bin of candidates) {
+ const py = join(projectRoot, venvDir, binDir, bin)
if (existsSync(py)) return py
}
}
// VIRTUAL_ENV (set by activate scripts)
const virtualEnv = process.env.VIRTUAL_ENV
if (virtualEnv) {
- for (const bin of ["python3", "python"]) {
- const py = join(virtualEnv, "bin", bin)
+ for (const bin of candidates) {
+ const py = join(virtualEnv, binDir, bin)
if (existsSync(py)) return py
}
}
// CONDA_PREFIX
const condaPrefix = process.env.CONDA_PREFIX
if (condaPrefix) {
- for (const bin of ["python3", "python"]) {
- const py = join(condaPrefix, "bin", bin)
+ for (const bin of candidates) {
+ const py = join(condaPrefix, binDir, bin)
if (existsSync(py)) return py
}
}
// PATH-based discovery
- for (const cmd of ["python3", "python"]) {
+ for (const cmd of isWin ? ["python", "py"] : ["python3", "python"]) {
try {
- return execFileSync("which", [cmd], { encoding: "utf-8" }).trim()
+ const locator = isWin ? "where" : "which"
+ return execFileSync(locator, [cmd], { encoding: "utf-8" }).trim().split(/\r?\n/)[0]!
} catch {}
}
return "python3"
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/dbt-tools/src/config.ts` at line 43, The Windows path/command
assumptions must be removed: update configDir() to use os.homedir() (or
homedir()) instead of process.env.HOME, and in discoverPython() replace
POSIX-only checks (e.g., "bin/..." and `which`) with platform-aware logic that
also checks Windows locations ("Scripts\\...") and uses `where` on Windows;
additionally, when handling altPython (the variable referenced in the line with
`if (altPython && existsSync(altPython)) return altPython`) validate that the
path is an executable file (not just exists) before returning it, and update
other discovery steps (the calls that check `bin/...`, `which`, and similar
paths) to perform the same executable validation and platform branching so
discovery works on both POSIX and Windows.
Comment on lines
+94
to
+95
| return JSON.parse(raw) as Config | ||
| } catch { |
There was a problem hiding this comment.
Validate parsed config shape before returning it.
JSON.parse success does not guarantee a valid Config. A malformed-but-valid object (missing projectRoot, wrong dbtIntegration, etc.) will propagate and can break downstream consumers.
Suggested patch
type Config = {
projectRoot: string
pythonPath: string
dbtIntegration: string
queryLimit: number
}
+
+function isConfig(value: unknown): value is Config {
+ if (!value || typeof value !== "object") return false
+ const v = value as Record<string, unknown>
+ return (
+ typeof v.projectRoot === "string" &&
+ typeof v.pythonPath === "string" &&
+ typeof v.dbtIntegration === "string" &&
+ typeof v.queryLimit === "number"
+ )
+}
async function read(): Promise<Config | null> {
const p = configPath()
if (existsSync(p)) {
try {
const raw = await readFile(p, "utf-8")
- return JSON.parse(raw) as Config
+ const parsed: unknown = JSON.parse(raw)
+ if (isConfig(parsed)) return parsed
} catch {
// Malformed config — fall through to auto-discovery
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/dbt-tools/src/config.ts` around lines 94 - 95, The code currently
returns JSON.parse(raw) as Config without runtime validation; after parsing the
raw string, validate the resulting object’s shape (required fields like
projectRoot and dbtIntegration and their expected types/enum values) before
returning it from the function that contains "JSON.parse(raw) as Config"; if
validation fails, throw a clear error in the catch/validation path instead of
returning the unchecked object. Use a runtime schema validator (e.g., zod) or
explicit property/type checks against the Config interface, and replace the
blind cast with the validated Config instance so downstream consumers receive a
correct shape.
| const env = resolved.binDir | ||
| ? { ...process.env, PATH: `${resolved.binDir}:${process.env.PATH}` } | ||
| : process.env | ||
| const env = resolved.binDir ? { ...process.env, PATH: `${resolved.binDir}:${process.env.PATH}` } : process.env |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find and examine the file
fd "dbt-resolve.ts" --type fRepository: AltimateAI/altimate-code
Length of output: 105
🏁 Script executed:
# Read the file around line 210 to see context
cat -n packages/dbt-tools/src/dbt-resolve.ts | sed -n '200,220p'Repository: AltimateAI/altimate-code
Length of output: 991
🏁 Script executed:
# Check Node.js process.env behavior and verify the concern
# Also check if there are other similar PATH concatenations in the codebase
rg "process\.env\.PATH" packages/dbt-tools/src/ -A 2 -B 2Repository: AltimateAI/altimate-code
Length of output: 542
🏁 Script executed:
# Check if there are any tests or documentation about environment handling
rg "process\.env" packages/dbt-tools/src/dbt-resolve.ts | head -20
# Also check the type definition of ResolvedDbt to understand the full context
rg "type ResolvedDbt|interface ResolvedDbt" packages/dbt-tools/src/ -A 5Repository: AltimateAI/altimate-code
Length of output: 1675
🏁 Script executed:
# Check Node.js documentation behavior and verify if process.env.PATH can be undefined
# Also look for how environment is used elsewhere in the codebase
rg "execFileSync|spawnSync" packages/dbt-tools/src/ -B 2 -A 2 | head -30Repository: AltimateAI/altimate-code
Length of output: 1981
🏁 Script executed:
# Verify Node.js behavior with undefined PATH
# Check if there are any environment setup or tests that might show how PATH is handled
rg "PATH" packages/dbt-tools/src/dbt-resolve.ts -B 1 -A 1
# Also check if there's any documentation or comments about environment handling
rg "binDir" packages/dbt-tools/src/dbt-resolve.ts -B 2 -A 2Repository: AltimateAI/altimate-code
Length of output: 5142
Guard PATH concatenation when PATH is unset.
Line 210 can produce PATH="<bin>:undefined" if process.env.PATH is missing. The codebase already uses the correct pattern with the nullish coalescing operator in the buildDbtEnv function—apply the same approach here.
🔧 Proposed fix
- const env = resolved.binDir ? { ...process.env, PATH: `${resolved.binDir}:${process.env.PATH}` } : process.env
+ const env = resolved.binDir
+ ? { ...process.env, PATH: `${resolved.binDir}:${process.env.PATH ?? ""}` }
+ : process.env📝 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 env = resolved.binDir ? { ...process.env, PATH: `${resolved.binDir}:${process.env.PATH}` } : process.env | |
| const env = resolved.binDir | |
| ? { ...process.env, PATH: `${resolved.binDir}:${process.env.PATH ?? ""}` } | |
| : process.env |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/dbt-tools/src/dbt-resolve.ts` at line 210, The PATH concatenation on
the env creation line can produce "PATH=<bin>:undefined" when process.env.PATH
is missing; update the construction around resolved.binDir (the const env
assignment that currently uses `${resolved.binDir}:${process.env.PATH}`) to
guard against a missing PATH by using the same nullish coalescing approach as
buildDbtEnv (e.g., use process.env.PATH ?? '' or equivalent) so the PATH becomes
`${resolved.binDir}:${process.env.PATH ?? ''}` when resolved.binDir is present,
otherwise keep process.env unchanged.
…tibility Drop ALTIMATE_CODE_PYTHON_PATH and ALTIMATE_CODE_VIRTUAL_ENV — dbt and Python will be on PATH anyway. Add Windows support: use Scripts/ instead of bin/, .exe suffix for binaries, where instead of which, skip Unix-only pyenv/asdf resolution, and add Windows-specific known fallback paths. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/dbt-tools/src/dbt-resolve.ts (1)
214-214:⚠️ Potential issue | 🟠 MajorFix PATH construction in Line 214 for Windows and missing PATH values.
Line 214 still hardcodes
:and can produce...:undefined. This breaks cross-platform intent (Windows requires;) and repeats the previously flagged issue.🔧 Proposed fix
-import { dirname, join } from "path" +import { delimiter, dirname, join } from "path" @@ - const env = resolved.binDir ? { ...process.env, PATH: `${resolved.binDir}:${process.env.PATH}` } : process.env + const env = resolved.binDir + ? { ...process.env, PATH: `${resolved.binDir}${delimiter}${process.env.PATH ?? ""}` } + : process.envAlso align
buildDbtEnvfor consistency (outside this line range):- env.PATH = `${resolved.binDir}:${env.PATH ?? ""}` + env.PATH = `${resolved.binDir}${delimiter}${env.PATH ?? ""}`#!/bin/bash # Verify PATH concatenation sites and delimiter usage in dbt resolver. rg -n -C2 'PATH:\s*`\$\{[^}]+\}:\$\{[^}]*PATH[^}]*\}`|env\.PATH\s*=\s*`\$\{[^}]+\}:\$\{[^}]*PATH[^}]*\}`' packages/dbt-tools/src/dbt-resolve.ts rg -n -C2 '\bdelimiter\b' packages/dbt-tools/src/dbt-resolve.ts🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/dbt-tools/src/dbt-resolve.ts` at line 214, The PATH construction uses a hardcoded ':' and can produce '...:undefined' on missing PATH and on Windows; update the env creation in the resolved.binDir branch (the const env assignment referencing resolved.binDir) to join resolved.binDir with the existing PATH using Node's path.delimiter and default to an empty string or omit the delimiter when process.env.PATH is falsy, e.g. compute const pathDelimiter = require('path').delimiter and build PATH as resolved.binDir + (process.env.PATH ? pathDelimiter + process.env.PATH : ''), and mirror the same approach in buildDbtEnv to keep behavior consistent across both places.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/dbt-tools/test/config.test.ts`:
- Around line 164-193: The tests for discoverPython are flaking when
ALTIMATE_CODE_PYTHON_PATH or ALTIMATE_CODE_VIRTUAL_ENV are set in the host
environment; modify the test setup to save and clear these two env vars before
each test and restore them in afterEach so discoverPython runs in a controlled
environment. Specifically, in the beforeEach/afterEach surrounding the tests
that call discoverPython, capture process.env.ALTIMATE_CODE_PYTHON_PATH and
process.env.ALTIMATE_CODE_VIRTUAL_ENV, delete or set them to undefined before
invoking discoverPython, and restore the original values in afterEach so other
tests are unaffected.
---
Duplicate comments:
In `@packages/dbt-tools/src/dbt-resolve.ts`:
- Line 214: The PATH construction uses a hardcoded ':' and can produce
'...:undefined' on missing PATH and on Windows; update the env creation in the
resolved.binDir branch (the const env assignment referencing resolved.binDir) to
join resolved.binDir with the existing PATH using Node's path.delimiter and
default to an empty string or omit the delimiter when process.env.PATH is falsy,
e.g. compute const pathDelimiter = require('path').delimiter and build PATH as
resolved.binDir + (process.env.PATH ? pathDelimiter + process.env.PATH : ''),
and mirror the same approach in buildDbtEnv to keep behavior consistent across
both places.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: c1e588ec-c8cb-406f-ae96-a96b1b8207a6
📒 Files selected for processing (4)
packages/dbt-tools/src/config.tspackages/dbt-tools/src/dbt-resolve.tspackages/dbt-tools/test/config.test.tspackages/dbt-tools/test/dbt-resolve.test.ts
✅ Files skipped from review due to trivial changes (1)
- packages/dbt-tools/test/dbt-resolve.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/dbt-tools/src/config.ts
mdesmet
changed the title
anandgupta42
changed the title
- Use `path.delimiter` instead of hardcoded `:` in `validateDbt()` and `buildDbtEnv()` so PATH injection works on Windows (`;` separator) - Fix Conda Python discovery on Windows: check env root directly since Conda places `python.exe` at `%CONDA_PREFIX%\python.exe`, not in `Scripts/` - Add `timeout: 5_000` to `execFileSync` in `discoverPython()` to match the same pattern used in `dbt-resolve.ts` Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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
2 participants
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.
What does this PR do?
Auto-discover dbt project root and Python path when no config file exists, removing the requirement to run
altimate-dbt initbefore using any command. Adds full Windows compatibility for Python and dbt binary resolution.Changes:
config.ts: addfindProjectRoot()anddiscoverPython()(exported), updateread()to auto-discoverprojectRootandpythonPathat runtime when no config file exists. Gracefully handles malformed config JSON by falling back to auto-discovery.discoverPython()checks project-local venvs →VIRTUAL_ENV→CONDA_PREFIX→which/wherepython; triespython3beforepythonon Unix,python.exeon Windowsdbt-resolve.ts: platform-aware resolution usingScripts/vsbin/and.exesuffix on Windows;whereinstead ofwhichon Windows; pyenv/asdf shim resolution skipped on Windowsinit.ts: remove duplicatedfind()/python()helpers; import fromconfig.tsType of change
Issue for this PR
Improves developer experience — no linked issue.
How did you verify your code works?
findProjectRoot,discoverPython, malformed config fallbackChecklist
Co-Authored-By: Claude Sonnet 4.6 noreply@anthropic.com
Co-Authored-By: Claude Opus 4.6 (1M context) noreply@anthropic.com
Summary by CodeRabbit
New Features
Bug Fixes