feature(postgres): add listen/notify functionality

[structwafel]

What does this PR do?

Add notify/listen to the native psql

How did you verify your code works?

Tests: test/js/sql/sql-listen.test.ts

#18214

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 93b9b9b6-b334-49d2-9d22-2d263d697d0e

📥 Commits

Reviewing files that changed from the base of the PR and between 2ba992e and aed6a9d.

📒 Files selected for processing (1)

• packages/bun-types/sql.d.ts

---

Walkthrough

Adds PostgreSQL LISTEN/NOTIFY support: new sql.listen() and sql.unlisten() APIs and type declarations, runtime listener management with a dedicated listen connection, protocol-level notification handling in Postgres code, and integration tests verifying behavior.

Changes

Cohort / File(s)	Summary
Type definitions packages/bun-types/sql.d.ts	Adds listen(channel: string, callback: (payload: string) => void): Promise<() => Promise<void>> and unlisten(channel: string): Promise<void> to the SQL interface.
JS runtime class metadata src/bun.js/api/sql.classes.ts	Precomputes per-variant proto/values and includes onnotification in the PostgreSQL variant metadata used when defining the SQL class.
Core SQL implementation src/js/bun/sql.ts	Implements LISTEN/NOTIFY lifecycle: per-channel callback sets, ensureListenConnection/releaseListenConnectionIfUnused, listen() (register + LISTEN, returns unsubscribe), unlisten() (UNLISTEN + cleanup), resubscribe on reconnect, cleanup on sql.close, exposes __pooledConnection, and wires APIs onto the default SQL wrapper.
Postgres request dispatch src/sql/postgres/PostgresRequest.zig	Handles frontend message type 'A' to route NotificationResponse messages into the connection notification path.
Postgres connection lifecycle src/sql/postgres/PostgresSQLConnection.zig	Adds getOnNotification/setOnNotification accessors, keeps poll ref alive when notification listeners exist, and invokes JS onNotification callbacks for NotificationResponse messages.
Notification decoding src/sql/postgres/protocol/NotificationResponse.zig	Adjusts decodeInternal to propagate errors from both readZ() and toOwned() when decoding channel and payload.
Integration tests test/js/sql/sql-listen.test.ts	Adds Docker-gated integration tests covering single/multiple notifications, multiple channels, unlisten behavior, and argument validation.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly describes the main change: adding PostgreSQL LISTEN/NOTIFY functionality to the SQL client.
Description check	✅ Passed	The description covers the required template sections with concrete details about what was added and how it was verified through tests.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 8

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between a5712b9 and c9ec34bc21f7a681d371664bab89b36079d368d7.

📒 Files selected for processing (8)

• flake.nix (2 hunks)
• packages/bun-types/sql.d.ts (1 hunks)
• src/bun.js/api/sql.classes.ts (2 hunks)
• src/js/bun/sql.ts (3 hunks)
• src/sql/postgres/PostgresRequest.zig (1 hunks)
• src/sql/postgres/PostgresSQLConnection.zig (3 hunks)
• src/sql/postgres/protocol/NotificationResponse.zig (1 hunks)
• test/js/sql/sql-listen.test.ts (1 hunks)

🧰 Additional context used

📓 Path-based instructions (9)

src/**/*.{cpp,zig}

📄 CodeRabbit inference engine (.cursor/rules/building-bun.mdc)

src/**/*.{cpp,zig}: Use bun bd or bun run build:debug to build debug versions for C++ and Zig source files; creates debug build at ./build/debug/bun-debug
Run tests using bun bd test <test-file> with the debug build; never use bun test directly as it will not include your changes
Execute files using bun bd <file> <...args>; never use bun <file> directly as it will not include your changes
Enable debug logs for specific scopes using BUN_DEBUG_$(SCOPE)=1 environment variable
Code generation happens automatically as part of the build process; no manual code generation commands are required

Files:

• src/sql/postgres/PostgresRequest.zig
• src/sql/postgres/protocol/NotificationResponse.zig
• src/sql/postgres/PostgresSQLConnection.zig

src/**/*.zig

📄 CodeRabbit inference engine (.cursor/rules/building-bun.mdc)

Use bun.Output.scoped(.${SCOPE}, .hidden) for creating debug logs in Zig code

Implement core functionality in Zig, typically in its own directory in src/

src/**/*.zig: Private fields in Zig are fully supported using the # prefix: struct { #foo: u32 };
Use decl literals in Zig for declaration initialization: const decl: Decl = .{ .binding = 0, .value = 0 };
Prefer @import at the bottom of the file (auto formatter will move them automatically)

Be careful with memory management in Zig code - use defer for cleanup with allocators

Files:

• src/sql/postgres/PostgresRequest.zig
• src/sql/postgres/protocol/NotificationResponse.zig
• src/sql/postgres/PostgresSQLConnection.zig

**/*.zig

📄 CodeRabbit inference engine (.cursor/rules/zig-javascriptcore-classes.mdc)

**/*.zig: Expose generated bindings in Zig structs using pub const js = JSC.Codegen.JS<ClassName> with trait conversion methods: toJS, fromJS, and fromJSDirect
Use consistent parameter name globalObject instead of ctx in Zig constructor and method implementations
Use bun.JSError!JSValue return type for Zig methods and constructors to enable proper error handling and exception propagation
Implement resource cleanup using deinit() method that releases resources, followed by finalize() called by the GC that invokes deinit() and frees the pointer
Use JSC.markBinding(@src()) in finalize methods for debugging purposes before calling deinit()
For methods returning cached properties in Zig, declare external C++ functions using extern fn and callconv(JSC.conv) calling convention
Implement getter functions with naming pattern get<PropertyName> in Zig that accept this and globalObject parameters and return JSC.JSValue
Access JavaScript CallFrame arguments using callFrame.argument(i), check argument count with callFrame.argumentCount(), and get this with callFrame.thisValue()
For reference-counted objects, use .deref() in finalize instead of destroy() to release references to other JS objects

Files:

• src/sql/postgres/PostgresRequest.zig
• src/sql/postgres/protocol/NotificationResponse.zig
• src/sql/postgres/PostgresSQLConnection.zig

**/*.classes.ts

📄 CodeRabbit inference engine (.cursor/rules/zig-javascriptcore-classes.mdc)

**/*.classes.ts: Define JavaScript API using declarative .classes.ts files with properties: name, constructor, JSType, finalize, and proto object containing method/property definitions
Use WriteBarrier caching (cache: true) in .classes.ts property definitions to enable JSC's garbage-collected value storage for computed or lazily-created properties

Files:

• src/bun.js/api/sql.classes.ts

test/**/*.{js,ts,jsx,tsx}

📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)

test/**/*.{js,ts,jsx,tsx}: Write tests as JavaScript and TypeScript files using Jest-style APIs (test, describe, expect) and import from bun:test
Use test.each and data-driven tests to reduce boilerplate when testing multiple similar cases

Files:

• test/js/sql/sql-listen.test.ts

test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}

📄 CodeRabbit inference engine (test/CLAUDE.md)

