Fix database lease ownership, transaction cleanup and read extensions

[binaryfire]

This fixes the database lifecycle issues raised in the review of #653 and removes unused stream-ID plumbing from response cancellation. Ordinary queries still benefit from early connection release and reuse. The changes close gaps in ownership, transaction cleanup, and read-driver extensions.

End logical connection ownership

A retained connection or query builder could borrow another pool slot after its owning coroutine had finished. The cleanup registered for that owner had already run, so nothing returned the new slot to the pool.

Logical leases now end when their coroutine or task releases or discards them. A connection whose initial setup fails is also ended, so a listener retaining it cannot later borrow a slot without registered cleanup. Attempting to borrow through an ended lease throws an exception explaining where to resolve the connection. This check runs only before a new borrow. Early release, reconnect, and recovery from a failed reacquisition still work within the owning execution. Release and rollback callbacks can still use the session while cleanup owns it.

Keep raw transactions on their sessions

Transactions started through PDO or SQL don't update the framework's transaction counter. Early release previously treated those sessions as idle, allowing another request to use a transaction it didn't start.

Pinning now checks the physical transaction state of both open PDO handles. This covers native and SQL-started transactions on the writer or reader without opening an unused handle. For example, the transaction below retains its session across the HTTP call:

$connection = DB::connection();
$pdo = $connection->getPdo();
$pdo->beginTransaction();

try {
    Http::post($url, $payload);
    $pdo->commit();
} catch (Throwable $exception) {
    $pdo->rollBack();
    throw $exception;
}

The public inTransaction() method keeps its existing writer-only behavior. Code using temporary tables, session locks, or other session-dependent operations still uses withPinnedSession() as documented.

Settle unfinished transactions before reuse

Pool release now checks physical transaction state after callbacks finish and the logical lease detaches. This also catches a raw transaction opened by a release callback. If a transaction remains, or its state can't be inspected, the slot is discarded immediately so it doesn't sit idle holding transaction locks.

The discard decision is made before logging, so a failing logger can't return a dirty session to the pool. Cleanup still settles the slot when cancellation occurs. Other preparation failures retain their existing reconnect-before-reuse behavior when no transaction remains.

PDO disconnect now cleans both distinct handles and attempts reader cleanup even if writer cleanup fails. Cancellation retains precedence over ordinary errors. Shared in-memory SQLite keeps its committed data and schema after successful rollback and replacement of the pooled wrapper.

Honor effective read-driver extensions

An explicit ::read pool could select a DB::extend driver from a read record, then incorrectly try to rebuild its result as a PDO lease. That bypassed the extension and could fail with an unsupported-driver error or a type error.

Lease eligibility now checks every effective read record, including a single record and a list of records. These extensions retain their complete connection object. Named and top-level extension precedence, write-side construction, and database identity checks remain unchanged. Classification happens once when the pool is created.

Remove unused response stream IDs

Response cancellation tracks active producers by connection and coroutine. The stored stream ID wasn't used. This removes it and its argument forwarding from the response bridge, HTTP, gRPC, WebSocket, and test-fixture paths.

Connection-close cancellation still stops all opted-in producers on that connection, preserves other connections, and removes registrations after production. The existing HTTP/2 follow-up now states that stream identity will be added when a released Swoole stream-cancel event needs it.

Verification

The database regressions fail against the original source and pass with these changes. Coverage includes expired owners, raw transactions, read extensions, callback failures, cancellation, and cleanup of both PDO handles. Existing response tests verify that removing stream IDs preserves cancellation behavior.

Formatting, PHPStan, the full components suite, Testbench, and package-consumer checks passed. The MySQL, MariaDB, and PostgreSQL integration runs also passed, including their package-specific suites.

There are no new ownership checks on ordinary query execution or new database round trips for transaction inspection. The additional checks run at pool creation, acquisition, and release boundaries.

Summary by CodeRabbit

• Bug Fixes
  • Prevented pooled database connections from being reused while physical transactions remain open, including transactions started through PDO or SQL.
  • Improved cleanup of unfinished transactions across read and write connections, while preserving shared in-memory SQLite connections.
  • Prevented ended or discarded connection leases from borrowing another session.
  • Kept disconnect cancellation scoped to the affected connection.

Note

Fix database lease ownership, transaction cleanup, and read extensions

