Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions .opencode/command/dbt-health-config.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
---
description: infer a dbt health-check config from the project's own conventions and write .altimate/dbt-health.yml
agent: build
subtask: true
---

Generate a dbt health-check configuration by inferring it from a dbt project's **existing conventions** (deterministic — no guessing).

## Target project

Use `$ARGUMENTS` as the dbt project directory if provided. Otherwise, locate the dbt project in the current worktree by finding the directory that contains a `dbt_project.yml` (run `project_scan` if you need to discover it). If there are **multiple** dbt projects, do this for **each** one — every project gets its own `.altimate/dbt-health.yml`.

## Steps

1. For each target project directory, call the **`dbt_health_config`** tool:
`dbt_health_config({ project_dir: "<dir>" })`
This deterministically infers the config from the project (modal tags / meta keys / tests, configured schemas & databases, naming contracts, distribution-ratcheted thresholds) and writes it to `<dir>/.altimate/dbt-health.yml`.

2. Then run **`dbt_project_health`** on the same directory:
`dbt_project_health({ project_dir: "<dir>" })`
It auto-discovers the freshly written `.altimate/dbt-health.yml`, so the report now reflects the inferred rules.

3. Summarize for the user: the path(s) written, which config-driven checks were activated (tags, meta keys, tests, naming contracts, parent schema/database whitelists, thresholds), and the resulting finding counts by severity. Tell them the file is committed with the project and can be hand-edited — re-running this command regenerates it deterministically.

Do not fabricate values; everything comes from the two tools above.
2 changes: 1 addition & 1 deletion packages/opencode/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@
"@ai-sdk/togetherai": "2.0.41",
"@ai-sdk/vercel": "2.0.39",
"@ai-sdk/xai": "3.0.82",
"@altimateai/altimate-core": "0.5.1",
"@altimateai/altimate-core": "0.8.0",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

CRITICAL — 0.8.0 is not published, and bun.lock still pins 0.5.1

npm view @altimateai/altimate-core versions
→ … "0.5.1", "0.6.0", "0.7.0"     dist-tags.latest = 0.7.0

0.8.0 does not exist on npm today. Separately, bun.lock still resolves 0.5.1 at lines 273 and 642 plus all five platform binaries, and the lockfile isn't part of this PR (git log -1 -- bun.lock points at a pre-existing commit).

Every CI job runs plain bun install (.github/workflows/ci.yml:100,108,287,348,408,465,505), and Bun applies frozen-lockfile behaviour when CI is set — so the manifest/lock mismatch fails the install step. If it did install from the current lock it would fetch 0.5.1, which exports neither dbtProjectHealth nor dbtHealthInferConfig, and both tools would return TypeError: core.dbtProjectHealth is not a function on every call.

The PR description already flags the ordering dependency, so this is partly known — noting it because the branch is red until that release lands.

Fix: publish 0.8.0, then bun install and commit the regenerated bun.lock with the root entry and all five platform optional deps at 0.8.0.

One wrinkle: bunfig.toml sets minimumReleaseAge = 259200 (3 days) and this package is not in minimumReleaseAgeExcludes, so a freshly published 0.8.0 will still be refused for three days after publish. Add it to the excludes list or plan the timing.

"@altimateai/drivers": "workspace:*",
"@aws-sdk/credential-providers": "3.1057.0",
"@clack/prompts": "1.0.0-alpha.1",
Expand Down
2 changes: 2 additions & 0 deletions packages/opencode/src/altimate/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ export * from "./tools/altimate-core-validate"
export * from "./tools/dbt-lineage"
export * from "./tools/dbt-manifest"
export * from "./tools/dbt-profiles"
export * from "./tools/dbt-project-health"
export * from "./tools/dbt-health-config"