test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}: Use bun:test with files that end in *.test.{ts,js,jsx,tsx,mjs,cjs}
Do not write flaky tests. Never wait for time to pass in tests; always wait for the condition to be met instead of using an arbitrary amount of time
Never use hardcoded port numbers in tests. Always use port: 0 to get a random port
Prefer concurrent tests over sequential tests using test.concurrent or describe.concurrent when multiple tests spawn processes or write files, unless it's very difficult to make them concurrent
When spawning Bun processes in tests, use bunExe and bunEnv from harness to ensure the same build of Bun is used and debug logging is silenced
Use -e flag for single-file tests when spawning Bun processes
Use tempDir() from harness to create temporary directories with files for multi-file tests instead of creating files manually
Prefer async/await over callbacks in tests
When callbacks must be used and it's just a single callback, use Promise.withResolvers to create a promise that can be resolved or rejected from a callback
Do not set a timeout on tests. Bun already has timeouts
Use Buffer.alloc(count, fill).toString() instead of 'A'.repeat(count) to create repetitive strings in tests, as ''.repeat is very slow in debug JavaScriptCore builds
Use describe blocks for grouping related tests
Always use await using or using to ensure proper resource cleanup in tests for APIs like Bun.listen, Bun.connect, Bun.spawn, Bun.serve, etc
Always check exit codes and test error scenarios in error tests
Use describe.each() for parameterized tests
Use toMatchSnapshot() for snapshot testing
Use beforeAll(), afterEach(), beforeEach() for setup/teardown in tests
Track resources (servers, clients) in arrays for cleanup in afterEach()

Files:

• test/js/sql/sql-listen.test.ts

test/**/*.test.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

test/**/*.test.{ts,tsx}: For single-file tests in Bun test suite, prefer using -e flag over tempDir
For multi-file tests in Bun test suite, prefer using tempDir and Bun.spawn
Always use port: 0 when spawning servers in tests - do not hardcode ports or use custom random port functions
Use normalizeBunSnapshot to normalize snapshot output in tests instead of manual output comparison
Never write tests that check for no 'panic', 'uncaught exception', or similar strings in test output - that is not a valid test
Use tempDir from harness to create temporary directories in tests - do not use tmpdirSync or fs.mkdtempSync
In tests, call expect(stdout).toBe(...) before expect(exitCode).toBe(0) when spawning processes for more useful error messages on failure
Do not write flaky tests - do not use setTimeout in tests; instead await the condition to be met since you're testing the CONDITION, not TIME PASSING
Verify your test fails with USE_SYSTEM_BUN=1 bun test <file> and passes with bun bd test <file> - tests are not valid if they pass with USE_SYSTEM_BUN=1
Avoid shell commands in tests - do not use find or grep; use Bun's Glob and built-in tools instead
Test files must end in .test.ts or .test.tsx and be created in the appropriate test folder structure

Files:

• test/js/sql/sql-listen.test.ts

src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js}

📄 CodeRabbit inference engine (src/js/CLAUDE.md)

src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js}: Use .$call() and .$apply() instead of .call() and .apply() to prevent user tampering with function invocation
Use string literal require() statements only; dynamic requires are not permitted
Export modules using export default { ... } syntax; modules are NOT ES modules
Use JSC intrinsics (prefixed with $) such as $Array.from(), $isCallable(), and $newArrayWithSize() for performance-critical operations
Use private globals and methods with $ prefix (e.g., $Array, map.$set()) instead of public JavaScript globals
Use $debug() for debug logging and $assert() for assertions; both are stripped in release builds
Validate function arguments using validators from internal/validators and throw $ERR_* error codes for invalid arguments
Use process.platform and process.arch for platform detection; these values are inlined and dead-code eliminated at build time

Files:

• src/js/bun/sql.ts

src/js/{builtins,node,bun,thirdparty,internal}/**/*.ts

📄 CodeRabbit inference engine (src/js/CLAUDE.md)

Builtin functions must include this parameter typing in TypeScript to enable direct method binding in C++

Files:

• src/js/bun/sql.ts

🧠 Learnings (23)

📓 Common learnings

Learnt from: cirospaciari
Repo: oven-sh/bun PR: 22946
File: test/js/sql/sql.test.ts:195-202
Timestamp: 2025-09-25T22:07:13.851Z
Learning: PR oven-sh/bun#22946: JSON/JSONB result parsing updates (e.g., returning parsed arrays instead of legacy strings) are out of scope for this PR; tests keep current expectations with a TODO. Handle parsing fixes in a separate PR.

📚 Learning: 2025-09-25T22:07:13.851Z

Learnt from: cirospaciari
Repo: oven-sh/bun PR: 22946
File: test/js/sql/sql.test.ts:195-202
Timestamp: 2025-09-25T22:07:13.851Z
Learning: PR oven-sh/bun#22946: JSON/JSONB result parsing updates (e.g., returning parsed arrays instead of legacy strings) are out of scope for this PR; tests keep current expectations with a TODO. Handle parsing fixes in a separate PR.

Applied to files:

• src/bun.js/api/sql.classes.ts

📚 Learning: 2025-09-25T01:04:19.262Z

Learnt from: cirospaciari
Repo: oven-sh/bun PR: 22946
File: src/js/internal/sql/postgres.ts:1185-1416
Timestamp: 2025-09-25T01:04:19.262Z
Learning: In the PostgreSQL array implementation for Bun.SQL, when serializing arrays for JSON/JSONB columns, the user prefers using PostgreSQL array literal format (serializeArray(value, "JSON")) over standard JSON serialization (JSON.stringify(value)). PostgreSQL JSON array handling is different from JavaScript JSON array handling according to the user's requirements.

Applied to files:

• src/bun.js/api/sql.classes.ts

📚 Learning: 2025-09-26T01:36:48.705Z

Learnt from: cirospaciari
Repo: oven-sh/bun PR: 22946
File: src/js/internal/sql/postgres.ts:1326-1332
Timestamp: 2025-09-26T01:36:48.705Z
Learning: In PostgreSQL single-object INSERT path (src/js/internal/sql/postgres.ts), when handling array values in columnValue, the user prefers to rely on PostgreSQL's native type conversion rather than explicitly checking for typed arrays with isTypedArray(). The current $isArray(columnValue) check with serializeArray(columnValue, "JSON") is sufficient, and native PostgreSQL handling is preferred over explicit typed array detection in this specific INSERT context.

Applied to files:

• src/bun.js/api/sql.classes.ts

