fix(drizzle): add top-level url property to PostgresDrizzleConfig#889
fix(drizzle): add top-level url property to PostgresDrizzleConfig#889
Conversation
Add url as a convenience property on PostgresDrizzleConfig so users can pass the connection URL directly instead of nesting it under connection. Priority chain: connection.url > url > connectionString > DATABASE_URL
📝 WalkthroughWalkthroughAdds an optional top-level Changes
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
📦 Canary Packages Publishedversion: PackagesInstallAdd to your {
"dependencies": {
"@agentuity/evals": "https://agentuity-sdk-objects.t3.storageapi.dev/npm/1.0.3-1564153/agentuity-evals-1.0.3-1564153.tgz",
"@agentuity/opencode": "https://agentuity-sdk-objects.t3.storageapi.dev/npm/1.0.3-1564153/agentuity-opencode-1.0.3-1564153.tgz",
"@agentuity/core": "https://agentuity-sdk-objects.t3.storageapi.dev/npm/1.0.3-1564153/agentuity-core-1.0.3-1564153.tgz",
"@agentuity/auth": "https://agentuity-sdk-objects.t3.storageapi.dev/npm/1.0.3-1564153/agentuity-auth-1.0.3-1564153.tgz",
"@agentuity/runtime": "https://agentuity-sdk-objects.t3.storageapi.dev/npm/1.0.3-1564153/agentuity-runtime-1.0.3-1564153.tgz",
"@agentuity/workbench": "https://agentuity-sdk-objects.t3.storageapi.dev/npm/1.0.3-1564153/agentuity-workbench-1.0.3-1564153.tgz",
"@agentuity/react": "https://agentuity-sdk-objects.t3.storageapi.dev/npm/1.0.3-1564153/agentuity-react-1.0.3-1564153.tgz",
"@agentuity/server": "https://agentuity-sdk-objects.t3.storageapi.dev/npm/1.0.3-1564153/agentuity-server-1.0.3-1564153.tgz",
"@agentuity/drizzle": "https://agentuity-sdk-objects.t3.storageapi.dev/npm/1.0.3-1564153/agentuity-drizzle-1.0.3-1564153.tgz",
"@agentuity/postgres": "https://agentuity-sdk-objects.t3.storageapi.dev/npm/1.0.3-1564153/agentuity-postgres-1.0.3-1564153.tgz",
"@agentuity/cli": "https://agentuity-sdk-objects.t3.storageapi.dev/npm/1.0.3-1564153/agentuity-cli-1.0.3-1564153.tgz",
"@agentuity/frontend": "https://agentuity-sdk-objects.t3.storageapi.dev/npm/1.0.3-1564153/agentuity-frontend-1.0.3-1564153.tgz",
"@agentuity/schema": "https://agentuity-sdk-objects.t3.storageapi.dev/npm/1.0.3-1564153/agentuity-schema-1.0.3-1564153.tgz"
}
}Or install directly: bun add https://agentuity-sdk-objects.t3.storageapi.dev/npm/1.0.3-1564153/agentuity-evals-1.0.3-1564153.tgz
bun add https://agentuity-sdk-objects.t3.storageapi.dev/npm/1.0.3-1564153/agentuity-opencode-1.0.3-1564153.tgz
bun add https://agentuity-sdk-objects.t3.storageapi.dev/npm/1.0.3-1564153/agentuity-core-1.0.3-1564153.tgz
bun add https://agentuity-sdk-objects.t3.storageapi.dev/npm/1.0.3-1564153/agentuity-auth-1.0.3-1564153.tgz
bun add https://agentuity-sdk-objects.t3.storageapi.dev/npm/1.0.3-1564153/agentuity-runtime-1.0.3-1564153.tgz
bun add https://agentuity-sdk-objects.t3.storageapi.dev/npm/1.0.3-1564153/agentuity-workbench-1.0.3-1564153.tgz
bun add https://agentuity-sdk-objects.t3.storageapi.dev/npm/1.0.3-1564153/agentuity-react-1.0.3-1564153.tgz
bun add https://agentuity-sdk-objects.t3.storageapi.dev/npm/1.0.3-1564153/agentuity-server-1.0.3-1564153.tgz
bun add https://agentuity-sdk-objects.t3.storageapi.dev/npm/1.0.3-1564153/agentuity-drizzle-1.0.3-1564153.tgz
bun add https://agentuity-sdk-objects.t3.storageapi.dev/npm/1.0.3-1564153/agentuity-postgres-1.0.3-1564153.tgz
bun add https://agentuity-sdk-objects.t3.storageapi.dev/npm/1.0.3-1564153/agentuity-cli-1.0.3-1564153.tgz
bun add https://agentuity-sdk-objects.t3.storageapi.dev/npm/1.0.3-1564153/agentuity-frontend-1.0.3-1564153.tgz
bun add https://agentuity-sdk-objects.t3.storageapi.dev/npm/1.0.3-1564153/agentuity-schema-1.0.3-1564153.tgz |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/drizzle/test/config.test.ts`:
- Around line 232-241: The test "can create instance with no config" relies on
ambient DATABASE_URL and may connect to a real DB; update the test around
createPostgresDrizzle() to make it deterministic by saving
process.env.DATABASE_URL, then explicitly unset or set it to a safe dummy value
before calling createPostgresDrizzle(), run assertions, and finally restore the
original process.env.DATABASE_URL in the finally block (ensuring close() still
runs); reference the createPostgresDrizzle() call and the DATABASE_URL
environment variable when making the change.
🧹 Nitpick comments (3)
packages/drizzle/src/types.ts (1)
13-18: Clean addition of theurlproperty.The new property is well-documented with clear fallback semantics. The JSDoc accurately describes the resolution chain from the
urlfield's perspective.One small observation: the JSDoc on
connection(line 28) says it "takes precedence overconnectionString" but doesn't mention it also takes precedence overurl. Consider updating it to say: "Takes precedence overurlandconnectionStringif both are provided." This would make the full priority chain discoverable from either direction.📝 Suggested doc update
/** * Full PostgreSQL connection configuration. - * Takes precedence over `connectionString` if both are provided. + * Takes precedence over `url` and `connectionString` if both are provided. */ connection?: PostgresConfig;packages/drizzle/test/config.test.ts (2)
112-123:close()is async but never awaited infinallyblocks.Throughout this file,
close()returns aPromise<void>but every call site infinallyblocks is synchronous (e.g., line 121). This means cleanup is fire-and-forget — the test can finish before the connection is released, potentially causing resource leaks or flaky interference between tests.Mark the test callbacks as
asyncandawait close():♻️ Example fix (apply to all similar tests)
- it('can create instance with url', () => { + it('can create instance with url', async () => { const { db, client, close } = createPostgresDrizzle({ url: 'postgres://localhost:5432/nonexistent_db', }); try { expect(db).toBeDefined(); expect(client).toBeDefined(); expect(typeof close).toBe('function'); } finally { - close(); + await close(); } });
244-284: URL priority tests don't actually verify priority at runtime.These tests are labeled "URL priority chain" but only confirm that configs compile and produce a defined
dbinstance. They don't inspect which URL was actually selected by the client. The inline comment (lines 245–247) does acknowledge this, but the test names like "connection.url takes highest precedence" could mislead future readers into thinking the priority logic is regression-tested.Consider either:
- Renaming them to clearly indicate they are structural/compile-time checks (e.g., "accepts config with connection.url, url, and connectionString"), or
- Adding a spy/mock on the
postgres()factory (via@agentuity/test-utilsor a manual spy) to assert the actual URL passed to the client, which would make these true behavioral tests.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/drizzle/src/postgres.tspackages/drizzle/src/types.tspackages/drizzle/test/config.test.ts
🧰 Additional context used
📓 Path-based instructions (6)
packages/drizzle/**/*.{ts,tsx}
📄 CodeRabbit inference engine (packages/drizzle/AGENTS.md)
packages/drizzle/**/*.{ts,tsx}: UsecreatePostgresDrizzle()factory function to create Drizzle ORM instances
Define schemas using Drizzle'spgTableand column types rather than other schema definition methods
Use full TypeScript support with schema inference from Drizzle ORM for type-safe queries
UsedrizzleAdapterfrom better-auth when integrating with@agentuity/auth
Files:
packages/drizzle/src/types.tspackages/drizzle/src/postgres.tspackages/drizzle/test/config.test.ts
**/*.{ts,tsx,js,jsx,json,md}
📄 CodeRabbit inference engine (AGENTS.md)
Use Prettier for code formatting with tabs (width 3), single quotes, and semicolons
Files:
packages/drizzle/src/types.tspackages/drizzle/src/postgres.tspackages/drizzle/test/config.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript in strict mode with ESNext target and bundler moduleResolution
Files:
packages/drizzle/src/types.tspackages/drizzle/src/postgres.tspackages/drizzle/test/config.test.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use
StructuredErrorfrom@agentuity/corefor error handling
Files:
packages/drizzle/src/types.tspackages/drizzle/src/postgres.tspackages/drizzle/test/config.test.ts
packages/drizzle/**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (packages/drizzle/AGENTS.md)
Ensure tests run against a PostgreSQL instance and use
@agentuity/test-utilsfor mocking
Files:
packages/drizzle/test/config.test.ts
packages/*/test/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
packages/*/test/**/*.{ts,tsx}: Place test files intest/folder, never insrc/or__tests__/
Import from../src/in test files
Use@agentuity/test-utilsfor mocks in tests
Files:
packages/drizzle/test/config.test.ts
🧠 Learnings (1)
📚 Learning: 2025-12-21T00:31:41.858Z
Learnt from: jhaynie
Repo: agentuity/sdk PR: 274
File: packages/cli/src/cmd/build/vite/server-bundler.ts:12-41
Timestamp: 2025-12-21T00:31:41.858Z
Learning: In Bun runtime, BuildMessage and ResolveMessage are global types and are not exported from the bun module. Do not import { BuildMessage } from 'bun' or similar; these types are available globally and should be used without import. This applies to all TypeScript files that target the Bun runtime within the repository.
Applied to files:
packages/drizzle/src/types.tspackages/drizzle/src/postgres.tspackages/drizzle/test/config.test.ts
🧬 Code graph analysis (1)
packages/drizzle/test/config.test.ts (2)
packages/drizzle/src/types.ts (1)
PostgresDrizzleConfig(10-58)packages/drizzle/src/postgres.ts (1)
createPostgresDrizzle(49-105)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: Queue SDK Tests
- GitHub Check: Build
- GitHub Check: Package Installation & Usage Test
- GitHub Check: Framework Integration Tests (TanStack & Next.js)
- GitHub Check: Cloud Deployment Tests
- GitHub Check: Sandbox CLI Tests
- GitHub Check: SDK Integration Test Suite
- GitHub Check: Playwright E2E Smoke Test
- GitHub Check: Template Integration Tests
- GitHub Check: Pack & Upload
🔇 Additional comments (2)
packages/drizzle/src/postgres.ts (1)
58-66: URL priority chain is correctly implemented.The cascading fallback logic is clean and correctly enforces
connection.url > url > connectionString > DATABASE_URL. The outer!clientConfig.urlguard on line 58 ensures that a URL from the clonedconnectionobject always wins.packages/drizzle/test/config.test.ts (1)
1-11: Good test structure and organization.The file follows the project convention: it's placed under
test/, imports from../src/, and is well-organized into logical describe blocks (type compat, direct usage, priority, backward compat). The comment header referencing the Kysely test pattern is helpful context. As per coding guidelines,@agentuity/test-utilsshould be used for mocking — that would be relevant if the URL priority tests are upgraded to behavioral tests.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/drizzle/test/config.test.ts`:
- Around line 112-123: The test calls createPostgresDrizzle and invokes the
returned close function in a finally block but close is async (async () => {
await client.close(); }), so the Promise is dropped; update all finally blocks
in this test file to await close() (e.g., change finally { close(); } to finally
{ await close(); }) and make the surrounding test callback async so the await is
valid; ensure the same change is applied to every test in this file that calls
close in a finally.
🧹 Nitpick comments (2)
packages/drizzle/test/config.test.ts (2)
254-293: URL priority tests don't actually verify the priority chain.These tests create instances and assert
dbis defined, but they never inspect which URL the underlyingpostgresclient received. The names ("takes highest precedence", "takes precedence over") imply behavioural verification that isn't happening — they're structurally identical to the "direct usage" tests above.To meaningfully test the priority logic in
createPostgresDrizzle, you'd need to either mock thepostgresconstructor and assert the URL it received, or expose/inspect the resolvedclientConfig. Without that, regressions in the priority chain (connection.url > url > connectionString > DATABASE_URL) would go undetected.
296-361: Several backward-compatibility tests duplicate "direct usage" tests above.For example, "connectionString still works" (Line 297) is nearly identical to "can create instance with connectionString" (Line 125), and "connection object still works" (Line 310) mirrors "can create instance with connection.url" (Line 138). Consider removing the duplicates or differentiating them (e.g., asserting specific backward-compat behaviour rather than repeating the same existence checks).
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/drizzle/test/config.test.tspackages/server/src/api/sandbox/create.ts
✅ Files skipped from review due to trivial changes (1)
- packages/server/src/api/sandbox/create.ts
🧰 Additional context used
📓 Path-based instructions (6)
packages/drizzle/**/*.{ts,tsx}
📄 CodeRabbit inference engine (packages/drizzle/AGENTS.md)
packages/drizzle/**/*.{ts,tsx}: UsecreatePostgresDrizzle()factory function to create Drizzle ORM instances
Define schemas using Drizzle'spgTableand column types rather than other schema definition methods
Use full TypeScript support with schema inference from Drizzle ORM for type-safe queries
UsedrizzleAdapterfrom better-auth when integrating with@agentuity/auth
Files:
packages/drizzle/test/config.test.ts
packages/drizzle/**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (packages/drizzle/AGENTS.md)
Ensure tests run against a PostgreSQL instance and use
@agentuity/test-utilsfor mocking
Files:
packages/drizzle/test/config.test.ts
**/*.{ts,tsx,js,jsx,json,md}
📄 CodeRabbit inference engine (AGENTS.md)
Use Prettier for code formatting with tabs (width 3), single quotes, and semicolons
Files:
packages/drizzle/test/config.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript in strict mode with ESNext target and bundler moduleResolution
Files:
packages/drizzle/test/config.test.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use
StructuredErrorfrom@agentuity/corefor error handling
Files:
packages/drizzle/test/config.test.ts
packages/*/test/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
packages/*/test/**/*.{ts,tsx}: Place test files intest/folder, never insrc/or__tests__/
Import from../src/in test files
Use@agentuity/test-utilsfor mocks in tests
Files:
packages/drizzle/test/config.test.ts
🧠 Learnings (1)
📚 Learning: 2025-12-21T00:31:41.858Z
Learnt from: jhaynie
Repo: agentuity/sdk PR: 274
File: packages/cli/src/cmd/build/vite/server-bundler.ts:12-41
Timestamp: 2025-12-21T00:31:41.858Z
Learning: In Bun runtime, BuildMessage and ResolveMessage are global types and are not exported from the bun module. Do not import { BuildMessage } from 'bun' or similar; these types are available globally and should be used without import. This applies to all TypeScript files that target the Bun runtime within the repository.
Applied to files:
packages/drizzle/test/config.test.ts
🧬 Code graph analysis (1)
packages/drizzle/test/config.test.ts (2)
packages/drizzle/src/types.ts (1)
PostgresDrizzleConfig(10-58)packages/drizzle/src/postgres.ts (1)
createPostgresDrizzle(49-105)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: SDK Integration Test Suite
- GitHub Check: Playwright E2E Smoke Test
- GitHub Check: Template Integration Tests
- GitHub Check: Framework Integration Tests (TanStack & Next.js)
- GitHub Check: Sandbox CLI Tests
- GitHub Check: Package Installation & Usage Test
- GitHub Check: Build
- GitHub Check: Cloud Deployment Tests
🔇 Additional comments (3)
packages/drizzle/test/config.test.ts (3)
232-251: Good fix —DATABASE_URLis now properly saved and restored.The nested
try/finallywith explicit save/restore ofprocess.env.DATABASE_URLmakes this test deterministic regardless of the CI environment. Nice work addressing the earlier feedback.
1-11: File structure and imports look good.Test file is correctly placed in
packages/drizzle/test/, imports use../src/paths, andbun:testis used as the test runner. Aligns with the coding guidelines. As per coding guidelines, test files should be placed intest/folder and import from../src/.
14-108: Type-compatibility tests are a clean compile-time safety net.The pattern of wrapping config construction in a function and asserting
typeofis a pragmatic way to catch type regressions without needing a running database. These cover all the relevant config shapes well.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| it('can create instance with url', () => { | ||
| const { db, client, close } = createPostgresDrizzle({ | ||
| url: 'postgres://localhost:5432/nonexistent_db', | ||
| }); | ||
| try { | ||
| expect(db).toBeDefined(); | ||
| expect(client).toBeDefined(); | ||
| expect(typeof close).toBe('function'); | ||
| } finally { | ||
| close(); | ||
| } | ||
| }); |
There was a problem hiding this comment.
close() returns a Promise but is never awaited.
close is async () => { await client.close(); } (see postgres.ts Lines 100-102). Dropping the promise in finally blocks means the underlying connection teardown may not complete before the next test starts, potentially leaking sockets or causing flaky interference between tests across the whole file.
Example fix (apply to all `finally` blocks in this file)
- it('can create instance with url', () => {
+ it('can create instance with url', async () => {
const { db, client, close } = createPostgresDrizzle({
url: 'postgres://localhost:5432/nonexistent_db',
});
try {
expect(db).toBeDefined();
expect(client).toBeDefined();
expect(typeof close).toBe('function');
} finally {
- close();
+ await close();
}
});🤖 Prompt for AI Agents
In `@packages/drizzle/test/config.test.ts` around lines 112 - 123, The test calls
createPostgresDrizzle and invokes the returned close function in a finally block
but close is async (async () => { await client.close(); }), so the Promise is
dropped; update all finally blocks in this test file to await close() (e.g.,
change finally { close(); } to finally { await close(); }) and make the
surrounding test callback async so the await is valid; ensure the same change is
applied to every test in this file that calls close in a finally.
1 participant
Summary
urlas a top-level convenience property onPostgresDrizzleConfigso users can pass the connection URL directly instead of nesting it underconnection: { url }.connection.url>url>connectionString>DATABASE_URL.Closes #887
Summary by CodeRabbit
New Features
urlsetting; when present it takes precedence over other connection sources (falls back to other methods as before).Tests