export * from "./tools/finops-analyze-credits"
export * from "./tools/finops-expensive-queries"
Expand Down
17 changes: 17 additions & 0 deletions packages/opencode/src/altimate/native/altimate-core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,23 @@ export function registerAll(): void {
return fail(e)
}
})
// dbt project health checks — reads the dbt project files directly (no manifest required).
register("altimate_core.dbt_project_health", async (params) => {
try {
const json = core.dbtProjectHealth(params.project_dir, params.config_json, params.catalog_path)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

MINOR — no version guard on the new exports

This file already establishes the guard pattern for exactly this situation at line 176:

params.base_sql && typeof core.lintDiff === "function"

added because lintDiff postdates 0.5.1. The surrounding try/catch does convert a missing-export TypeError into the normal failure envelope, so this is diagnostics quality rather than a crash — but the operator sees a raw TypeError string instead of an actionable message.

Fix: guard with typeof core.dbtProjectHealth === "function" and fail with "dbt_project_health requires @altimateai/altimate-core >= 0.8.0". Same for dbtHealthInferConfig below.

return ok(true, { findings: JSON.parse(json) })
} catch (e) {
return fail(e)
}
})
// Deterministically infer a dbt health-check config (YAML) from a project's conventions.
register("altimate_core.dbt_health_infer_config", async (params) => {
try {
return ok(true, { config: core.dbtHealthInferConfig(params.project_dir) })
} catch (e) {
return fail(e)
}
})
Comment on lines +270 to +286

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

CRITICAL — these two registrations break altimate-core-native.test.ts

packages/opencode/test/altimate/altimate-core-native.test.ts:176-231 keeps a hand-maintained ALL_METHODS array (43 entries) and asserts an exact count:

const coreCount = registered.filter((m) => m.startsWith("altimate_core.")).length
expect(coreCount).toBe(ALL_METHODS.length)

Registering two more methods without adding them to the array makes it 45 vs 43. This is a deterministic CI failure, not a coverage gap.

Fix: add "altimate_core.dbt_project_health" and "altimate_core.dbt_health_infer_config" to ALL_METHODS.

// altimate_change end

// 7. altimate_core.fix
Expand Down
8 changes: 8 additions & 0 deletions packages/opencode/src/altimate/native/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1244,6 +1244,14 @@ export const BridgeMethods = {
params: { base_sql: string; head_sql: string }
result: AltimateCoreResult
},
"altimate_core.dbt_project_health": {} as {
params: { project_dir: string; config_json?: string; catalog_path?: string }
result: AltimateCoreResult
},
"altimate_core.dbt_health_infer_config": {} as {
params: { project_dir: string }
result: AltimateCoreResult
},
// altimate_change end
// --- altimate-core Phase 1 (P0) ---
"altimate_core.fix": {} as { params: AltimateCoreFixParams; result: AltimateCoreResult },
Expand Down
55 changes: 55 additions & 0 deletions packages/opencode/src/altimate/tools/dbt-health-config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
import { mkdir } from "node:fs/promises"
import { dirname } from "node:path"
import z from "zod"
import { Tool } from "../../tool/tool"
import { Dispatcher } from "../native"

/** Relative path (from the dbt project root) where the inferred config is written. */
export const DBT_HEALTH_CONFIG_RELPATH = ".altimate/dbt-health.yml"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

MINOR — this constant isn't used by the reader

DBT_HEALTH_CONFIG_RELPATH is exported here, but dbt-project-health.ts:42-44 rebuilds .altimate/dbt-health.* independently as three string literals. One rename desynchronises write and discovery, and the failure is silent: the config just stops being found.

Fix: export the directory and basename (or a candidates(projectDir) helper) from one place and import it in the reader.