📚 Learning: 2025-11-24T18:36:08.558Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-11-24T18:36:08.558Z
Learning: Applies to **/*.classes.ts : Define JavaScript API using declarative `.classes.ts` files with properties: name, constructor, JSType, finalize, and proto object containing method/property definitions

Applied to files:

• src/bun.js/api/sql.classes.ts

📚 Learning: 2025-11-24T18:37:30.259Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Always use `await using` or `using` to ensure proper resource cleanup in tests for APIs like Bun.listen, Bun.connect, Bun.spawn, Bun.serve, etc

Applied to files:

• test/js/sql/sql-listen.test.ts

📚 Learning: 2025-09-03T01:30:58.001Z

Learnt from: markovejnovic
Repo: oven-sh/bun PR: 21728
File: test/js/valkey/valkey.test.ts:264-271
Timestamp: 2025-09-03T01:30:58.001Z
Learning: For test/js/valkey/valkey.test.ts PUB/SUB tests, avoid arbitrary sleeps and async-forEach. Instead, resolve a Promise from the subscriber callback when the expected number of messages is observed and await it with a bounded timeout (e.g., withTimeout + Promise.withResolvers) to account for Redis server→subscriber propagation.

Applied to files:

• test/js/sql/sql-listen.test.ts

📚 Learning: 2025-09-30T02:56:30.615Z

Learnt from: cirospaciari
Repo: oven-sh/bun PR: 22700
File: test/js/sql/sql.test.ts:11673-11710
Timestamp: 2025-09-30T02:56:30.615Z
Learning: Repository oven-sh/bun tests run in a Docker image where the pgcrypto extension is pre-installed, so gen_random_uuid() is available without explicitly running CREATE EXTENSION in tests (e.g., in test/js/sql/sql.test.ts “upsert helper”).

Applied to files:

• test/js/sql/sql-listen.test.ts

📚 Learning: 2025-12-02T05:59:51.485Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-02T05:59:51.485Z
Learning: Applies to test/**/*.test.{ts,tsx} : Use `normalizeBunSnapshot` to normalize snapshot output in tests instead of manual output comparison

Applied to files:

• test/js/sql/sql-listen.test.ts

📚 Learning: 2025-11-24T18:35:50.422Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-11-24T18:35:50.422Z
Learning: Applies to test/**/*.{js,ts,jsx,tsx} : Write tests as JavaScript and TypeScript files using Jest-style APIs (`test`, `describe`, `expect`) and import from `bun:test`

Applied to files:

• test/js/sql/sql-listen.test.ts

📚 Learning: 2025-11-24T18:36:59.706Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/test/v8/v8.test.ts : Add corresponding test cases to test/v8/v8.test.ts using checkSameOutput() function to compare Node.js and Bun output

Applied to files:

• test/js/sql/sql-listen.test.ts

📚 Learning: 2025-10-19T02:44:46.354Z

Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: packages/bun-otel/context-propagation.test.ts:1-1
Timestamp: 2025-10-19T02:44:46.354Z
Learning: In the Bun repository, standalone packages under packages/ (e.g., bun-vscode, bun-inspector-protocol, bun-plugin-yaml, bun-plugin-svelte, bun-debug-adapter-protocol, bun-otel) co-locate their tests with package source code using *.test.ts files. This follows standard npm/monorepo patterns. The test/ directory hierarchy (test/js/bun/, test/cli/, test/js/node/) is reserved for testing Bun's core runtime APIs and built-in functionality, not standalone packages.

Applied to files:

• test/js/sql/sql-listen.test.ts

📚 Learning: 2025-10-18T05:23:24.403Z

Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: test/js/bun/telemetry-server.test.ts:91-100
Timestamp: 2025-10-18T05:23:24.403Z
Learning: In the Bun codebase, telemetry tests (test/js/bun/telemetry-*.test.ts) should focus on telemetry API behavior: configure/disable/isEnabled, callback signatures and invocation, request ID correlation, and error handling. HTTP protocol behaviors like status code normalization (e.g., 200 with empty body → 204) should be tested in HTTP server tests (test/js/bun/http/), not in telemetry tests. Keep separation of concerns: telemetry tests verify the telemetry API contract; HTTP tests verify HTTP semantics.

Applied to files:

• test/js/sql/sql-listen.test.ts

📚 Learning: 2025-11-24T18:37:30.259Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*-fixture.ts : Test files that spawn Bun processes should end in `*-fixture.ts` to identify them as test fixtures and not tests themselves

Applied to files:

• test/js/sql/sql-listen.test.ts

📚 Learning: 2025-11-24T18:37:30.259Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Prefer async/await over callbacks in tests

Applied to files:

• test/js/sql/sql-listen.test.ts

📚 Learning: 2025-10-30T03:48:10.513Z

Learnt from: theshadow27
Repo: oven-sh/bun PR: 24063
File: packages/bun-otel/test/context-propagation.test.ts:1-7
Timestamp: 2025-10-30T03:48:10.513Z
Learning: In Bun test files, `using` declarations at the describe block level execute during module load/parsing, not during test execution. This means they acquire and dispose resources before any tests run. For test-scoped resource management, use beforeAll/afterAll hooks instead. The pattern `beforeAll(beforeUsingEchoServer); afterAll(afterUsingEchoServer);` is correct for managing ref-counted test resources like the EchoServer in packages/bun-otel/test/ - the using block pattern should not be used at describe-block level for test resources.
<!-- [/add_learning]

Applied to files:

• test/js/sql/sql-listen.test.ts

📚 Learning: 2025-12-02T05:59:51.485Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-02T05:59:51.485Z
Learning: Applies to test/**/*.test.{ts,tsx} : Verify your test fails with `USE_SYSTEM_BUN=1 bun test <file>` and passes with `bun bd test <file>` - tests are not valid if they pass with `USE_SYSTEM_BUN=1`

Applied to files:

• test/js/sql/sql-listen.test.ts

📚 Learning: 2025-09-20T00:58:38.042Z

Learnt from: markovejnovic
Repo: oven-sh/bun PR: 22568
File: test/js/valkey/valkey.test.ts:561-564
Timestamp: 2025-09-20T00:58:38.042Z
Learning: For test/js/valkey/valkey.test.ts, do not comment on synchronous throw assertions for async Redis methods (like ctx.redis.set(), ctx.redis.unsubscribe(), etc.) - Bun's Redis client implementation differs from Node.js and can throw synchronously even for async methods. The maintainer has explicitly requested to stop looking at this error pattern.

Applied to files:

• test/js/sql/sql-listen.test.ts

