Skip to content

Commit 2cc41eb

Browse files
committed
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`
1 parent baa5dfd commit 2cc41eb

3 files changed

Lines changed: 10 additions & 7 deletions

File tree

.github/workflows/release.yml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -161,9 +161,9 @@ jobs:
161161
echo "::error::node_python_bridge.py missing from dbt-tools dist"
162162
exit 1
163163
fi
164-
# Verify no hardcoded CI runner paths remain in the bundle
165-
if grep -q 'home/runner' packages/dbt-tools/dist/index.js; then
166-
echo "::error::dbt-tools bundle contains hardcoded CI runner path"
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"
167167
exit 1
168168
fi
169169
# Verify __dirname was patched to runtime resolution

packages/dbt-tools/script/copy-python.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,10 @@ const indexPath = join(dist, "index.js")
2222
let code = readFileSync(indexPath, "utf8")
2323
const pattern = /var __dirname\s*=\s*"[^"]*python-bridge[^"]*"/
2424
if (pattern.test(code)) {
25-
code = code.replace(pattern, `var __dirname = import.meta.dirname`)
25+
// Fallback for Node < 20.11.0 where import.meta.dirname is unavailable.
26+
// The bundle already imports fileURLToPath from "url", so path + url are in scope.
27+
const replacement = `var __dirname = typeof import.meta.dirname === "string" ? import.meta.dirname : path.dirname(fileURLToPath(import.meta.url))`
28+
code = code.replace(pattern, replacement)
2629
writeFileSync(indexPath, code)
2730
console.log(`Patched __dirname in dist/index.js`)
2831
} else {

packages/dbt-tools/test/build-integrity.test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,14 @@ describe("build integrity", () => {
1515
expect(existsSync(join(dist, "node_python_bridge.py"))).toBe(true)
1616
})
1717

18-
test("no hardcoded CI runner paths in bundle", () => {
18+
test("no hardcoded absolute paths in __dirname", () => {
1919
const code = readFileSync(join(dist, "index.js"), "utf8")
20-
expect(code).not.toContain("home/runner")
20+
// Catch both Unix ("/...") and Windows ("C:\\...") hardcoded paths
21+
expect(code).not.toMatch(/var __dirname\s*=\s*"(?:[A-Za-z]:\\\\|\/)/)
2122
})
2223

2324
test("__dirname is patched to runtime resolution", () => {
2425
const code = readFileSync(join(dist, "index.js"), "utf8")
2526
expect(code).toContain("import.meta.dirname")
26-
expect(code).not.toMatch(/var __dirname\s*=\s*"\//)
2727
})
2828
})

0 commit comments

Comments
 (0)