export const DbtHealthConfigTool = Tool.define("dbt_health_config", {
description:
"Deterministically infer a dbt health-check config from a dbt project's EXISTING conventions (modal tags/meta keys/tests, configured schemas & databases, naming contracts, distribution-ratcheted thresholds) and write it to <project_dir>/.altimate/dbt-health.yml. This activates the config-driven governance checks (which are no-ops until configured). Per-project: each dbt project gets its own config file. Reproducible — no LLM, same project always yields the same file.",
parameters: z.object({
project_dir: z.string().describe("Path to the dbt project root (the directory containing dbt_project.yml)"),
write: z
.boolean()
.optional()
.describe("Write the config to <project_dir>/.altimate/dbt-health.yml (default true). If false, only return the YAML."),
Comment on lines +15 to +18

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

MAJOR — hand-edited config is silently destroyed

write defaults to true and Bun.write at line 40 unconditionally replaces an existing .altimate/dbt-health.yml — no diff, no backup, no overwrite argument, and no mention in the tool description.

.opencode/command/dbt-health-config.md:23 explicitly tells users the file "is committed with the project and can be hand-edited", so the advertised workflow is precisely the one that erases their edits.

Fix: add overwrite: boolean defaulting to false, or open with wx and fail when the file exists. At minimum, report that the file already existed and what changed.

}),
async execute(args, _ctx) {
const result = await Dispatcher.call("altimate_core.dbt_health_infer_config", {
project_dir: args.project_dir,
})

if (!result.success) {
const error = result.error ?? "unknown error"
return {
title: "dbt health config: ERROR",
metadata: { error },
output: `Failed to infer dbt health config: ${error}`,
}
}

const yaml = String((result.data as { config?: unknown }).config ?? "")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[🔴 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:

Suggested change
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}`

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 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 }));
}
JS

Repository: 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 }));
}
JS

Repository: 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),
  }));
}
JS

Repository: 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.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[🔴 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:

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 })

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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>

await Bun.write(outPath, yaml)
Comment on lines +39 to +40

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 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 -S

Repository: 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 -S

Repository: 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

written = true
}
Comment on lines +37 to +42

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[🔴 HIGH] Unhandled exceptions in file system operations and inconsistent FS API usage.

  1. Unhandled Exceptions: mkdir and file write operations are not enclosed in a try...catch block. 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).
  2. Inconsistent API: The code imports mkdir from standard node:fs/promises but uses the runtime-specific Bun.write. For consistency within the file and to avoid potential ReferenceErrors outside of Bun, it's recommended to use writeFile from node:fs/promises.

Suggested change:

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}`,
}
}
}

Comment on lines +35 to +42

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

CRITICAL — this write has none of the repo's write guards

project_dir comes straight from the model. There is no path.resolve against Instance.directory, no containment check, no symlink resolution, and no permission prompt. WriteTool (src/tool/write.ts:64-70) resolves against Instance.directory and then gates every write behind assertExternalDirectoryEffect and assertSensitiveWriteEffect.

This is the first write-capable tool under src/altimate/tools/ — no other file there calls Bun.write/writeFile — so it establishes a path around the guard layer.

Three concrete failure modes:

  • an absolute or ../-containing project_dir creates directories and writes files outside the workspace with no prompt;
  • a .altimate directory or .altimate/dbt-health.yml symlink inside an otherwise legitimate project is followed and its external target truncated;
  • mkdir(…, { recursive: true }) creates the whole tree on the way there.

Fix: resolve project_dir against Instance.directory, build the path with path.join, canonicalize and verify containment, then gate the write. This tool uses the Promise/Zod adapter, so call the Promise-style helpers — assertExternalDirectory(ctx, target, options) / assertExternalDirectoryLegacy(...) / assertSensitiveWrite(ctx, target) (src/tool/external-directory.ts:54,89,113) — not the Effect-only variants. Re-validate symlinked path components immediately before writing.


const checkCount = (yaml.match(/^ {2}\S/gm) ?? []).length

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

MINOR — checkCount is a fragile heuristic

(yaml.match(/^ {2}\S/gm) ?? []).length counts every key indented exactly two spaces at any nesting depth, not tuned checks. It may match the current emitter's output, but any nesting or indentation change on the core side silently turns the title's (N checks tuned) into a wrong number that nobody will notice.

Fix: have the core return the count, parse the YAML, or drop the claim from the title.

return {
title: written
? `dbt health config written (${checkCount} checks tuned) → ${outPath}`
: `dbt health config inferred (${checkCount} checks tuned)`,
metadata: { path: outPath, written, project_dir: args.project_dir },
output: written
? `Wrote inferred config to ${outPath}:\n\n${yaml}`
: `Inferred config (not written):\n\n${yaml}`,
}
},
})
100 changes: 100 additions & 0 deletions packages/opencode/src/altimate/tools/dbt-project-health.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
import z from "zod"
import { Tool } from "../../tool/tool"
import { Dispatcher } from "../native"