📚 Learning: 2025-10-26T01:32:04.844Z

Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 24082
File: test/cli/test/coverage.test.ts:60-112
Timestamp: 2025-10-26T01:32:04.844Z
Learning: In the Bun repository test files (test/cli/test/*.test.ts), when spawning Bun CLI commands with Bun.spawnSync for testing, prefer using stdio: ["inherit", "inherit", "inherit"] to inherit stdio streams rather than piping them.

Applied to files:

• test/js/sql/sql-listen.test.ts

📚 Learning: 2025-10-08T13:48:02.430Z

Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 23373
File: test/js/bun/tarball/extract.test.ts:107-111
Timestamp: 2025-10-08T13:48:02.430Z
Learning: In Bun's test runner, use `expect(async () => { await ... }).toThrow()` to assert async rejections. Unlike Jest/Vitest, Bun does not require `await expect(...).rejects.toThrow()` - the async function wrapper with `.toThrow()` is the correct pattern for async error assertions in Bun tests.

Applied to files:

• test/js/sql/sql-listen.test.ts

📚 Learning: 2025-11-24T18:37:30.259Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : When spawning Bun processes in tests, use `bunExe` and `bunEnv` from `harness` to ensure the same build of Bun is used and debug logging is silenced

Applied to files:

• test/js/sql/sql-listen.test.ts

📚 Learning: 2025-09-15T20:47:57.118Z

Learnt from: cirospaciari
Repo: oven-sh/bun PR: 22696
File: src/sql/mysql/js/JSMySQLConnection.zig:306-322
Timestamp: 2025-09-15T20:47:57.118Z
Learning: bun.Async.KeepAlive uses a simple state machine (active/inactive/done) rather than reference counting, with internal status field preventing issues from multiple ref/unref calls, making additional idempotence guards unnecessary.

Applied to files:

• src/sql/postgres/PostgresSQLConnection.zig

📚 Learning: 2025-11-24T18:34:55.173Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/building-bun.mdc:0-0
Timestamp: 2025-11-24T18:34:55.173Z
Learning: Applies to src/**/*.{cpp,zig} : Use `bun bd` or `bun run build:debug` to build debug versions for C++ and Zig source files; creates debug build at `./build/debug/bun-debug`

Applied to files:

• flake.nix

🧬 Code graph analysis (1)

src/bun.js/api/sql.classes.ts (1)

src/codegen/class-definitions.ts (1)

• define (258-307)

🔇 Additional comments (9)

flake.nix (1)

139-140: Debug build fix is reasonable but tangential to the main feature.

Disabling Fortify hardening for debug builds is a standard practice when using -O0 optimization, as _FORTIFY_SOURCE requires -O1 or higher. This change is appropriate for a development environment.

However, this appears to be a general development environment fix rather than something specific to the PostgreSQL LISTEN/NOTIFY feature. If this was blocking your development, it's fine to include here, but consider mentioning in the PR description why this became necessary (e.g., "Fixed debug build compilation issue encountered during development").

src/sql/postgres/protocol/NotificationResponse.zig (1)

12-21: Good: propagate OOM from toOwned()

Wrapping toOwned() with try is the right fix so allocation failures don’t get silently ignored.

src/sql/postgres/PostgresRequest.zig (1)

262-323: Add A → .NotificationResponse dispatch looks correct

Postgres backend message type A is NotificationResponse, so routing it via connection.on(.NotificationResponse, ...) is the expected plumbing.

src/bun.js/api/sql.classes.ts (1)

5-68: LGTM: Postgres-only onnotification wiring and per-variant proto/values

The per-type proto/values construction and conditional onnotification accessor match the new Zig getOnNotification/setOnNotification API and keeps MySQL unaffected.

packages/bun-types/sql.d.ts (1)

695-739: Types look consistent with JS API (payload-only callback + returned unlisten fn)

listen(channel, cb) returning an async unlisten() and unlisten(channel) are consistent with the intended UX. Please double-check that non-Postgres adapters throw a clear runtime error since the method is declared on SQL universally.

src/sql/postgres/PostgresSQLConnection.zig (2)

171-183: Good: setOnNotification triggers updateRef()

This matches the intent: changing listener presence immediately updates whether the poll handle should keep the process alive.

---

504-526: Keep-alive logic is reasonable; verify “listener present” detection matches codegen semantics

You’re treating js.onnotificationGetCached(this.js_value) != null as “listener exists”. Please sanity-check that setting onnotification = undefined (from JS) makes GetCached return null (not a non-null .js_undefined), otherwise Bun may stay artificially alive.

Also applies to: 1832-1841

src/js/bun/sql.ts (2)

1163-1170: Delegating defaultSQLObject.listen/unlisten is fine

---

942-1083: All internal API assumptions verified

The three required APIs exist with the expected shapes:

1. pool.escapeIdentifier(...) is properly defined on the Postgres adapter (and all other adapters) at src/js/internal/sql/shared.ts:901 and implemented at src/js/internal/sql/postgres.ts:699. Used consistently throughout the codebase.

2. __pooledConnection property is the established internal API for accessing the pooled connection object, set at src/js/bun/sql.ts:454 and accessed throughout the codebase via the same pattern used in the review code.

3. onnotification is properly plumbed as a Zig-backed connection property with getter/setter defined in src/bun.js/api/sql.classes.ts:42-47 and implemented in src/sql/postgres/PostgresSQLConnection.zig:171-179. The Zig layer properly handles notification callbacks via js.onnotificationGetCached() and js.onnotificationSetCached().

The code follows the established patterns and makes valid assumptions about these internal APIs.

[coderabbitai]

Actionable comments posted: 2

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between c9ec34bc21f7a681d371664bab89b36079d368d7 and 30093cd54be07f2d0f825cfbfea5f86e5143424d.

📒 Files selected for processing (3)

• src/js/bun/sql.ts (4 hunks)
• src/sql/postgres/PostgresSQLConnection.zig (3 hunks)
• test/js/sql/sql-listen.test.ts (1 hunks)

🧰 Additional context used

📓 Path-based instructions (9)

test/**/*.{js,ts,jsx,tsx}

📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)

test/**/*.{js,ts,jsx,tsx}: Write tests as JavaScript and TypeScript files using Jest-style APIs (test, describe, expect) and import from bun:test
Use test.each and data-driven tests to reduce boilerplate when testing multiple similar cases

Files:

• test/js/sql/sql-listen.test.ts

test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}

📄 CodeRabbit inference engine (test/CLAUDE.md)

test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}: Use bun:test with files that end in *.test.{ts,js,jsx,tsx,mjs,cjs}
Do not write flaky tests. Never wait for time to pass in tests; always wait for the condition to be met instead of using an arbitrary amount of time
Never use hardcoded port numbers in tests. Always use port: 0 to get a random port
Prefer concurrent tests over sequential tests using test.concurrent or describe.concurrent when multiple tests spawn processes or write files, unless it's very difficult to make them concurrent
When spawning Bun processes in tests, use bunExe and bunEnv from harness to ensure the same build of Bun is used and debug logging is silenced
Use -e flag for single-file tests when spawning Bun processes
Use tempDir() from harness to create temporary directories with files for multi-file tests instead of creating files manually
Prefer async/await over callbacks in tests
When callbacks must be used and it's just a single callback, use Promise.withResolvers to create a promise that can be resolved or rejected from a callback
Do not set a timeout on tests. Bun already has timeouts
Use Buffer.alloc(count, fill).toString() instead of 'A'.repeat(count) to create repetitive strings in tests, as ''.repeat is very slow in debug JavaScriptCore builds
Use describe blocks for grouping related tests
Always use await using or using to ensure proper resource cleanup in tests for APIs like Bun.listen, Bun.connect, Bun.spawn, Bun.serve, etc
Always check exit codes and test error scenarios in error tests
Use describe.each() for parameterized tests
Use toMatchSnapshot() for snapshot testing
Use beforeAll(), afterEach(), beforeEach() for setup/teardown in tests
Track resources (servers, clients) in arrays for cleanup in afterEach()

Files:

• test/js/sql/sql-listen.test.ts

