feat: dbt_project_health tool — reuse altimate-core dbt health checks#1030
feat: dbt_project_health tool — reuse altimate-core dbt health checks#1030mdesmet wants to merge 2 commits into
Conversation
Wires the new `altimate_core::dbt::health` checks (altimate-core-internal, requires @altimateai/altimate-core 0.8.0) into altimate-code: - Bridge method type `altimate_core.dbt_project_health` + Dispatcher registration calling core.dbtProjectHealth. - New DbtProjectHealthTool: runs dbt health checks directly against a dbt project directory (no manifest required), optional config + catalog paths, formats findings grouped by severity. - Registers the tool in the tool registry and altimate barrel export; bumps the @altimateai/altimate-core dependency 0.5.1 -> 0.8.0. Co-Authored-By: Claude Opus 4.8 (1M context) <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 for a one-time review, or @claude review always to subscribe this PR to a review on every future push.
Tip: disable this comment in your organization's Code Review settings.
|
This PR doesn't fully meet our contributing guidelines and PR template. What needs to be fixed:
Please edit this PR description to address the above within 2 hours, or it will be automatically closed. If you believe this was flagged incorrectly, please let a maintainer know. |
📝 WalkthroughWalkthroughAdds built-in Changesdbt health tooling
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Command
participant DbtHealthConfigTool
participant DbtProjectHealthTool
participant Dispatcher
participant AltimateCore
Command->>DbtHealthConfigTool: Infer and write dbt health config
DbtHealthConfigTool->>Dispatcher: Call altimate_core.dbt_health_infer_config
Dispatcher->>AltimateCore: Send project directory
AltimateCore-->>Dispatcher: Return inferred YAML
DbtHealthConfigTool-->>Command: Return written path and check count
Command->>DbtProjectHealthTool: Evaluate project health
DbtProjectHealthTool->>Dispatcher: Call altimate_core.dbt_project_health
Dispatcher->>AltimateCore: Send project, config, and catalog paths
AltimateCore-->>Dispatcher: Return JSON findings
DbtProjectHealthTool-->>Command: Return sorted findings and severity counts
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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: 2
🤖 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 `@packages/opencode/src/altimate/tools/dbt-project-health.ts`:
- Around line 37-40: Update the config_path handling in the dbt project health
tool so an explicitly supplied configuration file that is missing or unreadable
produces a clear tool error instead of falling back to default checks. Validate
file existence and propagate read failures, while preserving the current
behavior when config_path is not provided.
- Around line 57-63: Update the findings handling in the dbt project health flow
to validate result.data.findings with a runtime schema before copying or sorting
it. Reject malformed or missing payloads by returning the existing error
response, while preserving the current severity and code ordering for valid
HealthFinding arrays.
🪄 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: 71c27c6d-b7a3-4fad-9d71-6ce242129103
📒 Files selected for processing (6)
packages/opencode/package.jsonpackages/opencode/src/altimate/index.tspackages/opencode/src/altimate/native/altimate-core.tspackages/opencode/src/altimate/native/types.tspackages/opencode/src/altimate/tools/dbt-project-health.tspackages/opencode/src/tool/registry.ts
| if (args.config_path) { | ||
| const file = Bun.file(args.config_path) | ||
| if (await file.exists()) config_json = await file.text() | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not silently ignore an explicitly supplied config file.
When config_path is set but the file is missing or unreadable, the tool runs with default checks and reports results as if the requested configuration was applied. Return a clear tool error instead.
Suggested handling
if (args.config_path) {
- const file = Bun.file(args.config_path)
- if (await file.exists()) config_json = await file.text()
+ try {
+ config_json = await Bun.file(args.config_path).text()
+ } catch (error) {
+ const message = error instanceof Error ? error.message : String(error)
+ return {
+ title: "dbt health: ERROR",
+ metadata: { error: message },
+ output: `Failed to read dbt health config: ${message}`,
+ }
+ }
}📝 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.
| if (args.config_path) { | |
| const file = Bun.file(args.config_path) | |
| if (await file.exists()) config_json = await file.text() | |
| } | |
| if (args.config_path) { | |
| try { | |
| config_json = await Bun.file(args.config_path).text() | |
| } catch (error) { | |
| const message = error instanceof Error ? error.message : String(error) | |
| return { | |
| title: "dbt health: ERROR", | |
| metadata: { error: message }, | |
| output: `Failed to read dbt health config: ${message}`, | |
| } | |
| } | |
| } |
🤖 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 `@packages/opencode/src/altimate/tools/dbt-project-health.ts` around lines 37 -
40, Update the config_path handling in the dbt project health tool so an
explicitly supplied configuration file that is missing or unreadable produces a
clear tool error instead of falling back to default checks. Validate file
existence and propagate read failures, while preserving the current behavior
when config_path is not provided.
| const findings = ((result.data.findings as HealthFinding[] | undefined) ?? []).slice() | ||
| findings.sort( | ||
| (a, b) => (SEVERITY_ORDER[a.severity] ?? 3) - (SEVERITY_ORDER[b.severity] ?? 3) || a.code.localeCompare(b.code), | ||
| ) | ||
|
|
||
| const counts = { error: 0, warning: 0, info: 0 } as Record<string, number> | ||
| for (const f of findings) counts[f.severity] = (counts[f.severity] ?? 0) + 1 |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,220p' packages/opencode/src/altimate/tools/dbt-project-health.ts
printf '\n---- search HealthFinding ----\n'
rg -n "HealthFinding|findings" packages/opencode/src/altimate -g '!**/dist/**' -g '!**/build/**'Repository: AltimateAI/altimate-code
Length of output: 27057
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- file ---\n'
nl -ba packages/opencode/src/altimate/tools/dbt-project-health.ts | sed -n '1,220p'
printf '\n--- search definitions ---\n'
rg -n "type HealthFinding|interface HealthFinding|Schema.*HealthFinding|findings:" packages/opencode/src/altimate -g '!**/dist/**' -g '!**/build/**'Repository: AltimateAI/altimate-code
Length of output: 253
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- altimate-core dbt_project_health references ---'
rg -n "dbt_project_health|HealthFinding|JSON.parse\\(json\\)" packages/opencode/src/altimate/native/altimate-core.ts packages/opencode/src/altimate/native -g '!**/dist/**' -g '!**/build/**'
printf '\n%s\n' '--- dispatcher definition ---'
rg -n "class Dispatcher|export const Dispatcher|call\\(" packages/opencode/src/altimate/native packages/opencode/src/altimate -g '!**/dist/**' -g '!**/build/**'Repository: AltimateAI/altimate-code
Length of output: 14037
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- native altimate_core.dbt_project_health implementation ---'
nl -ba packages/opencode/src/altimate/native/altimate-core.ts | sed -n '260,285p'
printf '\n%s\n' '--- dispatcher return typing ---'
nl -ba packages/opencode/src/altimate/native/types.ts | sed -n '1238,1260p'
printf '\n%s\n' '--- dispatcher implementation ---'
rg -n "class Dispatcher|export class Dispatcher|static async call|async call" packages/opencode/src/altimate/native packages/opencode/src/altimate -g '!**/dist/**' -g '!**/build/**'Repository: AltimateAI/altimate-code
Length of output: 265
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- native altimate_core.dbt_project_health implementation ---'
nl -ba packages/opencode/src/altimate/native/altimate-core.ts | sed -n '260,285p'
printf '%s\n' '--- dispatcher return typing ---'
nl -ba packages/opencode/src/altimate/native/types.ts | sed -n '1238,1260p'
printf '%s\n' '--- dispatcher implementation ---'
rg -n "class Dispatcher|export class Dispatcher|static async call|async call" packages/opencode/src/altimate/native packages/opencode/src/altimate -g '!**/dist/**' -g '!**/build/**'Repository: AltimateAI/altimate-code
Length of output: 265
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- native altimate_core.dbt_project_health implementation ---'
sed -n '268,278p' packages/opencode/src/altimate/native/altimate-core.ts | cat -n
printf '\n%s\n' '--- native types entry ---'
sed -n '1242,1254p' packages/opencode/src/altimate/native/types.ts | cat -n
printf '\n%s\n' '--- dispatcher implementation files ---'
fd -a "dispatcher*.ts" packages/opencode/src/altimate packages/opencode/src -E dist -E buildRepository: AltimateAI/altimate-code
Length of output: 1545
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- dispatcher.ts ---'
sed -n '1,260p' packages/opencode/src/altimate/native/dispatcher.ts | cat -n
printf '\n%s\n' '--- AltimateCoreResult definition ---'
rg -n "type AltimateCoreResult|interface AltimateCoreResult|export type AltimateCoreResult" packages/opencode/src/altimate/native/types.ts packages/opencode/src/altimate/native -g '!**/dist/**' -g '!**/build/**'Repository: AltimateAI/altimate-code
Length of output: 3918
Validate the native findings payload at runtime. result.data.findings is only typed here; a malformed or missing bridge payload can throw on .slice()/sort() or be treated as zero findings. Parse it with a runtime schema and return an error on invalid data.
🤖 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 `@packages/opencode/src/altimate/tools/dbt-project-health.ts` around lines 57 -
63, Update the findings handling in the dbt project health flow to validate
result.data.findings with a runtime schema before copying or sorting it. Reject
malformed or missing payloads by returning the existing error response, while
preserving the current severity and code ordering for valid HealthFinding
arrays.
Source: Coding guidelines
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 6 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/opencode/src/altimate/tools/dbt-project-health.ts">
<violation number="1" location="packages/opencode/src/altimate/tools/dbt-project-health.ts:39">
P2: A misspelled or unavailable `config_path` silently runs default health checks and can report a clean result despite requested disabled checks or severity overrides not being applied. Return an explicit tool error when the config cannot be read instead of omitting it.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| let config_json: string | undefined | ||
| if (args.config_path) { | ||
| const file = Bun.file(args.config_path) | ||
| if (await file.exists()) config_json = await file.text() |
There was a problem hiding this comment.
P2: A misspelled or unavailable config_path silently runs default health checks and can report a clean result despite requested disabled checks or severity overrides not being applied. Return an explicit tool error when the config cannot be read instead of omitting it.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/opencode/src/altimate/tools/dbt-project-health.ts, line 39:
<comment>A misspelled or unavailable `config_path` silently runs default health checks and can report a clean result despite requested disabled checks or severity overrides not being applied. Return an explicit tool error when the config cannot be read instead of omitting it.</comment>
<file context>
@@ -0,0 +1,88 @@
+ let config_json: string | undefined
+ if (args.config_path) {
+ const file = Bun.file(args.config_path)
+ if (await file.exists()) config_json = await file.text()
+ }
+
</file context>
…project config)
Adds a deterministic config-inference flow for the dbt health checks:
- `dbt_health_config` tool: calls the new core `dbtHealthInferConfig` (via the dispatcher)
and writes the inferred config to <project_dir>/.altimate/dbt-health.yml (per-project, so
multiple dbt projects each get their own file). Requires @altimateai/altimate-core 0.8.0.
- `dbt_project_health` now auto-discovers <project_dir>/.altimate/dbt-health.{yml,yaml,json}
when no explicit config_path is passed, so the inferred config is used automatically.
- `/dbt-health-config` slash command (.opencode/command): infers + writes the config for
each dbt project in the worktree, then runs the health checks to show the effect.
- Bridge type + dispatcher registration for altimate_core.dbt_health_infer_config.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@packages/opencode/src/altimate/tools/dbt-health-config.ts`:
- Around line 39-40: Update the output-writing flow around mkdir and Bun.write
to prevent symlink escapes outside project_dir. Use the existing symlink-aware
containsReal check on the final output path and reject any symlinked directory
or file components before writing, or use an equivalent atomic no-follow file
creation approach; preserve normal writes for safe paths.
- Line 35: Update the output-path construction around outPath to use node:path’s
join with args.project_dir and DBT_HEALTH_CONFIG_RELPATH. Remove the manual
trailing-slash trimming and string concatenation so path separators are
normalized consistently across platforms.
🪄 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: 25a9ad29-0e52-48e6-ab98-5954341588fe
📒 Files selected for processing (7)
.opencode/command/dbt-health-config.mdpackages/opencode/src/altimate/index.tspackages/opencode/src/altimate/native/altimate-core.tspackages/opencode/src/altimate/native/types.tspackages/opencode/src/altimate/tools/dbt-health-config.tspackages/opencode/src/altimate/tools/dbt-project-health.tspackages/opencode/src/tool/registry.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/opencode/src/altimate/index.ts
- packages/opencode/src/tool/registry.ts
- packages/opencode/src/altimate/native/altimate-core.ts
- packages/opencode/src/altimate/tools/dbt-project-health.ts
| } | ||
|
|
||
| const yaml = String((result.data as { config?: unknown }).config ?? "") | ||
| const outPath = `${args.project_dir.replace(/\/+$/, "")}/${DBT_HEALTH_CONFIG_RELPATH}` |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- file outline ---'
ast-grep outline packages/opencode/src/altimate/tools/dbt-health-config.ts --view expanded || true
echo '--- relevant lines ---'
sed -n '1,120p' packages/opencode/src/altimate/tools/dbt-health-config.ts
echo '--- path semantics probe ---'
node - <<'JS'
const path = require('node:path');
const rel = 'dbt_health_config.yml';
const inputs = [
'/',
'/tmp/',
'C:/',
'C:/work/',
'C:\\',
'C:\\work\\',
'relative/',
];
for (const p of inputs) {
const stripped = `${p.replace(/\/+$/, '')}/${rel}`;
const joined = path.join(p, rel);
const posixJoined = path.posix.join(p, rel);
const winJoined = path.win32.join(p, rel);
console.log(JSON.stringify({ p, stripped, joined, posixJoined, winJoined }));
}
JSRepository: AltimateAI/altimate-code
Length of output: 4011
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,120p' packages/opencode/src/altimate/tools/dbt-health-config.ts
node - <<'JS'
const path = require('node:path');
const rel = 'dbt_health_config.yml';
for (const p of ['/', '/tmp/', 'C:/', 'C:/work/', 'C:\\', 'C:\\work\\', 'relative/']) {
const stripped = `${p.replace(/\/+$/, '')}/${rel}`;
const joined = path.join(p, rel);
console.log(JSON.stringify({ p, stripped, joined }));
}
JSRepository: AltimateAI/altimate-code
Length of output: 3259
🏁 Script executed:
#!/bin/bash
set -euo pipefail
node - <<'JS'
const path = require('node:path');
const samples = [
'/.altimate/dbt-health.yml',
'C:/.altimate/dbt-health.yml',
'C:\\/.altimate/dbt-health.yml',
'C:/work/.altimate/dbt-health.yml',
'C:\\work\\/.altimate/dbt-health.yml',
'//server/share/.altimate/dbt-health.yml',
];
for (const s of samples) {
console.log(JSON.stringify({
s,
posix_dirname: path.posix.dirname(s),
win_dirname: path.win32.dirname(s),
posix_isAbs: path.posix.isAbsolute(s),
win_isAbs: path.win32.isAbsolute(s),
normalized_win: path.win32.normalize(s),
}));
}
JSRepository: AltimateAI/altimate-code
Length of output: 1352
Build the output path with node:path. Manual trimming plus string concatenation can leave mixed separators on Windows paths; join(args.project_dir, DBT_HEALTH_CONFIG_RELPATH) normalizes it consistently.
🤖 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 `@packages/opencode/src/altimate/tools/dbt-health-config.ts` at line 35, Update
the output-path construction around outPath to use node:path’s join with
args.project_dir and DBT_HEALTH_CONFIG_RELPATH. Remove the manual trailing-slash
trimming and string concatenation so path separators are normalized consistently
across platforms.
| await mkdir(dirname(outPath), { recursive: true }) | ||
| await Bun.write(outPath, yaml) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== File outline ==\n'
ast-grep outline packages/opencode/src/altimate/tools/dbt-health-config.ts --view expanded || true
printf '\n== File contents (relevant slice) ==\n'
wc -l packages/opencode/src/altimate/tools/dbt-health-config.ts
sed -n '1,220p' packages/opencode/src/altimate/tools/dbt-health-config.ts | cat -n
printf '\n== Search for related path / symlink handling ==\n'
rg -n "symlink|realpath|canonical|containment|mkdir\\(|Bun\\.write\\(|dbt-health.yml|\\.altimate|project_dir|projectDir|outPath" packages/opencode/src -SRepository: AltimateAI/altimate-code
Length of output: 41320
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== util/filesystem.ts excerpt ==\n'
sed -n '200,280p' packages/opencode/src/util/filesystem.ts | cat -n
printf '\n== write helpers / containment usages ==\n'
rg -n "containsReal|containsPath|mkdir\\(|writeFile\\(|Bun\\.write\\(" packages/opencode/src/util packages/opencode/src/altimate packages/opencode/src/cli -SRepository: AltimateAI/altimate-code
Length of output: 6872
Prevent symlink escapes in the write path. mkdir(..., { recursive: true }) plus Bun.write(outPath, yaml) can still follow a pre-existing .altimate directory or dbt-health.yml symlink and write outside project_dir. Reuse the symlink-aware containment check (containsReal) on the final path and reject symlinked output components, or create the file atomically with no-follow semantics.
🤖 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 `@packages/opencode/src/altimate/tools/dbt-health-config.ts` around lines 39 -
40, Update the output-writing flow around mkdir and Bun.write to prevent symlink
escapes outside project_dir. Use the existing symlink-aware containsReal check
on the final output path and reject any symlinked directory or file components
before writing, or use an equivalent atomic no-follow file creation approach;
preserve normal writes for safe paths.
Source: Coding guidelines
There was a problem hiding this comment.
2 issues found across 7 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/opencode/src/altimate/tools/dbt-health-config.ts">
<violation number="1" location="packages/opencode/src/altimate/tools/dbt-health-config.ts:35">
P3: The output path is built with manual slash-trimming and template concatenation, but `dirname` from `node:path` is already imported. Using `join(args.project_dir, DBT_HEALTH_CONFIG_RELPATH)` would normalize separators consistently (particularly relevant on Windows) and is more idiomatic given the existing import.</violation>
<violation number="2" location="packages/opencode/src/altimate/tools/dbt-health-config.ts:39">
P2: The `mkdir` + `Bun.write` sequence will follow pre-existing symlinks. If `.altimate/` or `dbt-health.yml` is a symlink pointing outside `project_dir`, this writes attacker-controlled content to an arbitrary path. Consider resolving `outPath` after directory creation and verifying it still resides within `project_dir` (e.g., via a realpath containment check) before writing.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
|
||
| let written = false | ||
| if (args.write !== false) { | ||
| await mkdir(dirname(outPath), { recursive: true }) |
There was a problem hiding this comment.
P2: The mkdir + Bun.write sequence will follow pre-existing symlinks. If .altimate/ or dbt-health.yml is a symlink pointing outside project_dir, this writes attacker-controlled content to an arbitrary path. Consider resolving outPath after directory creation and verifying it still resides within project_dir (e.g., via a realpath containment check) before writing.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/opencode/src/altimate/tools/dbt-health-config.ts, line 39:
<comment>The `mkdir` + `Bun.write` sequence will follow pre-existing symlinks. If `.altimate/` or `dbt-health.yml` is a symlink pointing outside `project_dir`, this writes attacker-controlled content to an arbitrary path. Consider resolving `outPath` after directory creation and verifying it still resides within `project_dir` (e.g., via a realpath containment check) before writing.</comment>
<file context>
@@ -0,0 +1,55 @@
+
+ let written = false
+ if (args.write !== false) {
+ await mkdir(dirname(outPath), { recursive: true })
+ await Bun.write(outPath, yaml)
+ written = true
</file context>
| } | ||
|
|
||
| const yaml = String((result.data as { config?: unknown }).config ?? "") | ||
| const outPath = `${args.project_dir.replace(/\/+$/, "")}/${DBT_HEALTH_CONFIG_RELPATH}` |
There was a problem hiding this comment.
P3: The output path is built with manual slash-trimming and template concatenation, but dirname from node:path is already imported. Using join(args.project_dir, DBT_HEALTH_CONFIG_RELPATH) would normalize separators consistently (particularly relevant on Windows) and is more idiomatic given the existing import.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/opencode/src/altimate/tools/dbt-health-config.ts, line 35:
<comment>The output path is built with manual slash-trimming and template concatenation, but `dirname` from `node:path` is already imported. Using `join(args.project_dir, DBT_HEALTH_CONFIG_RELPATH)` would normalize separators consistently (particularly relevant on Windows) and is more idiomatic given the existing import.</comment>
<file context>
@@ -0,0 +1,55 @@
+ }
+
+ const yaml = String((result.data as { config?: unknown }).config ?? "")
+ const outPath = `${args.project_dir.replace(/\/+$/, "")}/${DBT_HEALTH_CONFIG_RELPATH}`
+
+ let written = false
</file context>
dev-punia-altimate
left a comment
dev-punia-altimate
left a comment
There was a problem hiding this comment.
🤖 Code Review — OpenCodeReview (Gemini) — 5 finding(s)
- 5 anchored to a line (posted inline when the comment stream is on)
- 0 without a line anchor
All findings (full text)
1. packages/opencode/src/altimate/tools/dbt-project-health.ts (L38-L52)
[🟠 MEDIUM] If args.config_path is explicitly provided but the file does not exist, this logic will silently proceed with config_json = undefined. It is recommended to throw an error or return an error response when an explicitly provided configuration file is not found, to avoid running health checks with unintended settings.
Suggested change:
let config_json: string | undefined
if (args.config_path) {
const file = Bun.file(args.config_path)
if (!(await file.exists())) {
return {
title: "dbt health: ERROR",
metadata: { error: `Config file not found: ${args.config_path}` },
output: `Failed to find config file at ${args.config_path}`,
}
}
config_json = await file.text()
} else {
const candidates = [
`${args.project_dir.replace(/\/+$/, "")}/.altimate/dbt-health.yml`,
`${args.project_dir.replace(/\/+$/, "")}/.altimate/dbt-health.yaml`,
`${args.project_dir.replace(/\/+$/, "")}/.altimate/dbt-health.json`,
]
for (const candidate of candidates) {
const file = Bun.file(candidate)
if (await file.exists()) {
config_json = await file.text()
break
}
}
}
2. packages/opencode/src/altimate/tools/dbt-project-health.ts (L93-L95)
[🔵 LOW] According to the review checklist, nested ternary expressions are not allowed. Please refactor this to use an if...else statement or logical operators to improve readability.
Suggested change:
for (const f of findings) {
let where = ""
if (f.file) {
where = ` (${f.file})`
} else if (f.unique_id) {
where = ` (${f.unique_id})`
}
lines.push(`[${f.severity.toUpperCase()}] ${f.code} ${f.alias}${where}`)
3. packages/opencode/src/altimate/tools/dbt-health-config.ts (L34)
[🔴 HIGH] Potential runtime TypeError. Accessing .config directly on the typecasted result.data will throw a TypeError: Cannot read properties of undefined if result.data is null or undefined (even when result.success is true). Please use optional chaining (?.config) to ensure safe access.
Suggested change:
const yaml = String((result.data as { config?: unknown })?.config ?? "")
4. packages/opencode/src/altimate/tools/dbt-health-config.ts (L35)
[🔴 HIGH] Unsafe path construction and potential directory traversal. The output path is constructed using simple string concatenation. If the AI agent provides an empty string, /, or ../ for project_dir, it could attempt to write to unexpected system locations outside the intended workspace.
Consider importing join from node:path, using join(args.project_dir, DBT_HEALTH_CONFIG_RELPATH), and adding a check to ensure the resolved path remains within the allowed workspace context.
Suggested change:
// Ensure `join` is imported from `node:path`
const outPath = join(args.project_dir, DBT_HEALTH_CONFIG_RELPATH)
// Recommended: add validation to ensure `outPath` is within the workspace root
5. packages/opencode/src/altimate/tools/dbt-health-config.ts (L37-L42)
[🔴 HIGH] Unhandled exceptions in file system operations and inconsistent FS API usage.
- Unhandled Exceptions:
mkdirand file write operations are not enclosed in atry...catchblock. If the tool encounters permission issues or invalid paths, these async calls will throw exceptions that could crash the execution. They should be caught to gracefully return an error payload (similar to!result.success). - Inconsistent API: The code imports
mkdirfrom standardnode:fs/promisesbut uses the runtime-specificBun.write. For consistency within the file and to avoid potentialReferenceErrors outside of Bun, it's recommended to usewriteFilefromnode:fs/promises.
Suggested change:
let written = false
if (args.write !== false) {
try {
await mkdir(dirname(outPath), { recursive: true })
// Ensure `writeFile` is imported from `node:fs/promises` alongside `mkdir`
await writeFile(outPath, yaml)
written = true
} catch (err) {
return {
title: "dbt health config: ERROR",
metadata: { error: String(err) },
output: `Failed to write config file to ${outPath}: ${err}`,
}
}
}
| let config_json: string | undefined | ||
| const candidates = args.config_path | ||
| ? [args.config_path] | ||
| : [ | ||
| `${args.project_dir.replace(/\/+$/, "")}/.altimate/dbt-health.yml`, | ||
| `${args.project_dir.replace(/\/+$/, "")}/.altimate/dbt-health.yaml`, | ||
| `${args.project_dir.replace(/\/+$/, "")}/.altimate/dbt-health.json`, | ||
| ] | ||
| for (const candidate of candidates) { | ||
| const file = Bun.file(candidate) | ||
| if (await file.exists()) { | ||
| config_json = await file.text() | ||
| break | ||
| } | ||
| } |
There was a problem hiding this comment.
[🟠 MEDIUM] If args.config_path is explicitly provided but the file does not exist, this logic will silently proceed with config_json = undefined. It is recommended to throw an error or return an error response when an explicitly provided configuration file is not found, to avoid running health checks with unintended settings.
Suggested change:
| let config_json: string | undefined | |
| const candidates = args.config_path | |
| ? [args.config_path] | |
| : [ | |
| `${args.project_dir.replace(/\/+$/, "")}/.altimate/dbt-health.yml`, | |
| `${args.project_dir.replace(/\/+$/, "")}/.altimate/dbt-health.yaml`, | |
| `${args.project_dir.replace(/\/+$/, "")}/.altimate/dbt-health.json`, | |
| ] | |
| for (const candidate of candidates) { | |
| const file = Bun.file(candidate) | |
| if (await file.exists()) { | |
| config_json = await file.text() | |
| break | |
| } | |
| } | |
| let config_json: string | undefined | |
| if (args.config_path) { | |
| const file = Bun.file(args.config_path) | |
| if (!(await file.exists())) { | |
| return { | |
| title: "dbt health: ERROR", | |
| metadata: { error: `Config file not found: ${args.config_path}` }, | |
| output: `Failed to find config file at ${args.config_path}`, | |
| } | |
| } | |
| config_json = await file.text() | |
| } else { | |
| const candidates = [ | |
| `${args.project_dir.replace(/\/+$/, "")}/.altimate/dbt-health.yml`, | |
| `${args.project_dir.replace(/\/+$/, "")}/.altimate/dbt-health.yaml`, | |
| `${args.project_dir.replace(/\/+$/, "")}/.altimate/dbt-health.json`, | |
| ] | |
| for (const candidate of candidates) { | |
| const file = Bun.file(candidate) | |
| if (await file.exists()) { | |
| config_json = await file.text() | |
| break | |
| } | |
| } | |
| } |
| for (const f of findings) { | ||
| const where = f.file ? ` (${f.file})` : f.unique_id ? ` (${f.unique_id})` : "" | ||
| lines.push(`[${f.severity.toUpperCase()}] ${f.code} ${f.alias}${where}`) |
There was a problem hiding this comment.
[🔵 LOW] According to the review checklist, nested ternary expressions are not allowed. Please refactor this to use an if...else statement or logical operators to improve readability.
Suggested change:
| for (const f of findings) { | |
| const where = f.file ? ` (${f.file})` : f.unique_id ? ` (${f.unique_id})` : "" | |
| lines.push(`[${f.severity.toUpperCase()}] ${f.code} ${f.alias}${where}`) | |
| for (const f of findings) { | |
| let where = "" | |
| if (f.file) { | |
| where = ` (${f.file})` | |
| } else if (f.unique_id) { | |
| where = ` (${f.unique_id})` | |
| } | |
| lines.push(`[${f.severity.toUpperCase()}] ${f.code} ${f.alias}${where}`) |
| } | ||
| } | ||
|
|
||
| const yaml = String((result.data as { config?: unknown }).config ?? "") |
There was a problem hiding this comment.
[🔴 HIGH] Potential runtime TypeError. Accessing .config directly on the typecasted result.data will throw a TypeError: Cannot read properties of undefined if result.data is null or undefined (even when result.success is true). Please use optional chaining (?.config) to ensure safe access.
Suggested change:
| const yaml = String((result.data as { config?: unknown }).config ?? "") | |
| const yaml = String((result.data as { config?: unknown })?.config ?? "") |
| } | ||
|
|
||
| const yaml = String((result.data as { config?: unknown }).config ?? "") | ||
| const outPath = `${args.project_dir.replace(/\/+$/, "")}/${DBT_HEALTH_CONFIG_RELPATH}` |
There was a problem hiding this comment.
[🔴 HIGH] Unsafe path construction and potential directory traversal. The output path is constructed using simple string concatenation. If the AI agent provides an empty string, /, or ../ for project_dir, it could attempt to write to unexpected system locations outside the intended workspace.
Consider importing join from node:path, using join(args.project_dir, DBT_HEALTH_CONFIG_RELPATH), and adding a check to ensure the resolved path remains within the allowed workspace context.
Suggested change:
| const outPath = `${args.project_dir.replace(/\/+$/, "")}/${DBT_HEALTH_CONFIG_RELPATH}` | |
| // Ensure `join` is imported from `node:path` | |
| const outPath = join(args.project_dir, DBT_HEALTH_CONFIG_RELPATH) | |
| // Recommended: add validation to ensure `outPath` is within the workspace root |
| let written = false | ||
| if (args.write !== false) { | ||
| await mkdir(dirname(outPath), { recursive: true }) | ||
| await Bun.write(outPath, yaml) | ||
| written = true | ||
| } |
There was a problem hiding this comment.
[🔴 HIGH] Unhandled exceptions in file system operations and inconsistent FS API usage.
- Unhandled Exceptions:
mkdirand file write operations are not enclosed in atry...catchblock. If the tool encounters permission issues or invalid paths, these async calls will throw exceptions that could crash the execution. They should be caught to gracefully return an error payload (similar to!result.success). - Inconsistent API: The code imports
mkdirfrom standardnode:fs/promisesbut uses the runtime-specificBun.write. For consistency within the file and to avoid potentialReferenceErrors outside of Bun, it's recommended to usewriteFilefromnode:fs/promises.
Suggested change:
| let written = false | |
| if (args.write !== false) { | |
| await mkdir(dirname(outPath), { recursive: true }) | |
| await Bun.write(outPath, yaml) | |
| written = true | |
| } | |
| let written = false | |
| if (args.write !== false) { | |
| try { | |
| await mkdir(dirname(outPath), { recursive: true }) | |
| // Ensure `writeFile` is imported from `node:fs/promises` alongside `mkdir` | |
| await writeFile(outPath, yaml) | |
| written = true | |
| } catch (err) { | |
| return { | |
| title: "dbt health config: ERROR", | |
| metadata: { error: String(err) }, | |
| output: `Failed to write config file to ${outPath}: ${err}`, | |
| } | |
| } | |
| } |
🤖 Code Review — OpenCodeReview (Gemini) — No Issues FoundNo supported files changed. |
2 participants
Summary
Wires the new Rust dbt health checks (
altimate_core::dbt::health, added in AltimateAI/altimate-core-internal#762) into altimate-code, so dbt governance / modelling / documentation / test checks run directly against a dbt project's files — no compiledmanifest.jsonrequired — through the@altimateai/altimate-corenapi binding.Changes
altimate_core.dbt_project_healthbridge-method type and aDispatcher.registerhandler callingcore.dbtProjectHealth(projectDir, configJson?, catalogPath?).DbtProjectHealthTool(dbt_project_health) — takes aproject_dir(+ optionalconfig_path,catalog_path), runs the checks, and formats findings grouped by severity. Registered in the tool registry and the altimate barrel export.@altimateai/altimate-core0.5.1 → 0.8.0 (the version that exportsdbtProjectHealth).Dependency
Requires
@altimateai/altimate-core0.8.0, published from AltimateAI/altimate-core-internal#762 (stacked on #761). Merge/release that first.Verification
TypeScript typecheck is clean on all changed files against the 0.8.0 core types (verified by overlaying the freshly-built core
index.d.ts). End-to-end, the underlying core returns findings for a sample project (25 findings across 13 check codes on the fixture).🤖 Generated with Claude Code
Summary by cubic
Adds dbt project health checks that run directly on project files and a deterministic config inference flow, powered by
@altimateai/altimate-core0.8.0. You can now generate a per-project.altimate/dbt-health.ymland have the health tool pick it up automatically.New Features
altimate_core.dbt_project_healthandaltimate_core.dbt_health_infer_config.dbt_project_healthtool: runs checks on files; acceptsproject_dirwith optionalconfig_path/catalog_path; auto-discovers.altimate/dbt-health.{yml,yaml,json}; outputs findings with severity counts.dbt_health_configtool: infers and writes.altimate/dbt-health.ymlper project; adds/dbt-health-configcommand to infer then run health; tools registered and exported.Dependencies
@altimateai/altimate-corefrom0.5.1to0.8.0.@altimateai/altimate-core0.8.0to be published first.Written for commit fefe41f. Summary will update on new commits.
Summary by CodeRabbit
.altimate/dbt-health.yml, returning the written path(s) and resulting severity counts.