/** A single finding returned by `altimate_core.dbt_project_health` (Rust `HealthFinding`). */
interface HealthFinding {
code: string
alias: string
resource_type: "model" | "source" | "exposure" | "macro" | "project"
unique_id?: string
file?: string
severity: "error" | "warning" | "info"
message: string
recommendation?: string
reason_to_flag?: string
metadata?: unknown
Comment on lines +15 to +16

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

MINOR — these two fields are declared but never rendered

formatFindings (lines 90-100) emits code, alias, file/unique_id, message, and recommendationreason_to_flag and metadata never reach the output. reason_to_flag is arguably the single most useful field in a governance report, since it's what tells the reader why a rule fired.

Fix: render reason_to_flag (it's what makes a finding actionable), or drop both from the interface.

}
Comment on lines +5 to +17

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

MINOR — HealthFinding is an unvalidated local guess

This interface is hand-declared here with no link to the Rust HealthFinding struct and no runtime validation — the shape is only asserted via a cast at line 69. Every other bridge method has a typed result interface in native/types.ts. If the core renames or drops a field, TypeScript won't catch it and the formatter silently emits undefined text.

Fix: move the type into native/types.ts as part of a DbtProjectHealthResult, or parse the JSON through a z.object() schema in the handler.


const SEVERITY_ORDER: Record<string, number> = { error: 0, warning: 1, info: 2 }

export const DbtProjectHealthTool = Tool.define("dbt_project_health", {
description:
"Run dbt project health checks (governance, modelling, documentation, tests, sources) directly against a dbt project's files — no compiled manifest.json required. Reads .sql models (via the dbt Jinja AST parser), schema.yml properties and dbt_project.yml. Optionally uses a catalog.json for column-level checks; otherwise columns are inferred from the SQL parser.",
parameters: z.object({
project_dir: z.string().describe("Path to the dbt project root (the directory containing dbt_project.yml)"),
config_path: z
.string()
.optional()
.describe("Optional path to a JSON health-check config (disabled checks, severity overrides, options)"),
Comment on lines +26 to +29

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

MINOR — this description is inaccurate and incomplete

It says "path to a JSON health-check config", but auto-discovery below accepts .yml/.yaml/.json and the sibling dbt_health_config tool writes YAML. As written, the model is steered away from passing the very file it just generated.

The description also never mentions that a project-local config is auto-discovered when this is omitted, so the model can't reason about when to pass it.

Fix: "Optional path to a health-check config (YAML or JSON). When omitted, auto-discovers <project_dir>/.altimate/dbt-health.{yml,yaml,json}."

catalog_path: z
.string()
.optional()
.describe("Optional path to a dbt catalog.json to power column-aware checks"),
}),
async execute(args, _ctx) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

MINOR — execute isn't wrapped in try/catch

Bun.file(candidate).text() throws on an unreadable config, and Dispatcher.call throws No native handler for … (native/dispatcher.ts:48-50) if lazy registration failed. In dbt-health-config.ts, mkdir throws EACCES/ENOTDIR and Bun.write throws if the target is a directory.

The framework's Effect.onError hook does record and surface these, so nothing is lost — but they bypass the structured {title, metadata, output} error shape both tools already implement for the !result.success path. Sibling dbt-manifest.ts:13-28 wraps its whole body.

Also: the soft-error returns omit metadata.success: false, which dbt-profiles.ts:27,50 sets — so telemetry records these as successful calls despite the ERROR title.

Fix: wrap each execute body and return the same error shape with success: false in metadata.

// Explicit config_path wins; otherwise auto-discover a per-project inferred config
// at <project_dir>/.altimate/dbt-health.{yml,yaml,json} (YAML or JSON both parse).
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`,
Comment on lines +42 to +44

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

MINOR — build these with node:path.join

${args.project_dir.replace(/\/+$/, "")}/… hand-rolls path joining in three places here and once more in dbt-health-config.ts:35. It doesn't normalize .. segments and assumes POSIX separators.

Fix: path.join(projectDir, ".altimate", "dbt-health.yml"). dbt-health-config.ts already imports dirname from node:path.

]
for (const candidate of candidates) {
const file = Bun.file(candidate)
if (await file.exists()) {
config_json = await file.text()
break
}
}
Comment on lines +38 to +52

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[🟠 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:

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
}
}
}

Comment on lines +39 to +52

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

MAJOR — an explicit config_path that doesn't exist is silently ignored

When config_path is supplied it becomes the sole candidate, and if file.exists() is false the loop just falls through leaving config_json undefined. The checks then run fully unconfigured and the report gives no hint. The auto-discovery fallback is skipped entirely in that branch too.

For a governance tool this is the worst failure mode: a typo'd path yields ✓ No dbt health issues found. for a project whose policy never loaded. Anyone reading that output concludes the project is clean.

Fix: if config_path was passed explicitly and is missing or unreadable, return an error naming the path. Either way, report the loaded config path (or "none") in metadata so the result is self-describing.


const result = await Dispatcher.call("altimate_core.dbt_project_health", {
project_dir: args.project_dir,
config_json,
catalog_path: args.catalog_path,
})
Comment on lines +54 to +58

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

MAJOR — relative paths resolve against process.cwd(), not the active project

project_dir, config_path, and catalog_path are passed straight to Bun and to Rust. This codebase models the current project as Instance.directory — the pattern used by write.ts:65-67, ls.ts, glob.ts, and bash.ts — which can differ from process.cwd() in server, desktop, attach, and multi-instance execution.

Users will reasonably expect config_path / catalog_path to be project-relative; today they're process-relative, and neither the parameter descriptions nor the returned metadata say which.

Fix:

const projectDir = path.resolve(Instance.directory, args.project_dir)
const configPath = args.config_path ? path.resolve(projectDir, args.config_path) : undefined
const catalogPath = args.catalog_path ? path.resolve(projectDir, args.catalog_path) : undefined

Pass only normalized paths to the dispatcher and report them in metadata.


if (!result.success) {
const error = result.error ?? "unknown error"
return {
title: "dbt health: ERROR",
metadata: { error },
output: `Failed to run dbt health checks: ${error}`,
}
}

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
Comment on lines +69 to +75

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ 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 build

Repository: 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


return {
title: `dbt health: ${findings.length} finding(s) — ${counts.error} error, ${counts.warning} warning, ${counts.info} info`,
metadata: {
finding_count: findings.length,
error_count: counts.error,
warning_count: counts.warning,
info_count: counts.info,
},
output: formatFindings(findings),
}
},
})

function formatFindings(findings: HealthFinding[]): string {
if (findings.length === 0) return "✓ No dbt health issues found."
const lines: string[] = []
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}`)
Comment on lines +93 to +95

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[🔵 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:

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}`)

lines.push(` ${f.message}`)
if (f.recommendation) lines.push(` → ${f.recommendation}`)
}
return lines.join("\n")
}
Comment on lines +90 to +100

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

MINOR — no domain-level cap on output

formatFindings emits 2–3 lines per finding with no limit. This is bounded in practice: the shared Tool.define wrapper applies Truncate.Service at MAX_LINES = 2000 / MAX_BYTES = 50 * 1024 (src/tool/truncate.ts:15-16) and spills the full text to a file, and because findings are sorted error-first the head truncation keeps the important ones while metadata counts stay accurate. That's a good outcome largely by accident of the sort order.

What's missing is that the model is never told it's seeing a partial list.

Fix: cap at a domain-appropriate number and append "… N more findings omitted", so the truncation is explicit rather than inferred.

4 changes: 4 additions & 0 deletions packages/opencode/src/tool/registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ import { WarehouseDiscoverTool } from "../altimate/tools/warehouse-discover"
import { McpDiscoverTool } from "../altimate/tools/mcp-discover"

import { DbtManifestTool } from "../altimate/tools/dbt-manifest"
import { DbtProjectHealthTool } from "../altimate/tools/dbt-project-health"
import { DbtHealthConfigTool } from "../altimate/tools/dbt-health-config"
// altimate_change start - import dbt unit test generation tool
import { DbtUnitTestGenTool } from "../altimate/tools/dbt-unit-test-gen"
// altimate_change end
Expand Down Expand Up @@ -405,6 +407,8 @@ export namespace ToolRegistry {
DbtUnitTestGenTool,
// altimate_change end
DbtProfilesTool,
DbtProjectHealthTool,
DbtHealthConfigTool,
DbtLineageTool,
SchemaIndexTool,
SchemaSearchTool,
Expand Down
Loading