**/*.test.ts?(x)

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.test.ts?(x): Never use bun test directly - always use bun bd test to run tests with debug build changes
For single-file tests, prefer -e flag over tempDir
For multi-file tests, prefer tempDir and Bun.spawn over single-file tests
Use normalizeBunSnapshot to normalize snapshot output of tests
Never write tests that check for 'panic', 'uncaught exception', or similar strings in test output
Use tempDir from harness to create temporary directories - do not use tmpdirSync or fs.mkdtempSync
When spawning processes in tests, expect stdout before expecting exit code for more useful error messages on test failure
Do not write flaky tests - do not use setTimeout in tests; instead await the condition to be met
Verify tests fail with USE_SYSTEM_BUN=1 bun test <file> and pass with bun bd test <file> - tests are invalid if they pass with USE_SYSTEM_BUN=1
Test files must end with .test.ts or .test.tsx
Avoid shell commands like find or grep in tests - use Bun's Glob and built-in tools instead

Files:

• test/js/sql/sql-listen.test.ts

test/**/*.test.ts?(x)

📄 CodeRabbit inference engine (CLAUDE.md)

Always use port: 0 in tests - do not hardcode ports or use custom random port number functions

Files:

• test/js/sql/sql-listen.test.ts

src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js}

📄 CodeRabbit inference engine (src/js/CLAUDE.md)

src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js}: Use .$call() and .$apply() instead of .call() and .apply() to prevent user tampering with function invocation
Use string literal require() statements only; dynamic requires are not permitted
Export modules using export default { ... } syntax; modules are NOT ES modules
Use JSC intrinsics (prefixed with $) such as $Array.from(), $isCallable(), and $newArrayWithSize() for performance-critical operations
Use private globals and methods with $ prefix (e.g., $Array, map.$set()) instead of public JavaScript globals
Use $debug() for debug logging and $assert() for assertions; both are stripped in release builds
Validate function arguments using validators from internal/validators and throw $ERR_* error codes for invalid arguments
Use process.platform and process.arch for platform detection; these values are inlined and dead-code eliminated at build time

Files:

• src/js/bun/sql.ts

src/js/{builtins,node,bun,thirdparty,internal}/**/*.ts

📄 CodeRabbit inference engine (src/js/CLAUDE.md)

Builtin functions must include this parameter typing in TypeScript to enable direct method binding in C++

Files:

• src/js/bun/sql.ts

src/**/*.{cpp,zig}

📄 CodeRabbit inference engine (.cursor/rules/building-bun.mdc)

src/**/*.{cpp,zig}: Use bun bd or bun run build:debug to build debug versions for C++ and Zig source files; creates debug build at ./build/debug/bun-debug
Run tests using bun bd test <test-file> with the debug build; never use bun test directly as it will not include your changes
Execute files using bun bd <file> <...args>; never use bun <file> directly as it will not include your changes
Enable debug logs for specific scopes using BUN_DEBUG_$(SCOPE)=1 environment variable
Code generation happens automatically as part of the build process; no manual code generation commands are required

Files:

• src/sql/postgres/PostgresSQLConnection.zig

src/**/*.zig

📄 CodeRabbit inference engine (.cursor/rules/building-bun.mdc)

Use bun.Output.scoped(.${SCOPE}, .hidden) for creating debug logs in Zig code

Implement core functionality in Zig, typically in its own directory in src/

src/**/*.zig: Private fields in Zig are fully supported using the # prefix: struct { #foo: u32 };
Use decl literals in Zig for declaration initialization: const decl: Decl = .{ .binding = 0, .value = 0 };
Prefer @import at the bottom of the file (auto formatter will move them automatically)

Files:

• src/sql/postgres/PostgresSQLConnection.zig

**/*.zig

📄 CodeRabbit inference engine (.cursor/rules/zig-javascriptcore-classes.mdc)

**/*.zig: Expose generated bindings in Zig structs using pub const js = JSC.Codegen.JS<ClassName> with trait conversion methods: toJS, fromJS, and fromJSDirect
Use consistent parameter name globalObject instead of ctx in Zig constructor and method implementations
Use bun.JSError!JSValue return type for Zig methods and constructors to enable proper error handling and exception propagation
Implement resource cleanup using deinit() method that releases resources, followed by finalize() called by the GC that invokes deinit() and frees the pointer
Use JSC.markBinding(@src()) in finalize methods for debugging purposes before calling deinit()
For methods returning cached properties in Zig, declare external C++ functions using extern fn and callconv(JSC.conv) calling convention
Implement getter functions with naming pattern get<PropertyName> in Zig that accept this and globalObject parameters and return JSC.JSValue
Access JavaScript CallFrame arguments using callFrame.argument(i), check argument count with callFrame.argumentCount(), and get this with callFrame.thisValue()
For reference-counted objects, use .deref() in finalize instead of destroy() to release references to other JS objects

In Zig code, be careful with allocators and use defer for cleanup

Files:

• src/sql/postgres/PostgresSQLConnection.zig

🧠 Learnings (35)

📓 Common learnings

Learnt from: cirospaciari
Repo: oven-sh/bun PR: 22946
File: test/js/sql/sql.test.ts:195-202
Timestamp: 2025-09-25T22:07:13.851Z
Learning: PR oven-sh/bun#22946: JSON/JSONB result parsing updates (e.g., returning parsed arrays instead of legacy strings) are out of scope for this PR; tests keep current expectations with a TODO. Handle parsing fixes in a separate PR.

📚 Learning: 2025-11-24T18:37:30.259Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Always use `await using` or `using` to ensure proper resource cleanup in tests for APIs like Bun.listen, Bun.connect, Bun.spawn, Bun.serve, etc

Applied to files:

• test/js/sql/sql-listen.test.ts

📚 Learning: 2025-11-24T18:35:50.422Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-11-24T18:35:50.422Z
Learning: Applies to test/**/*.{js,ts,jsx,tsx} : Write tests as JavaScript and TypeScript files using Jest-style APIs (`test`, `describe`, `expect`) and import from `bun:test`

Applied to files:

• test/js/sql/sql-listen.test.ts

📚 Learning: 2025-11-24T18:36:59.706Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/test/v8/v8.test.ts : Add corresponding test cases to test/v8/v8.test.ts using checkSameOutput() function to compare Node.js and Bun output

Applied to files:

• test/js/sql/sql-listen.test.ts

📚 Learning: 2025-09-30T02:56:30.615Z

Learnt from: cirospaciari
Repo: oven-sh/bun PR: 22700
File: test/js/sql/sql.test.ts:11673-11710
Timestamp: 2025-09-30T02:56:30.615Z
Learning: Repository oven-sh/bun tests run in a Docker image where the pgcrypto extension is pre-installed, so gen_random_uuid() is available without explicitly running CREATE EXTENSION in tests (e.g., in test/js/sql/sql.test.ts “upsert helper”).

Applied to files:

• test/js/sql/sql-listen.test.ts

📚 Learning: 2025-11-24T18:37:30.259Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*-fixture.ts : Test files that spawn Bun processes should end in `*-fixture.ts` to identify them as test fixtures and not tests themselves

Applied to files:

• test/js/sql/sql-listen.test.ts

