Skip to content

fix(postgres): fail fast when SQL loader directory cannot be resolved - #4482

Merged
roggervalf merged 3 commits into
masterfrom
copilot/fix-review-comments-4473
Aug 5, 2026
Merged

fix(postgres): fail fast when SQL loader directory cannot be resolved#4482
roggervalf merged 3 commits into
masterfrom
copilot/fix-review-comments-4473

Conversation

Copilot AI commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Port Impact Checklist

  • Python – does this change need to be ported or documented in the Python library?
  • Elixir – does this change need to be ported or documented in the Elixir library?
  • PHP – does this change need to be ported or documented in the PHP library?
  • Rust – does this change need to be ported or documented in the Rust library?
  • .NET – does this change need to be ported or documented in the .NET library?

Why

A PR review identified that sql-loader could fall back to '' when directory resolution failed, causing SQL asset lookup to become relative to process.cwd(). This risks confusing ENOENTs or loading unintended SQL files from the working directory.

How

  • Directory resolution hardening

    • Added getDirname() resolution path for CJS/ESM compatibility (__dirname first, then stack/file URL resolution).
    • Replaced silent fallback with an explicit throw when directory cannot be determined.
  • SQL path binding

    • MIGRATIONS_DIR and COMMANDS_DIR now always derive from the resolved module directory (currentDir), never from implicit cwd behavior.
  • Focused loader coverage

    • Added tests/postgres/sql_loader.test.ts covering migration SQL loading, command SQL loading, and command SQL cache reuse.
function getDirname(): string {
  if (typeof __dirname !== 'undefined' && __dirname) return __dirname;
  // ...stack/file URL resolution...
  throw new Error('Could not determine sql-loader directory path');
}

Additional Notes (Optional)

This change is scoped to the review-thread concern only: fail fast on unresolved loader directory to prevent incorrect runtime SQL path resolution.

Copilot AI changed the title [WIP] Fix code according to review comments fix(postgres): fail fast when SQL loader directory cannot be resolved Aug 3, 2026
Copilot AI requested a review from manast August 3, 2026 13:56
@manast
manast marked this pull request as ready for review August 3, 2026 21:16
Copilot AI review requested due to automatic review settings August 3, 2026 21:16

Copilot AI left a comment

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.

Pull request overview

This PR hardens the PostgreSQL SQL asset loader so it never silently falls back to process.cwd() when its module directory can’t be resolved, and instead fails fast with an explicit error. This reduces the risk of confusing ENOENTs or accidentally loading unintended SQL files from the working directory.

Changes:

  • Added a getDirname() resolver to derive the loader’s directory in a CJS/ESM-compatible way and throw when it can’t be determined.
  • Bound MIGRATIONS_DIR / COMMANDS_DIR to the resolved module directory (currentDir) instead of relying on implicit cwd behavior.
  • Added a focused Postgres test file covering migration SQL loading, command SQL loading, and intended cache behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/postgres/sql-loader.ts Computes a stable module directory and derives migrations/commands paths from it, throwing if resolution fails.
tests/postgres/sql_loader.test.ts Adds basic runtime coverage for migration/command SQL loading and attempts to validate caching behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/postgres/sql_loader.test.ts Outdated
@manast
manast force-pushed the copilot/fix-review-comments-4473 branch from 3d1cf49 to 7b1bb71 Compare August 4, 2026 10:22
@manast
manast requested a review from roggervalf August 4, 2026 13:29
@roggervalf
roggervalf force-pushed the copilot/fix-review-comments-4473 branch 3 times, most recently from 222d020 to ff7dbed Compare August 5, 2026 01:51
@roggervalf
roggervalf force-pushed the copilot/fix-review-comments-4473 branch from ff7dbed to 0c0da2c Compare August 5, 2026 03:14
@roggervalf
roggervalf merged commit f3936a2 into master Aug 5, 2026
32 checks passed
@roggervalf
roggervalf deleted the copilot/fix-review-comments-4473 branch August 5, 2026 03:39
@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

🎉 This PR is included in version 6.0.8 🎉

The release is available on:

Your semantic-release bot 📦🚀

@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

🎉 This PR is included in version 3.0.4 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants