Skip to content

Commit 1a6610d

Browse files
suryaiyer95claudemdesmetanandgupta42
authored
fix: altimate-dbt commands fail with hardcoded CI path in published binary (#467)
* fix: patch hardcoded `__dirname` in `dbt-tools` bundle so `altimate-dbt` works in published releases `bun build` replaces `__dirname` with a compile-time constant when bundling `python-bridge` (transitive dep of `@altimateai/dbt-integration`). In CI this bakes `/home/runner/work/...` into the bundle, causing `altimate-dbt build` and all Python-bridge commands to fail with ENOENT on every user's machine. Fix: - Copy `node_python_bridge.py` into `dist/` alongside `index.js` - Post-process the bundle to replace the frozen path with `import.meta.dirname` - Fail the build if the patch pattern isn't found (safety net) - Add CI smoke test to prevent regression Broken since PR #201. Closes #466. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: address PR review — add Node 18 fallback and broaden path assertions - Use `fileURLToPath(import.meta.url)` fallback for Node < 20.11.0 - Broaden smoke test and build integrity checks to catch any hardcoded absolute path (Unix + Windows), not just `home/runner` * fix: drop Node 18 fallback — `path` is not in scope before `__dirname` in bundle GLM-5 review correctly identified that `path` is defined AFTER `__dirname` in the bundled output, so the `path.dirname(fileURLToPath(...))` fallback would throw `ReferenceError` on Node < 20.11.0. Since Node 18 is EOL (April 2025), use `import.meta.dirname` unconditionally. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: copy `node_python_bridge.py` in `publish.ts` so npm tarball includes it `copyAssets` copied `dbt-tools/dist/index.js` but not the `.py` file the patched `__dirname` resolves to. Without this, the fix would still break in published packages. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: spread real `child_process` in `mock.module` to prevent `execFileSync` leak `mock.module("child_process")` in `dbt-cli.test.ts` only provided `execFile`, causing `dbt-resolve.test.ts` to fail with `Export named 'execFileSync' not found` when Bun leaks the mock across test files. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: use __require fallback for __dirname to support Node < 20.11.0 `import.meta.dirname` is unavailable before Node 20.11.0. The previous fallback was dropped because `path`/`fileURLToPath` weren't in scope at that point in the bun-generated __commonJS IIFE. Using `__require` (a module-level closure bun always emits) works at any Node version. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * style: format `dbt-cli.test.ts` and `publish.ts` with Prettier Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * test: add adversarial tests for `__dirname` patch and build integrity 22 tests covering regex edge cases, runtime resolution, idempotency, CI smoke test parity, and built bundle invariants. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: address code review — existence check, comment fix, better diagnostics - Add `existsSync` check before copying `node_python_bridge.py` with a clear error message if the file is missing from the dependency - Fix misleading comment that said "no fallback needed" while fallback code existed — now accurately describes the `__require` fallback - Log the nearest `__dirname` match on pattern mismatch to aid debugging Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * test: expand adversarial tests to 43 cases covering runtime, publish, and mutation New test categories: - Script execution: verify exit codes, progress output, build determinism - Bundle runtime structure: `__require` origin, `__commonJS` scope, spawn chain - Publish pipeline: patched artifacts integrity before copy - Mutation testing: verify guards catch removal of key fix components - Regex performance: no catastrophic backtracking on 100KB+ input - Malformed inputs: unicode, spaces, special chars, surrounding code preservation Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: eliminate flaky CI test failures in dispatcher, dbt, and tracer tests Root causes: - `dispatcher.test.ts`: Bun's multi-file runner leaks `_ensureRegistered` hook from other files' `native/index.ts` imports. `reset()` clears handlers but not the hook, so `call()` triggers lazy registration instead of throwing. Fix: clear hook in `beforeEach`. - `dbt-first-execution.test.ts`: `mock.module` for DuckDB leaked across files. Fix: spread real module exports + ensure native/index.ts import. - `tracing-adversarial-final.test.ts`: 50ms snapshot waits too tight for CI under load. Fix: increase to 200ms/300ms. Result: 0 failures in full suite (4961 pass, 340 skip). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: replace environment-coupled `runner` assertion with directory existence check Address CodeRabbit review: `expect(dirname).not.toContain("runner")` would fail on GitHub CI where test runs under `/home/runner/`. Use `existsSync(dirname)` to validate the directory is real instead. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Michiel De Smet <mdesmet@gmail.com> Co-authored-by: anandgupta42 <anand@altimate.ai> Co-authored-by: anandgupta42 <93243293+anandgupta42@users.noreply.github.com>
1 parent 0e6bb70 commit 1a6610d

File tree

10 files changed

+662
-116
lines changed

10 files changed

+662
-116
lines changed

.github/workflows/release.yml

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,25 @@ jobs:
154154
run: bun run build
155155
working-directory: packages/dbt-tools
156156

157+
- name: Smoke test dbt-tools bundle
158+
run: |
159+
# Verify node_python_bridge.py was copied into dist
160+
if [ ! -f packages/dbt-tools/dist/node_python_bridge.py ]; then
161+
echo "::error::node_python_bridge.py missing from dbt-tools dist"
162+
exit 1
163+
fi
164+
# Verify no hardcoded absolute path in __dirname (catches any CI runner OS)
165+
if grep -qE 'var __dirname\s*=\s*"(/|[A-Za-z]:\\)' packages/dbt-tools/dist/index.js; then
166+
echo "::error::dbt-tools bundle contains hardcoded absolute path in __dirname"
167+
exit 1
168+
fi
169+
# Verify __dirname was patched to runtime resolution
170+
if ! grep -q 'import.meta.dirname' packages/dbt-tools/dist/index.js; then
171+
echo "::error::dbt-tools bundle missing import.meta.dirname patch"
172+
exit 1
173+
fi
174+
echo "dbt-tools smoke test passed"
175+
157176
- name: Free disk space for artifact download + npm publish
158177
run: |
159178
sudo rm -rf /usr/share/dotnet /usr/local/lib/android /opt/ghc /usr/local/share/boost

packages/dbt-tools/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
"scripts": {
1010
"build": "bun build src/index.ts --outdir dist --target node --format esm && bun run script/copy-python.ts",
1111
"typecheck": "tsc --noEmit",
12-
"test": "bun test test/cli.test.ts test/config.test.ts test/dbt-cli.test.ts test/dbt-resolve.test.ts --timeout 30000",
12+
"test": "bun test test/cli.test.ts test/config.test.ts test/dbt-cli.test.ts test/dbt-resolve.test.ts test/build-integrity.test.ts --timeout 30000",
1313
"test:e2e": "bun test test/e2e/ --timeout 300000"
1414
},
1515
"dependencies": {
Lines changed: 37 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,42 @@
1-
import { cpSync } from "fs"
1+
import { cpSync, existsSync, readFileSync, writeFileSync } from "fs"
22
import { dirname, join } from "path"
33

4+
const dist = join(import.meta.dir, "..", "dist")
5+
6+
// 1. Copy altimate_python_packages
47
const resolved = require.resolve("@altimateai/dbt-integration")
58
const source = join(dirname(resolved), "altimate_python_packages")
6-
const target = join(import.meta.dir, "..", "dist", "altimate_python_packages")
7-
8-
cpSync(source, target, { recursive: true })
9+
cpSync(source, join(dist, "altimate_python_packages"), { recursive: true })
910
console.log(`Copied altimate_python_packages → dist/`)
11+
12+
// 2. Copy node_python_bridge.py into dist so it lives next to index.js
13+
// node_python_bridge.py is shipped in dbt-integration's dist
14+
const bridgePy = join(dirname(resolved), "node_python_bridge.py")
15+
if (!existsSync(bridgePy)) {
16+
console.error(`ERROR: node_python_bridge.py not found at ${bridgePy}`)
17+
console.error(` Is @altimateai/dbt-integration up to date?`)
18+
process.exit(1)
19+
}
20+
cpSync(bridgePy, join(dist, "node_python_bridge.py"))
21+
console.log(`Copied node_python_bridge.py → dist/`)
22+
23+
// 3. Fix the hardcoded __dirname that bun bakes at compile time.
24+
// Replace it with a runtime resolution so the bridge script is found
25+
// relative to the built index.js, not the CI runner's node_modules.
26+
const indexPath = join(dist, "index.js")
27+
let code = readFileSync(indexPath, "utf8")
28+
const pattern = /var __dirname\s*=\s*"[^"]*python-bridge[^"]*"/
29+
if (pattern.test(code)) {
30+
// import.meta.dirname is supported by Bun and Node >= 20.11.0.
31+
// Fallback via __require handles older runtimes where import.meta.dirname is unavailable.
32+
const replacement = `var __dirname = typeof import.meta.dirname === "string" ? import.meta.dirname : __require("path").dirname(__require("url").fileURLToPath(import.meta.url))`
33+
code = code.replace(pattern, replacement)
34+
writeFileSync(indexPath, code)
35+
console.log(`Patched __dirname in dist/index.js`)
36+
} else {
37+
const found = code.match(/var __dirname[^;]*/)?.[0] ?? "(not found)"
38+
console.error(`ERROR: could not find python-bridge __dirname to patch — the bundle format may have changed`)
39+
console.error(` Pattern: ${pattern}`)
40+
console.error(` Nearest match: ${found}`)
41+
process.exit(1)
42+
}

0 commit comments

Comments
 (0)