📚 Learning: 2025-09-03T01:30:58.001Z

Learnt from: markovejnovic
Repo: oven-sh/bun PR: 21728
File: test/js/valkey/valkey.test.ts:264-271
Timestamp: 2025-09-03T01:30:58.001Z
Learning: For test/js/valkey/valkey.test.ts PUB/SUB tests, avoid arbitrary sleeps and async-forEach. Instead, resolve a Promise from the subscriber callback when the expected number of messages is observed and await it with a bounded timeout (e.g., withTimeout + Promise.withResolvers) to account for Redis server→subscriber propagation.

Applied to files:

• test/js/sql/sql-listen.test.ts

📚 Learning: 2025-10-19T02:44:46.354Z

Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: packages/bun-otel/context-propagation.test.ts:1-1
Timestamp: 2025-10-19T02:44:46.354Z
Learning: In the Bun repository, standalone packages under packages/ (e.g., bun-vscode, bun-inspector-protocol, bun-plugin-yaml, bun-plugin-svelte, bun-debug-adapter-protocol, bun-otel) co-locate their tests with package source code using *.test.ts files. This follows standard npm/monorepo patterns. The test/ directory hierarchy (test/js/bun/, test/cli/, test/js/node/) is reserved for testing Bun's core runtime APIs and built-in functionality, not standalone packages.

Applied to files:

• test/js/sql/sql-listen.test.ts

📚 Learning: 2025-11-24T18:37:30.259Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Use `bun:test` with files that end in `*.test.{ts,js,jsx,tsx,mjs,cjs}`

Applied to files:

• test/js/sql/sql-listen.test.ts

📚 Learning: 2025-11-24T18:37:30.259Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Track resources (servers, clients) in arrays for cleanup in `afterEach()`

Applied to files:

• test/js/sql/sql-listen.test.ts

📚 Learning: 2025-11-24T18:37:30.259Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Use `-e` flag for single-file tests when spawning Bun processes

Applied to files:

• test/js/sql/sql-listen.test.ts

📚 Learning: 2025-10-30T03:48:10.513Z

Learnt from: theshadow27
Repo: oven-sh/bun PR: 24063
File: packages/bun-otel/test/context-propagation.test.ts:1-7
Timestamp: 2025-10-30T03:48:10.513Z
Learning: In Bun test files, `using` declarations at the describe block level execute during module load/parsing, not during test execution. This means they acquire and dispose resources before any tests run. For test-scoped resource management, use beforeAll/afterAll hooks instead. The pattern `beforeAll(beforeUsingEchoServer); afterAll(afterUsingEchoServer);` is correct for managing ref-counted test resources like the EchoServer in packages/bun-otel/test/ - the using block pattern should not be used at describe-block level for test resources.
<!-- [/add_learning]

Applied to files:

• test/js/sql/sql-listen.test.ts

📚 Learning: 2025-12-16T00:21:32.179Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : Do not write flaky tests - do not use `setTimeout` in tests; instead await the condition to be met

Applied to files:

• test/js/sql/sql-listen.test.ts

📚 Learning: 2025-11-24T18:37:30.259Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Prefer async/await over callbacks in tests

Applied to files:

• test/js/sql/sql-listen.test.ts

📚 Learning: 2025-11-24T18:37:30.259Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Do not write flaky tests. Never wait for time to pass in tests; always wait for the condition to be met instead of using an arbitrary amount of time

Applied to files:

• test/js/sql/sql-listen.test.ts

📚 Learning: 2025-11-24T18:37:30.259Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Use `beforeAll()`, `afterEach()`, `beforeEach()` for setup/teardown in tests

Applied to files:

• test/js/sql/sql-listen.test.ts

📚 Learning: 2025-09-20T00:58:38.042Z

Learnt from: markovejnovic
Repo: oven-sh/bun PR: 22568
File: test/js/valkey/valkey.test.ts:561-564
Timestamp: 2025-09-20T00:58:38.042Z
Learning: For test/js/valkey/valkey.test.ts, do not comment on synchronous throw assertions for async Redis methods (like ctx.redis.set(), ctx.redis.unsubscribe(), etc.) - Bun's Redis client implementation differs from Node.js and can throw synchronously even for async methods. The maintainer has explicitly requested to stop looking at this error pattern.

Applied to files:

• test/js/sql/sql-listen.test.ts

📚 Learning: 2025-11-06T00:58:23.965Z

Learnt from: markovejnovic
Repo: oven-sh/bun PR: 24417
File: test/js/bun/spawn/spawn.test.ts:903-918
Timestamp: 2025-11-06T00:58:23.965Z
Learning: In Bun test files, `await using` with spawn() is appropriate for long-running processes that need guaranteed cleanup on scope exit or when explicitly testing disposal behavior. For short-lived processes that exit naturally (e.g., console.log scripts), the pattern `const proc = spawn(...); await proc.exited;` is standard and more common, as evidenced by 24 instances vs 4 `await using` instances in test/js/bun/spawn/spawn.test.ts.

Applied to files:

• test/js/sql/sql-listen.test.ts

📚 Learning: 2025-09-20T00:57:56.685Z

Learnt from: markovejnovic
Repo: oven-sh/bun PR: 22568
File: test/js/valkey/valkey.test.ts:268-276
Timestamp: 2025-09-20T00:57:56.685Z
Learning: For test/js/valkey/valkey.test.ts, do not comment on synchronous throw assertions for async Redis methods like ctx.redis.set() - the maintainer has explicitly requested to stop looking at this error pattern.

Applied to files:

• test/js/sql/sql-listen.test.ts

📚 Learning: 2025-10-18T05:23:24.403Z

Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: test/js/bun/telemetry-server.test.ts:91-100
Timestamp: 2025-10-18T05:23:24.403Z
Learning: In the Bun codebase, telemetry tests (test/js/bun/telemetry-*.test.ts) should focus on telemetry API behavior: configure/disable/isEnabled, callback signatures and invocation, request ID correlation, and error handling. HTTP protocol behaviors like status code normalization (e.g., 200 with empty body → 204) should be tested in HTTP server tests (test/js/bun/http/), not in telemetry tests. Keep separation of concerns: telemetry tests verify the telemetry API contract; HTTP tests verify HTTP semantics.

Applied to files:

• test/js/sql/sql-listen.test.ts

📚 Learning: 2025-12-16T00:21:32.179Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : Never write tests that check for 'panic', 'uncaught exception', or similar strings in test output

Applied to files:

• test/js/sql/sql-listen.test.ts

📚 Learning: 2025-11-24T18:37:30.259Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Do not set a timeout on tests. Bun already has timeouts

Applied to files:

• test/js/sql/sql-listen.test.ts

📚 Learning: 2025-10-08T13:48:02.430Z

Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 23373
File: test/js/bun/tarball/extract.test.ts:107-111
Timestamp: 2025-10-08T13:48:02.430Z
Learning: In Bun's test runner, use `expect(async () => { await ... }).toThrow()` to assert async rejections. Unlike Jest/Vitest, Bun does not require `await expect(...).rejects.toThrow()` - the async function wrapper with `.toThrow()` is the correct pattern for async error assertions in Bun tests.