• Adds an ended-state flag to ConnectionLease in ConnectionLease.php. Early idle release keeps the lease usable within the same execution, while explicit release, discard, and resolvePdo after ownership ends now reject borrowing another pooled session.
• PooledConnection::release now inspects physical transaction state and discards the connection when a raw PDO transaction is still open or inspection fails, instead of returning it to the pool for reuse. PdoConnection::disconnectDriverResources rolls back open transactions on both the write and read PDO handles, avoiding duplicate work when they are shared.
• Connection::hasPinnedSession now counts active physical driver transactions, so raw transactions keep their session out of the idle pool.
• DatabasePool::supportsSessionLeases is extension-aware: pools disable session leases when a PDO extension is selected by the base config, a read record, or a top-level driver, and single read endpoint records are now compared like lists.
• ConnectionResolver::connection discards an already-created ConnectionLease owner when setup fails, instead of only the initial pooled connection.
• Removes the stream identifier argument from ResponseBridge::send and ResponseCancellation registration; callers in the HTTP, gRPC, and WebSocket servers are updated.
• Behavioral Change: a lease whose ownership ended now throws a LogicException when resolving a new PDO session; connections with leftover raw transactions are discarded rather than reused; ResponseBridge::send signature changed for all callers.

^{Macroscope summarized a3d1479.}

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: hypervel/components/.coderabbit.yaml
• Review profile: CHILL
• Plan: Advanced
• Run ID: 5c3c5a62-6d98-4df4-926a-a4a6c4f83b85

📥 Commits

Reviewing files that changed from the base of the PR and between bf1e605 and ee282f7.

📒 Files selected for processing (19)

• docs/plans/2026-10-08-1451-framework-lease-cleanup.md
• docs/todo.md
• src/database/src/Connection.php
• src/database/src/PdoConnection.php
• src/database/src/Pool/ConnectionLease.php
• src/database/src/Pool/DatabasePool.php
• src/database/src/Pool/PooledConnection.php
• src/docs/database.md
• src/grpc/src/Server/Server.php
• src/http-server/src/ResponseBridge.php
• src/http-server/src/Server.php
• src/server/src/ResponseCancellation.php
• src/websocket-server/src/Server.php
• tests/Database/DatabaseConnectionLeaseLifecycleTest.php
• tests/Database/DatabaseConnectionLeaseTest.php
• tests/Database/DatabasePdoConnectionTest.php
• tests/HttpServer/Fixtures/disconnect-server.php
• tests/HttpServer/ResponseBridgeTest.php
• tests/Integration/Database/PooledConnectionTest.php

💤 Files with no reviewable changes (1)

• src/http-server/src/Server.php

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

The changes update database lease ownership and physical transaction cleanup, including read-connection handling. They also remove stream identifiers from response cancellation registration and its callers.

Changes

Database Lease and Transaction Lifecycle

Layer / File(s)	Summary
Lease ownership and eligibility src/database/src/Pool/ConnectionLease.php, src/database/src/Pool/DatabasePool.php, tests/Database/DatabaseConnectionLeaseLifecycleTest.php, tests/Database/DatabaseConnectionLeaseTest.php, src/docs/database.md, docs/plans/2026-10-08-1451-framework-lease-cleanup.md	Explicit lease release and discard end logical ownership. A later borrow attempt fails with a LogicException. Pool lease eligibility now accounts for extensions in effective read configurations. Tests and documentation cover these behaviors.
Physical transaction detection and cleanup src/database/src/Connection.php, src/database/src/PdoConnection.php, src/database/src/Pool/PooledConnection.php, tests/Database/DatabasePdoConnectionTest.php, tests/Integration/Database/PooledConnectionTest.php, tests/Database/DatabaseConnectionLeaseLifecycleTest.php, tests/Database/DatabaseConnectionLeaseTest.php, src/docs/database.md, docs/plans/2026-10-08-1451-framework-lease-cleanup.md	Connections detect physical transactions during pinning and release. Release discards connections when a transaction remains or transaction inspection fails. PDO disconnect cleanup checks distinct read and write handles and rolls back active transactions.

Response Cancellation Registration

Layer / File(s)	Summary
Remove stream identity from cancellation calls src/http-server/src/ResponseBridge.php, src/server/src/ResponseCancellation.php, src/http-server/src/Server.php, src/grpc/src/Server/Server.php, src/websocket-server/src/Server.php, tests/HttpServer/ResponseBridgeTest.php, tests/HttpServer/Fixtures/disconnect-server.php, docs/todo.md, docs/plans/2026-10-08-1451-framework-lease-cleanup.md	ResponseBridge::send() and cancellation registration no longer accept or store a stream identifier. HTTP, gRPC, and WebSocket callers omit it. Tests continue to cover cancellation behavior by connection.