Applied to files:

• test/js/sql/sql-listen.test.ts

📚 Learning: 2025-11-03T20:40:59.655Z

Learnt from: pfgithub
Repo: oven-sh/bun PR: 24273
File: src/bun.js/bindings/JSValue.zig:545-586
Timestamp: 2025-11-03T20:40:59.655Z
Learning: In Bun's Zig codebase, JSErrors (returned as `bun.JSError!T`) must always be properly handled. Using `catch continue` or `catch { break; }` to silently suppress JSErrors is a bug. Errors should either be explicitly handled or propagated with `try`. This applies to snapshot serializer error handling where Jest's behavior is to throw when serializers throw.

Applied to files:

• test/js/sql/sql-listen.test.ts
• src/sql/postgres/PostgresSQLConnection.zig

📚 Learning: 2025-11-24T18:37:30.259Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : When spawning Bun processes in tests, use `bunExe` and `bunEnv` from `harness` to ensure the same build of Bun is used and debug logging is silenced

Applied to files:

• test/js/sql/sql-listen.test.ts

📚 Learning: 2025-12-16T00:21:32.179Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : Verify tests fail with `USE_SYSTEM_BUN=1 bun test <file>` and pass with `bun bd test <file>` - tests are invalid if they pass with USE_SYSTEM_BUN=1

Applied to files:

• test/js/sql/sql-listen.test.ts

📚 Learning: 2025-10-26T01:32:04.844Z

Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 24082
File: test/cli/test/coverage.test.ts:60-112
Timestamp: 2025-10-26T01:32:04.844Z
Learning: In the Bun repository test files (test/cli/test/*.test.ts), when spawning Bun CLI commands with Bun.spawnSync for testing, prefer using stdio: ["inherit", "inherit", "inherit"] to inherit stdio streams rather than piping them.

Applied to files:

• test/js/sql/sql-listen.test.ts

📚 Learning: 2025-11-24T18:35:50.422Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-11-24T18:35:50.422Z
Learning: Applies to test/cli/**/*.{js,ts,jsx,tsx} : When testing Bun as a CLI, use the `spawn` API from `bun` with the `bunExe()` and `bunEnv` from `harness` to execute Bun commands and validate exit codes, stdout, and stderr

Applied to files:

• test/js/sql/sql-listen.test.ts

📚 Learning: 2025-11-24T18:37:11.466Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:11.466Z
Learning: Applies to src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js} : Use private globals and methods with `$` prefix (e.g., `$Array`, `map.$set()`) instead of public JavaScript globals

Applied to files:

• src/js/bun/sql.ts

📚 Learning: 2025-11-24T18:35:39.205Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/registering-bun-modules.mdc:0-0
Timestamp: 2025-11-24T18:35:39.205Z
Learning: Applies to **/js_*.zig : Use `bun.JSError!JSValue` for proper error propagation in JavaScript bindings

Applied to files:

• src/sql/postgres/PostgresSQLConnection.zig

📚 Learning: 2025-11-24T18:36:08.558Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-11-24T18:36:08.558Z
Learning: Applies to **/*.zig : Use `bun.JSError!JSValue` return type for Zig methods and constructors to enable proper error handling and exception propagation

Applied to files:

• src/sql/postgres/PostgresSQLConnection.zig

📚 Learning: 2025-09-05T18:45:29.200Z

Learnt from: markovejnovic
Repo: oven-sh/bun PR: 21728
File: src/valkey/js_valkey.zig:0-0
Timestamp: 2025-09-05T18:45:29.200Z
Learning: In JSValkeyClient.cloneWithoutConnecting() in src/valkey/js_valkey.zig, the address/username/password fields must be repointed to the duplicated connection_strings buffer to avoid use-after-free when the original client is destroyed. The original client properly frees connection_strings in ValkeyClient.deinit().

Applied to files:

• src/sql/postgres/PostgresSQLConnection.zig

📚 Learning: 2025-09-05T19:49:26.188Z

Learnt from: markovejnovic
Repo: oven-sh/bun PR: 21728
File: src/valkey/js_valkey_functions.zig:852-867
Timestamp: 2025-09-05T19:49:26.188Z
Learning: In Bun’s Zig code, `.js_undefined` is a valid and preferred JSValue literal for “undefined” (e.g., resolving JSPromise). Do not refactor usages to `jsc.JSValue.jsUndefined()`, especially in src/valkey/js_valkey_functions.zig unsubscribe().

Applied to files:

• src/sql/postgres/PostgresSQLConnection.zig

📚 Learning: 2025-09-05T20:20:04.858Z

Learnt from: markovejnovic
Repo: oven-sh/bun PR: 21728
File: src/valkey/js_valkey.zig:163-178
Timestamp: 2025-09-05T20:20:04.858Z
Learning: Node-redis PUB/SUB callback error handling is broken - when a callback throws an error, it first emits the error as "Redis Client Error", then enters an undefined state with cascading internal errors like "undefined is not an object (evaluating 'this.#waitingForReply.head.value')", making the client unusable for further processing.

Applied to files:

• src/sql/postgres/PostgresSQLConnection.zig

📚 Learning: 2025-09-15T20:47:57.118Z

Learnt from: cirospaciari
Repo: oven-sh/bun PR: 22696
File: src/sql/mysql/js/JSMySQLConnection.zig:306-322
Timestamp: 2025-09-15T20:47:57.118Z
Learning: bun.Async.KeepAlive uses a simple state machine (active/inactive/done) rather than reference counting, with internal status field preventing issues from multiple ref/unref calls, making additional idempotence guards unnecessary.

Applied to files:

• src/sql/postgres/PostgresSQLConnection.zig

🧬 Code graph analysis (1)

test/js/sql/sql-listen.test.ts (2)

test/harness.ts (1)

• isDockerEnabled (865-888)

test/js/sql/adapter-env-var-precedence.test.ts (1)

• PostgresOrMySQLOptions (8-10)

🔇 Additional comments (14)

test/js/sql/sql-listen.test.ts (5)

33-57: LGTM! Well-structured test for basic notification delivery.

The test properly uses await using for SQL cleanup, Promise.withResolvers to wait for async notifications without arbitrary timeouts, and explicitly calls unlisten() for cleanup.

---

59-82: LGTM! Proper test for multiple notifications on same channel.

Correctly waits for all three notifications before asserting, avoiding race conditions.

---

84-112: LGTM! Excellent test for multi-channel isolation.

Properly verifies that each channel only receives its own notifications, not cross-channel leakage.

---

114-137: LGTM! Verifies connection health after unlisten.

The test confirms that unlisten() completes successfully and the connection remains functional for subsequent queries. This approach avoids flaky negative assertions about notifications not arriving.

---

139-161: LGTM! Tests the alternative unlisten API.

Properly verifies the sql.unlisten(channel) method works as an alternative to calling the returned unlisten function.

src/sql/postgres/PostgresSQLConnection.zig (4)

171-182: LGTM! Accessor pattern matches existing onConnect/onClose.

Correctly calls updateRef() when the notification callback is set to ensure the poll reference state reflects the presence of a listener.

---

504-526: LGTM! Correctly keeps connection alive when notification listener is registered.

The lifecycle logic properly unrefs the poll only when there are no queries, no pending writes, AND no notification listener, ensuring the connection stays alive to receive asynchronous NOTIFY messages.

---

1810-1827: LGTM! NotificationResponse handler correctly reports callback exceptions.

The implementation properly:

• Decodes the notification with cleanup
• Returns early if no listener is registered
• Converts PostgreSQL strings to JS values
• Reports user callback exceptions as unhandled without breaking the connection loop

---

1832-1841: LGTM! Poll reference correctly accounts for notification listeners.

The updateRef() logic properly keeps the poll active when a notification listener is registered, preventing premature connection closure while waiting for asynchronous NOTIFY messages.

src/js/bun/sql.ts (5)

453-459: LGTM! Non-enumerable property prevents accidental discovery.

Correctly exposes __pooledConnection as a non-enumerable, non-writable, non-configurable property for internal LISTEN/NOTIFY support while minimizing surface area for userland code.

---

934-950: LGTM! Cleanup properly releases listen connection before pool closure.

The implementation correctly clears the notification handler, releases the reserved connection, and resets state. Error suppression during cleanup is appropriate to ensure pool closure completes even if the listen connection is already disconnected.

---

962-1009: LGTM! Well-structured LISTEN/NOTIFY setup with proper error recovery.

The implementation correctly:

• Uses queueMicrotask to isolate callback execution from the notification dispatch path
• Wraps individual callbacks in try/catch to prevent one failing callback from affecting others
• Implements connection reservation with retry-on-failure semantics by resetting listenConnectionPromise in the catch block
• Properly configures the underlying connection's onnotification handler

---

1076-1108: LGTM! sql.unlisten() properly removes all channel listeners.

The implementation correctly removes all callbacks for the channel, sends UNLISTEN, and releases the connection when no listeners remain.

---

1189-1196: LGTM! Default SQL object properly delegates LISTEN/NOTIFY methods.

Follows the established pattern for lazy initialization and delegation.

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@flake.nix`:
- Around line 138-141: There are two duplicate attribute entries named
hardeningDisable in the flake.nix diff; remove one of them (or merge their lists
if they differ) so the attribute set has a single hardeningDisable definition —
locate the duplicate hardeningDisable entries and delete the redundant
definition (or combine values) to avoid the Nix evaluation error about duplicate
attributes.

[coderabbitai]

Actionable comments posted: 3

🤖 Fix all issues with AI agents

In `@src/js/bun/sql.ts`:
- Around line 1049-1077: The cleanup/release logic for the reserved listen
connection is duplicated in the returned unlisten function and in sql.unlisten;
extract that logic into a single helper (e.g., releaseListenConnectionIfUnused)
that inspects listeners, listenConnection and listenConnectionPromise, clears
pooledConn.connection.onnotification, calls listenConnection.release() inside
try/catch, and then nulls listenConnection and listenConnectionPromise; replace
the duplicated blocks in the anonymous unlisten closure and sql.unlisten with a
single await releaseListenConnectionIfUnused() call so both paths use the same
cleanup routine.
- Around line 967-981: The onNotification function currently wraps delivery in
queueMicrotask which adds an unnecessary microtask delay because the Zig-side
already invokes JS callbacks inside event_loop.enter()/exit(); update the
onNotification implementation: either remove the queueMicrotask wrapper and
execute the callbacks synchronously (preserving the try/catch and console.error
behavior) so notifications are delivered immediately, or if the wrapper is
required to prevent reentrancy when callbacks may issue SQL on the same
connection, keep it but add a clear comment above queueMicrotask explaining that
reentrancy protection is the reason for the deferral; locate onNotification and
the queueMicrotask call to make the change.
- Around line 984-1009: The listen connection has no recovery when the
underlying connection drops; in ensureListenConnection(), after obtaining
reserved (the reserved connection) and setting
pooledConn.connection.onnotification = onNotification, attach an
onclose/onend/close handler on the same pooledConn.connection (or reserved if it
exposes an event) that clears listenConnection and listenConnectionPromise (set
both to null) so future calls to ensureListenConnection() will re-reserve a
fresh connection; make sure the handler references the same symbols
(listenConnection, listenConnectionPromise, reserved, pooledConn,
onNotification) and is idempotent (no-op if already cleared).

[coderabbitai]

Actionable comments posted: 2

🤖 Fix all issues with AI agents

In `@src/js/bun/sql.ts`:
- Around line 1004-1010: The onClose handler currently clears listenConnection
and listenConnectionPromise but never re-subscribes the existing listeners, so
after a reconnect the channels stop delivering notifications; update
ensureListenConnection (or the reconnection path invoked from
pooledConn.onClose) to, after establishing a new pooledConn and assigning
listenConnection/listenConnectionPromise, iterate the listeners map (the same
map used by listen()) and re-issue SQL "LISTEN <channel>" for each channel (and
reattach their callbacks to the new connection), handling and logging
per-channel errors and ensuring the operation is idempotent if concurrent
ensureListenConnection calls run.
- Around line 1045-1051: The code around the new-channel LISTEN uses an
unnecessary try/catch that only rethrows; remove the try/catch and simply await
conn.unsafe(`LISTEN ${pool.escapeIdentifier(channel)}`) when isNewChannel is
true so failures propagate naturally (look for isNewChannel, conn.unsafe and
pool.escapeIdentifier(channel) in the snippet to update).

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@src/js/bun/sql.ts`:
- Around line 1054-1068: The concurrent-listen race is caused by creating and
setting a new Set before re-checking the map after the await; in the listen
logic, after awaiting conn.unsafe(`LISTEN ${pool.escapeIdentifier(channel)}`)
re-read listeners.get(channel) and use that existing Set if present (instead of
unconditionally creating and overwriting one), otherwise create a new Set and
set it; then add the callback to the resolved Set (references: listen flow that
uses listeners, callbacks, conn.unsafe, pool.escapeIdentifier).

[structwafel]

@coderabbitai review

[coderabbitai]

Only users with a collaborator, contributor, member, or owner role can interact with CodeRabbit.

[robobun]

Thanks for taking this on, @structwafel, this was the first PR for #18214. Since it was opened, the native side it changes (src/sql/postgres/*.zig, sql.classes.ts under src/bun.js) has been ported to Rust and those files no longer exist on main, so this branch cannot be rebased as is.

There are three open implementations of LISTEN/NOTIFY, and we are consolidating on #32089, which targets the current code and has had maintainer review on the API shape: listen() resolves to a subscription object with unlisten() (also usable with await using), there is a notify() method, subscriptions share one dedicated auto-reconnecting connection instead of a reserved pool connection, and callback exceptions are reported instead of logged. It also covers the cases exercised by the tests here (delivery, multiple channels, unlisten stopping delivery, argument validation). The bare unlisten function and sql.unlisten(channel) from this PR are the two parts of the shape the review on #32089 decided against, so there was nothing further to carry over.

Closing this one in favor of #32089.