Priority: ➖ Normal

Estimated code review effort: 4 (Complex) | ~40 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant PooledConnection
  participant PdoConnection
  participant DatabasePool
  participant PDO
  PooledConnection->>PdoConnection: Check physical transaction state
  PooledConnection->>DatabasePool: Discard connection when active or inspection fails
  DatabasePool->>PdoConnection: Disconnect driver resources
  PdoConnection->>PDO: Roll back active transaction

Loading

Merge Risk: ⚪ Minimal · up to ee282

No identified issue needs resolution before merge; the transaction check does not add per-query work.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 52.83% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 53 functions across 15 files. (3 skipped:…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
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.
Title check	✅ Passed	The title clearly and concisely summarizes the primary database lifecycle changes, including lease ownership, transaction cleanup, and read extensions.
Description check	✅ Passed	The description is mostly complete and directly addresses the problem, implementation, supporting regression coverage, verification results, and scope. It does not explicitly select a contribution typ…

Full details: Docstring Coverage

Explanation

Docstring coverage is 52.83% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 53 functions across 15 files. (3 skipped: 3 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

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.}

[binaryfire]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[binaryfire]

@cubic-dev-ai review

[cubic-dev-ai]

@cubic-dev-ai review

@binaryfire cubic can't start this review because your workspace has reached its free monthly review limit. cubic has reviewed 124,545 of the 120,000 allowed lines of code this month. Reviews resume on 10 October 2026 (in 2 days). Paid plans include much higher monthly review limits. Upgrade now to resume reviews.

To help optimise your usage, you can tune cubic to get the most out of your usage limits:

• Stop reviewing test files
• Turn off incremental reviews
• Use the cubic CLI locally
• Upgrade to a pro plan

Learn more →

[qodo-free-for-open-source-projects]

PR Summary by Qodo

Fix database lease ownership, transaction cleanup, and read extensions

🐞 Bug fix ✨ Enhancement 🧪 Tests 📝 Documentation 🕐 40+ Minutes

AI Description

• End logical leases with their owners so retained connections cannot borrow unowned pool slots.
• Pin raw transactions and discard unfinished sessions before another borrower can use them.
• Honor effective read-driver extensions and remove unused response stream-ID plumbing.

Diagram

graph TD
  Resolver["Connection resolver"] --> Lease["Logical lease"] --> Pool["Database pool"] --> Holder["Pooled connection"] --> PDO["PDO handles"]
  Factory["Connection factory"] --> Pool
  Bridge["Response bridge"] --> Registry["Cancellation registry"]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Transfer retained connections to new owners

• ➕ Could allow builders retained across execution boundaries to keep working.
• ➖ Requires transferring connection state and cleanup responsibility safely.
• ➖ Introduces ambiguity if executions use the same retained connection concurrently.

Recommendation: Keep the PR’s terminal-lease boundary. It prevents unowned borrows without changing ordinary early release or introducing automatic ownership transfer. Physical transaction checks at release boundaries likewise address dirty-session reuse without per-query checks.

Files changed (19) +590 / -82

Bug fix (5) +97 / -22

Connection.phpInclude physical transactions in session pinning +12/-1

Include physical transactions in session pinning

• Adds an internal physical-transaction check to session pinning, with a default implementation based on the connection’s existing transaction API.

src/database/src/Connection.php

PdoConnection.phpInspect and clean both open PDO handles +33/-8

Inspect and clean both open PDO handles

• Detects transactions on open writer and distinct reader handles without opening lazy handles. Disconnect attempts cleanup on both, preserves cancellation precedence, and clears driver references afterward.

src/database/src/PdoConnection.php

ConnectionLease.phpPrevent borrowing after logical ownership ends +13/-4

Prevent borrowing after logical ownership ends

• Makes terminal release and discard end the lease and rejects a subsequent borrow with an actionable exception. Early release and failed-acquisition recovery remain non-terminal.

src/database/src/Pool/ConnectionLease.php

DatabasePool.phpClassify effective read extensions before enabling leases +15/-6

Classify effective read extensions before enabling leases

• Checks single and listed effective read records for extensions, so explicit read pools retain whole-connection ownership when needed. Existing endpoint identity checks remain part of pool-creation classification.

src/database/src/Pool/DatabasePool.php

PooledConnection.phpDiscard sessions with unfinished or uninspectable transactions +24/-3

Discard sessions with unfinished or uninspectable transactions

• Inspects physical transaction state after release callbacks and lease detachment. Dirty or uninspectable slots are marked for discard before logging, while clean slots retain the existing release path.

src/database/src/Pool/PooledConnection.php

Refactor (5) +6 / -9

Server.phpStop forwarding stream IDs to gRPC response emission +2/-2

Stop forwarding stream IDs to gRPC response emission

• Removes the unused stream-ID argument from both normal and error response bridge calls.

src/grpc/src/Server/Server.php

ResponseBridge.phpRemove stream IDs from response bridge registration +1/-2

Remove stream IDs from response bridge registration

• Drops the unused send argument and registers opted-in response producers using their connection alone.

src/http-server/src/ResponseBridge.php

Server.phpStop forwarding HTTP request stream IDs +0/-1

Stop forwarding HTTP request stream IDs

• Removes the unused stream-ID argument from HTTP response emission.

src/http-server/src/Server.php

ResponseCancellation.phpRemove unused stream identity from cancellation records +2/-3

Remove unused stream identity from cancellation records

• Drops the stored stream ID and register argument. Producer tracking and cancellation remain keyed by connection and coroutine.

src/server/src/ResponseCancellation.php

Server.phpStop forwarding WebSocket handshake stream IDs +1/-1

Stop forwarding WebSocket handshake stream IDs

• Removes the unused stream-ID argument from handshake response emission.

src/websocket-server/src/Server.php

Tests (6) +340 / -45

DatabaseConnectionLeaseLifecycleTest.phpCover ended owners and terminal raw-transaction cleanup +82/-2

Cover ended owners and terminal raw-transaction cleanup

• Verifies retained builders cannot borrow after coroutine or task cleanup, including discard. Adds raw-transaction cleanup cases for application code and release listeners while checking shared SQLite data survives.

tests/Database/DatabaseConnectionLeaseLifecycleTest.php

DatabaseConnectionLeaseTest.phpTest raw-transaction pinning and read extensions +69/-21

Test raw-transaction pinning and read extensions

• Extends early-release coverage to native, SQL-started, and reader transactions across manual and HTTP-triggered release. Checks that named and effective read-record extensions retain their connection objects.

tests/Database/DatabaseConnectionLeaseTest.php

DatabasePdoConnectionTest.phpTest disconnect cleanup across PDO handle arrangements +69/-6

Test disconnect cleanup across PDO handle arrangements

• Covers distinct, shared, and lazy reader handles, plus continued reader cleanup after a writer failure. Verifies cancellation and existing disconnect failure precedence.

tests/Database/DatabasePdoConnectionTest.php

disconnect-server.phpAlign disconnect fixture with the response bridge API +1/-1

Align disconnect fixture with the response bridge API

• Removes the obsolete stream-ID argument from fixture response emission.

tests/HttpServer/Fixtures/disconnect-server.php

ResponseBridgeTest.phpVerify connection-close cancellation without stream IDs +11/-12

Verify connection-close cancellation without stream IDs

• Replaces stream-ID assertions with producer identities and removes now-duplicate cases. Retains checks for opt-in cancellation, multiple producers, and an unaffected sibling connection.

tests/HttpServer/ResponseBridgeTest.php

PooledConnectionTest.phpCover dirty-slot discard and release failure ordering +108/-3

Cover dirty-slot discard and release failure ordering

• Adds raw-transaction release coverage and verifies discard despite callback cancellation or logging failure. Tests that failed physical-transaction inspection also discards the slot and propagates its failure.

tests/Integration/Database/PooledConnectionTest.php

Documentation (3) +147 / -6

2026-10-08-1451-framework-lease-cleanup.mdRecord the lease-cleanup plan and verification scope +141/-0

Record the lease-cleanup plan and verification scope

• Documents the reported lifecycle defects, intended ownership and transaction boundaries, test cases, and verification workflow. It also records the limited scope of the stream-ID removal.

docs/plans/2026-10-08-1451-framework-lease-cleanup.md

todo.mdDefer stream identity to future HTTP/2 cancellation support +1/-1

Defer stream identity to future HTTP/2 cancellation support

• Clarifies that response registrations should gain stream identity when a supported Swoole stream-cancel event is integrated.

docs/todo.md

database.mdExplain lease ownership and raw-transaction behavior +5/-5

Explain lease ownership and raw-transaction behavior

• Documents terminal ownership, automatic pinning and cleanup of raw transactions, and read-record extension handling. Retains manual pinning guidance for other session-dependent operations.

src/docs/database.md

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Tip of the day

💡 Did you know, you can keep summaries lean with Findings visible per group, which tucks the rest behind a View link

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