Make Guzzle coroutine-safe and reduce AOP overhead by binaryfire · Pull Request #60 · hypervel/components-backup

binaryfire · 2026-10-09T14:54:43Z

Guzzle's process-wide promise queue can run one request's callbacks while another request drains it. In a long-lived coroutine worker, those callbacks can resolve the wrong request or tenant, or use a database transaction owned by another coroutine. Sharing an active cURL multi-handler has a second failure mode: one coroutine can drive another's native transfers and crash the worker.

This PR makes those operations stay with their owning coroutine, fixes the affected SDK consumers, and reduces the AOP cost of enforcing that rule. The same AOP improvements benefit existing instrumentation such as Sentry and Telescope.

Keep Guzzle callbacks and transfers in their owning coroutine

A mutable promise belongs to the native coroutine that creates it. Pending work must be registered, driven, settled and canceled there. Completed results remain reusable across coroutines, and an idle multi-handler can be reused sequentially. A promise that has adopted another pending promise is still unfinished work.

Each coroutine receives a normal Guzzle task queue through the supported Utils::queue() setter. Pending promise operations and active multi-handlers are checked before another coroutine can run their callbacks or change their state. A conflicting operation fails with an explanation of how to create and finish the work in the same coroutine. Applications retain concurrency through native coroutines and parallel(); they share completed results rather than handing pending operations between requests.

Transfer ownership is reserved before request preparation, because reading an upload body can yield. Active transfers remain alive until settlement or cancellation. At coroutine exit, unfinished transfers are canceled and the owner's queue drains their rejection handlers, allowing dependent promises and memoized providers to recover. Queue and transfer state is excluded from copied coroutine context. Outside coroutines, normal Guzzle queue and shutdown behavior is retained.

Registration belongs to the HTTP provider, so protection applies in HTTP requests, queue workers and console commands without requiring an observability package. It covers Guzzle's mutable promises and cURL multi-handler; arbitrary third-party promise implementations and custom transports do not acquire these guarantees automatically.

Why AOP is needed

Guzzle exposes a queue replacement hook, but no global hook for promise construction, pending promise operations, or multi-handler admission and driving. An application client subclass or outer middleware cannot intercept every promise created inside Guzzle or an SDK.

The aspects therefore target a small set of public methods: the mutable promise constructor, then, wait, cancel, resolve and reject; and the multi-handler's __invoke, tick, execute and close where available. They check ownership around the original methods without copying their implementations or replacing Guzzle's scheduling.

Proxy generation runs before provider boot. It rejects vendor classes loaded too early and loaded proxies missing required method interception, rather than silently leaving protection inactive. The error explains lazy client registration. Generated method metadata verifies coverage during bootstrap, without adding that inspection to each invocation.

Correct shared SDK and HTTP consumers

AWS credentials. SQS, S3 and SES clients can share a callable credential provider even when the clients themselves are pooled separately. The provider can retain an in-flight HTTP promise. A shared wrapper serializes provider invocation and its promise completion by callable identity, while each caller executes in its own coroutine and receives its own result or exception. Callbacks drained during that wait may request credentials again in the same coroutine without deadlocking; other coroutines still wait for the outermost call to finish. Existing SDK memoization, pool fingerprints, completed credential reuse and cancellation behavior are preserved. There is no additional credential cache or sharing of one caller's credentials with another.

Pusher and Reverb. The default client previously shared an async multi-handler between concurrent callers. Async handlers are now reused within the same client and coroutine, preserving same-coroutine fan-out and connection reuse. Weak client keys and coroutine-local storage release handlers when they are no longer needed. Synchronous transport reuse, middleware, timeouts, on_stats, custom handlers and supported transport options remain intact. Connection caps that require sharing one active multi-handler across coroutines are rejected; callers should bound coroutine concurrency or use rate limiting.

Named HTTP connections. Async requests previously lost the named connection's handler settings when creating a fresh handler. Factory::newConnectionHandler() now shares option derivation with the cached synchronous path. Async handlers remain isolated while retaining their registered transport settings. Per-call request presets and explicit handler/client overrides keep their existing meaning.

Reduce shared AOP work

The previous dispatcher rebuilt a generic pipeline for every intercepted call and reflected its closure to find the target instance. It now caches an immutable chain per class and method, containing only aspect class names and continuation closures. Each invocation still resolves its aspect from the current container. Generated proxies pass the intercepted instance directly, or null for static calls.

This preserves priority, short-circuiting, original-method bypass, exceptions, references, variadics, scoped aspects, container rebinding, and nested or concurrent calls. No target object, join point, container or aspect instance is retained in the chain. The unused AOP-specific pipeline and its dependency are removed; the general framework pipeline is unchanged.

Bootstrap now selects exact class rules directly, prepares wildcard matching once per rule, and creates the parser and printer only when generation is required. A separate authoritative Composer loader contains only proxy entries, keeping the application's source map unchanged. This avoids retaining copies of the complete application map. Composer discovery continues to identify the application loader, including after test resets. Content fingerprints, PSR-4 lookup, source overrides, proxy regeneration and validation before publication remain intact.

Make class-map overrides safe across application boots

The documented provider classMap() API previously copied the entire Composer application map when adding replacements. It also rejected the same replacement on a second Testbench boot, while test cleanup left unloaded overrides active for later applications that had not registered them.

Replacements now use their own authoritative Composer loader and leave the application map unchanged. Automatic test cleanup unregisters that loader and its generated replacement proxies together. A loaded target can be registered again only from the same canonical source; generated proxies record their original source as class metadata so the same rule applies to them. Different-source replacements still fail. Replacement proxies use separate, fixed filenames so they cannot overwrite ordinary proxies retained between application boots.

Generated proxies now live in bootstrap/cache/aop, alongside each release's other compiled framework files. Two overlapping releases sharing storage previously wrote the same proxy paths, allowing an older worker to load the newer release's code. The release-local directory prevents that collision without per-release hashes or accumulating files in shared storage. Cache clearing, Testbench cleanup and the about report follow the new location; about also recognizes the generated PHP files correctly.

Approaches considered and rejected

Approach	Why it was not used
Replace only Guzzle's queue	Necessary, but insufficient: another coroutine could still operate directly on a pending promise or active multi-handler.
Client subclasses or outer middleware alone	Do not cover promises and callbacks constructed internally by Guzzle and third-party SDKs.
Copy request context into another coroutine	Copying values does not transfer native resource or database transaction ownership.
Replace the scheduler or make requests pump each other's queues	Adds scheduling machinery and changes wait/progress behavior. Keeping work in its native owner preserves the relevant resource boundary.
Maintain a Guzzle fork, vendor patch, or copied promise implementation	Duplicates dependency maintenance. Supported queue replacement plus narrow public-method interception provides the required coverage.
Give every Pusher transfer its own handler	Loses supported same-coroutine fan-out and connection reuse. Per-client, per-coroutine reuse preserves both.
Cache resolved aspects or coroutine state globally	Would break scoped bindings, rebinding, or request isolation. Only immutable dispatch metadata is cached.
Publish proxies into the main Composer map	Retains another complete class-map allocation. Composer's own separate authoritative loader holds just the proxy entries.

The pass-through-aspect variant used below is a measurement control, not a production bypass. All measured outgoing request paths remain supported; the rejected items above are implementation approaches.

AOP performance improvements

These comparisons are against the initial ownership implementation, not against an uninstrumented framework:

Measurement	Initial implementation	Optimized	Change
Guarded promise-chain CPU	39.52 µs/op	20.57 µs/op	About 48% lower
Cached startup	181.16 ms	78.15 ms	About 57% lower
Retained PHP heap after idle workload	6,114.61 KiB	3,164.52 KiB	2.88 MiB less

The compiled-chain comparison reduced pass-through dispatch from 31.62 to 15.80 µs/op. A separate comparison isolated direct instance passing, reducing guarded chain CPU from 21.72 to 20.48 µs/op. The earlier complete matrix measured 20.50 µs/op; the final bootstrap refresh measured 20.57 with the same dispatch code.

Ownership overhead across outgoing paths

The following compares the unchanged framework with the protected implementation. Values are client CPU microseconds per operation, not batch latency. The loopback origin's CPU is excluded.

Workload	Concurrency	Baseline → protected	Added CPU
Idle loop †	1	0.33 → 0.34	0.01, rounding-sensitive
Promise chain †	1	3.23 → 20.57	17.34
Synchronous HTTP †	1	405.37 → 444.67	39.30
Async HTTP	1	289.60 → 377.82	88.22
Async HTTP	8	200.12 → 259.75	59.63
Async HTTP †	32	152.42 → 210.28	57.86
Coroutine-parallel HTTP	1	466.42 → 519.82	53.40
Coroutine-parallel HTTP	8	422.48 → 482.29	59.80
Coroutine-parallel HTTP †	32	430.56 → 468.59	38.03
Response middleware/event	1	301.30 → 339.90	38.61
Credential provider	1	371.19 → 419.12	47.94
Shared credential provider	8	294.69 → 409.29	114.60
Pusher	1	390.30 → 500.75	110.45
Pusher	8	299.13 → 374.29	75.16
Pusher †	32	267.77 → 330.34	62.57
Cancel pending transfer †	1	108.96 → 165.89	56.93

† Refreshed paired measurements after the bootstrap corrections. Other rows use the earlier complete matrix with the same dispatch and ownership implementation. Rows from different runs are not a controlled concurrency-scaling comparison.

Async completion through Utils::unwrap() makes 27 intercepted calls per operation at concurrency one, compared with nine in the promise probe. A separate pass-through comparison separates dispatch from ownership and queue handling; it explains the larger async and cancellation costs. There were no GC runs inside the timed operation loops.

The shared credential-provider workload deliberately serializes calls to one provider and includes a 1 ms endpoint delay. At concurrency eight, p95 latency increased from 5.396 to 22.474 ms. This includes waiting for the provider lock; it is not all AOP overhead. Completed memoized credentials remain reusable.

Guzzle 7 shows comparable absolute costs on the focused workloads: approximately 17 µs for promise chains, 43 µs for synchronous HTTP, 90 µs for async concurrency one, 53 µs for async concurrency eight, and 54 µs for cancellation. The source implementation does not need a separate version-specific AOP path.

Memory bounds and capacity planning

At the benchmarked revision, the integration retains approximately 124–188 KiB of additional PHP heap per warmed worker across the measured workloads, with roughly 0.94–1.19 MiB additional RSS. RSS includes shared code pages, so it is not a measure of private memory added to every worker. Allocated PHP memory after exit remained 16 → 16 MiB.

This retained metadata is not added for each request. Temporary ownership state follows outstanding promises and native transfers and is released as work finishes or is canceled. Repeated batches showed the same report-bookkeeping growth on baseline and protected processes, with no additional retained growth, and returned to the same descriptor count. Applications still need bounded outgoing concurrency and queues; the integration does not cap how much unfinished work an application can create.

At the measured 38.03 µs additional CPU per outgoing request for coroutine-parallel HTTP at concurrency 32:

Outgoing requests/sec	Additional CPU capacity across the deployment
1,000	0.038 cores
10,000	0.380 cores
50,000	1.902 cores

These are arithmetic projections, not throughput measurements at those traffic levels. The tested HTTP and Pusher paths suggest a planning range of approximately 40–110 µs/request, or 0.4–1.1 additional cores at 10,000 outgoing requests/sec. Credential-provider contention is a separate serialized workload. The integration adds predictable CPU work without a request-count-dependent retained-memory cost.

Cached startup compared with the unchanged framework is 69.95 → 78.15 ms. One cold process requiring proxy generation took 132.45 ms; cold startup is reported separately from repeated warm measurements.

Measurement conditions and validation

Measurements precede the subsequent class-map override reset and release-local proxy corrections, and the switch to xxh128 for internal proxy fingerprints and helper names. These add generated source metadata and bootstrap lookup for registered overrides, without changing the per-request dispatcher or ownership checks. Their startup and retained-memory effects have not been remeasured; the resource figures describe the benchmarked revision.

The credential-provider measurements also precede the same-coroutine reentry correction. Its active-owner lookup and cleanup have not been remeasured. This correction does not change the HTTP, Pusher or shared AOP dispatch paths.

Measurements used PHP 8.4.25, Swoole 6.2.2, cURL 8.5.0, CLI OPcache enabled, JIT disabled, and an Intel Core i7-7700 with six logical processors exposed. Both checkouts used independently installed identical dependencies and optimized Composer autoloading. The primary matrix used Guzzle 8.2.0/promises 3.0.2; focused comparisons also used Guzzle 7.15.5/promises 2.5.3. Each workload used three alternating process pairs with warmup and three measured samples per process. The report includes ranges, operation counts, latency and resource snapshots.

Async/Pusher wall-time measurements at higher concurrency include a separate native hooked-select readiness stall with an upstream fix. CPU is reported separately rather than using that stall to make ownership overhead appear negligible. The default AWS Guzzle transport also takes the async transport path for synchronous SDK calls.

Regression coverage exercises real generated proxies, yielding request/tenant and database-transaction isolation, pending and adopted promises, cancellation, owner exit, upload preparation, shared clients, credential memoization, Pusher fan-out, cold/cached bootstrap and cleanup. Representative tests fail with the protections removed. Both supported Guzzle dependency families and their upstream promise suites were validated, together with the full framework suite, affected SDK consumers, Sentry/Telescope, package-mode Testbench, formatting and static analysis. CI adds a focused Guzzle 7 compatibility job alongside the normal dependency resolution.

The shared PHPUnit extension prepares framework proxy targets before test discovery without enabling optional instrumentation and resets test-owned queues safely. The formatter's return_assignment rule is also disabled because it removes assignments captured by reference in nested closures, breaking valid Guzzle wait callbacks.

Validation also exposed test-harness gaps. Coroutine tests now retain PHPUnit's native busy-loop alarm and enforce the same deadline while the test or its children are suspended, reporting the named test through PHPUnit's normal timeout result. Synchronous output-assertion tests run outside coroutines pending the upstream output-buffer integration. PHP-clock rate-limiter contract tests freeze their clock so I/O delays cannot change back-to-back decisions.

Notification transport snapshots also use SplObjectStorage::offsetExists() instead of the PHP 8.5-deprecated contains(), retaining PHP 8.4 compatibility.

The technical design is in docs/plans/2026-10-09-guzzle-coroutine-ownership.md. Reproducible commands and complete measurements are in tests/Benchmarks/GuzzleOwnership/README.md and results.md.

Note

Make Guzzle coroutine-safe and reduce AOP dispatch overhead

Adds coroutine ownership tracking for Guzzle promises and cURL multi-handlers. Pending promises and active transfers belong to their creating coroutine; foreign operations raise CoroutineOwnershipException (CoroutineOwnership.php, Aspects/PromiseOperationAspect.php).
Adds CoroutineTaskQueue, CoroutineState, and TransportState so each coroutine has its own Guzzle task queue and pending transfers. Coroutine exit cancels unfinished transfers and drains that coroutine's callbacks (CoroutineTaskQueue.php).
Rewrites AOP dispatch: AspectManager caches compiled closure chains instead of aspect-name arrays, ProxyDispatcher invokes cached chains with the intercepted instance attached to the join point (AspectManager.php, ProxyDispatcher.php).
Wraps callable AWS credential providers in SerializedCredentialProvider for SQS, S3, SES mail, and filesystem clients, so shared providers serialize their work per caller. Adds shared Pusher/Reverb transports for synchronous calls and coroutine-local handlers for asynchronous calls (BroadcastManager.php).
Moves the AOP proxy cache from storage/framework/aop to bootstrap/cache/aop, switches generated fingerprints and helper names to xxh128, and adds ProxySource/ProxyMethod attributes to generated proxies. Adds a CI job for Guzzle 7 compatibility.
Behavioral Change: Pusher/Reverb clients without an explicit handler now throw InvalidArgumentException for host/total connection-limit options; ProceedingJoinPoint::getInstance now reports the dispatcher-supplied instance (null for static calls) instead of reflecting the closure; cached AOP entries are closures, so custom AspectManager consumers must be updated; existing proxy caches under storage/framework/aop are stale and regenerate under the new path.

^{Macroscope summarized 1e7f66f.}

The formatter removes assignments captured by reference in nested closures, breaking Guzzle wait callbacks. Explicitly disable return_assignment rather than changing valid code to avoid the rewrite. Verified with the failing formatter reproduction, a clean formatting run and the credential-provider regression tests.

SQS, S3 and SES clients can share a custom credential provider across pools. Serialize provider invocation and promise completion using its callable identity so concurrent callers cannot drive each other’s pending credential work or receive another caller’s credentials. Preserve SDK memoization, original pool identity and cancellation cleanup without adding a credential cache. Cover actual pooled manager paths, caller-specific request signing, callable aliases, expiring credential reuse, failure and cancellation. Document safe memoization for providers whose callers share credentials. Verified focused regressions, Queue, Filesystem and Mail suites, full static analysis and formatting.

Pusher and Reverb shared one Guzzle multi-handler across concurrent callers, allowing one coroutine to drive another’s native transfers. Reuse async handlers only within the same client and coroutine, using weak client keys and non-copyable context. Preserve same-coroutine promise fan-out and connection reuse alongside the existing synchronous transport. Keep custom-handler validation and transport options intact. Reject connection caps that require an unsafe cross-coroutine shared handler and document bounded concurrency alternatives. Regression tests reproduce the native failure, preserve overlapping requests, and verify copied-context isolation, option forwarding and connection reuse. Broadcasting tests, static analysis and formatting pass.

Async requests created fresh handlers without the named connection's transport settings. Add an uncached handler factory that shares option derivation with the synchronous path, preserving transport sharing and multiplexing policy while keeping each async handler isolated. Cover sync and async option forwarding, request-preset replacement, and handler reuse. Connection tests and affected consumers pass on Guzzle 7 and 8; static analysis and formatting pass.

Guzzle's global task queue can run one request's callbacks while another request drains it. Shared multi-handlers can also drive foreign transfers. Install coroutine-local stock queues and guard pending promise operations and active transports at their public method boundaries. Guzzle has no global hooks for these boundaries, so use narrowly targeted AOP interception. Retain active native transfers until settlement and propagate cancellation through the owner's queue at exit. Completed results and idle handlers remain reusable. Reject classes loaded before proxy generation or with stale method coverage, and prepare framework proxies before PHPUnit test discovery without activating optional instrumentation. Cover ownership, retries, cancellation, memoized credential recovery, context copying, shared synchronous clients, bootstrap ordering and resource cleanup. Fix whole-class aspect matching so explicit constructor interception does not activate unrelated class-wide aspects. Document ownership and async transport configuration behavior. Validated on Guzzle 8/promises 3 and Guzzle 7/promises 2, including regression mutations. HTTP, DI, testing, Sentry, Telescope, Testbench package-mode and affected AWS/broadcasting checks pass, along with analysis and formatting.

Remove each transfer from the coroutine holder before canceling it. Exit cleanup now makes progress through its own collection instead of depending on the promise aspect to remove the entry indirectly. Normal settlement still releases transport ownership through the existing bookkeeping. The ownership, exit cleanup and retry tests pass on Guzzle 7 and 8.

Add focused Guzzle 7 CI coverage, including streaming transports, and a manual runner for unchanged upstream promise suites. Consolidate yielding callback coverage around real request and database transaction isolation. Exercise Saloon Digest authentication over a loopback connection so its test works with both native cURL negotiation and Guzzle authentication middleware. Validation: full framework suites passed on Guzzle 8 and 7; both upstream promise suites passed with and without the integration. Expanded compatibility checks and the final consolidated tests pass on their supported versions. A stock-queue mutation reproduces the cross-request context failure.

Add a developer harness using production provider registration and generated proxies, with a separate loopback origin. Measure startup independently from promise, HTTP, middleware, credential, broadcasting and cancellation workloads, including process CPU, memory, descriptors and cleanup. Document paired baseline comparisons, named connection reuse, the diagnostic pass-through aspect and measurement limits. Benchmark PHP files pass syntax checks and formatting.

Avoid rebuilding the generic aspect pipeline and reflecting the original closure for every intercepted call. Cache only immutable method chains, while resolving each aspect from the current container and keeping all invocation state local. Preserve priority, rebinding, scoped aspects, nested and concurrent dispatch, short-circuiting, references, and cleanup. Remove the unused AOP pipeline and its direct package dependency. Validated with focused AOP and consumer tests, both Guzzle dependency families, static analysis, formatting, the full 44,038-test suite, and package-mode Testbench. Measured guarded promise-chain CPU drops by about 48% from the initial implementation.

Publish generated proxies through a separate authoritative Composer loader so the application class map remains unchanged. Resolve the application loader through Composer registration metadata and preserve source lookup, regeneration, and validation before publication. Select exact targets directly, compile wildcard patterns once, enumerate the fixed generator directory without Finder, and construct the parser and printer only when code generation is required. These changes remove repeated map scanning and two retained full-map copies across the initial implementation, while preserving proxy fingerprints and visitor behavior. Validated cold and cached startup, PSR-4 sources, overrides, atomic failures, and repeated applications. Both dependency families pass focused coverage; the full suite passes 44,038 tests, package-mode Testbench passes 635 tests, and the package consumer fixture passes 6 tests. Measured retained heap falls by 2.88 MiB and cached bootstrap time by about 57% versus the initial ownership implementation.

Record paired Guzzle 7 and 8 measurements, shared dispatcher improvements, cold and cached bootstrap, retained heap and RSS, cancellation, and repeated-batch cleanup. Identify the native Swoole select delay separately and distinguish refreshed measurements from earlier runs. Include the profile-backed explanation of remaining dispatch, queue, and startup costs. Report autoloader configuration in the reusable harness and release its measured closure before retained-memory snapshots. Keep raw reports and temporary investigation artifacts outside the repository. Full framework, package-mode Testbench, package consumer, focused compatibility, formatting, and static-analysis checks passed.

Translate measured per-request CPU overhead into deployment-wide core estimates at example traffic levels, distinguishing arithmetic projections from throughput measurements. Explain bounded worker metadata and temporary in-flight tracking, and identify the measured outgoing paths separately from serialized credential contention. Use public reproducibility instructions in the benchmark guide and identify the published implementation revision. Benchmark syntax checks and the latest base streaming tests pass; no timing measurements were rerun.

Preserve the technical plan: failure modes, ownership guarantees, bootstrap and cleanup, SDK consumers, supported-version coverage, and benchmark methodology. Link the measured results and retain the reasons for the chosen boundaries without implementation coordination or progress records.

PHPUnit captures output outside the automatic test coroutine, allowing empty-output expectations to pass without observing printed content. Keep the environment bootstrap and response factory tests in the native output-capture context and assert the streamed cancellation output directly. Document the current limitation and track the integration work for PHPUnit issue #7039. Both affected test classes and the focused formatting check pass.

Record upstream PR #7040 alongside issue #7039 so the coroutine test integration follow-up can track the proposed fix.

Freeze the PHP clock when constructing the SQLite, Swoole and worker-array contract limiters. Elapsed I/O and scheduling time must not change consecutive decisions that the shared contract expects to occur at one instant. Keep the existing clock advancement and global cleanup behavior. Validated the affected store tests, the delayed SQLite reproduction, and the full framework suite.

Keep the native alarm for non-yielding work and observe root completion through a private buffered channel. Continue checking remaining children against the same deadline without taking the join slots used by the tested code. Route expiry through the native PHPUnit timeout result and cancel blocked children without throwing into raw callbacks. Cover busy loops, sleeping tests, abandoned children, normal root and child joins, and disabled limits. Preserve coroutine-count assertions relative to their initial infrastructure state. Full framework, Testbench, dogfood, formatting and static-analysis checks pass. A temporary path count verified that after-root polling is rare; no diagnostic code is retained.

Use a separate authoritative loader for source overrides and their generated proxies so application maps are not copied or polluted. Reset unloaded overrides between application boots, accept loaded replacements only from the same canonical source, and record original proxy source paths as generated metadata. Separate replacement filenames preserve ordinary proxies retained by the persistent loader. Generate proxies beneath bootstrap/cache/aop so overlapping releases sharing storage cannot overwrite each other. Update cache clearing, the about report, Testbench cleanup and documentation. Use xxh128 for internal fingerprints and helper names, retaining source-path, content and rule invalidation coverage. Validate repeated boot, cleanup, replacement priority, source regeneration and shared-storage releases. Full framework, Testbench, dogfood, formatting and analysis pass. Preserve the benchmark revision and explicitly identify later unmeasured bootstrap corrections in the report; runtime dispatch and ownership checks are unchanged.

coderabbitai · 2026-10-09T14:54:55Z

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository: hypervel/components-backup/.coderabbit.yaml
Review profile: CHILL
Plan: Advanced
Run ID: a02101ad-1079-48bb-ba6b-0fbec0b0525a

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

Autofix · 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

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

qodo-free-for-open-source-projects · 2026-10-09T14:57:07Z

PR Summary by Qodo

Make Guzzle coroutine-safe and reduce AOP overhead

🐞 Bug fix ✨ Enhancement 🧪 Tests 📝 Documentation ⚙️ Configuration changes 🕐 40+ Minutes

AI Description

• Keep pending Guzzle callbacks and cURL transfers in their owning coroutine to prevent
 cross-request execution.
• Protect shared AWS credentials and Pusher transports while preserving named HTTP connection
 settings.
• Reduce AOP overhead, safeguard proxy loading across boots and releases, and expand regression
 coverage.

Diagram

graph TD
  Provider["HTTP provider"] --> Bootstrap["Proxy bootstrap"] --> Guards["Ownership aspects"] --> Promise["Guzzle promises"] --> Queue["Coroutine queue"] --> State["Coroutine state"]
  Guards --> Transport["cURL multi-handler"] --> State
  Consumers["SDK consumers"] --> Promise
  Consumers --> Transport

High-Level Assessment

The supported Guzzle queue setter plus narrow interception addresses both callback and native-transfer ownership without replacing Guzzle's scheduler or maintaining a fork. Middleware alone cannot cover internally constructed promises, and queue isolation alone cannot protect active multi-handlers. Caching only immutable AOP metadata preserves scoped aspect resolution.

Files changed (106) +4972 / -424

Enhancement (11) +193 / -80

PusherHandlerContext.phpHold coroutine-local Pusher handlers +26/-0

Hold coroutine-local Pusher handlers
• Adds non-copyable context with weak client keys for async handler reuse.
src/broadcasting/src/PusherHandlerContext.php

Ast.phpLazily construct AST tooling +6/-9

Lazily construct AST tooling
• Creates the parser and printer only when proxy work requires them.
src/di/src/Aop/Ast.php

ProceedingJoinPoint.phpReceive intercepted instances directly +8/-7

Receive intercepted instances directly
• Removes per-call closure reflection and represents static calls with a null instance.
src/di/src/Aop/ProceedingJoinPoint.php

ProxyCallVisitor.phpEmit instance and proxy metadata +12/-1

Emit instance and proxy metadata
• Passes the target to dispatch and marks rewritten methods and original source paths.
src/di/src/Aop/ProxyCallVisitor.php

ProxyDispatcher.phpCompile lightweight dispatch chains +25/-24

Compile lightweight dispatch chains
• Caches continuations while resolving aspect services from the current container for each invocation.
src/di/src/Aop/ProxyDispatcher.php

ProxyManager.phpOptimize proxy generation +26/-39

Optimize proxy generation
• Selects exact rules directly, separates replacement filenames, and switches internal fingerprints to xxh128.
src/di/src/Aop/ProxyManager.php

ProxyMethod.phpMark intercepted methods +18/-0

Mark intercepted methods
• Adds method metadata used to verify loaded proxy coverage.
src/di/src/Aop/ProxyMethod.php

ProxySource.phpRecord proxy source paths +18/-0

Record proxy source paths
• Adds class metadata for safe same-source override registration after a boot.
src/di/src/Aop/ProxySource.php

PromiseConstructionAspect.phpRecord promise creators +26/-0

Record promise creators
• Captures ownership after successful mutable promise construction.
src/http/src/Client/Guzzle/Aspects/PromiseConstructionAspect.php

TransportState.phpCount active handler reservations +17/-0

Count active handler reservations
• Stores a multi-handler's owner and active transfer count.
src/http/src/Client/Guzzle/TransportState.php

CoroutineOwnershipException.phpIdentify ownership violations +11/-0

Identify ownership violations
• Adds an exception for cross-coroutine operations on pending Guzzle work.
src/http/src/Exceptions/CoroutineOwnershipException.php

Bug fix (21) +870 / -101

BroadcastManager.phpIsolate async Pusher transports +51/-9

Isolate async Pusher transports

• Reuses asynchronous handlers within a client and coroutine while retaining synchronous reuse. Rejects default-client connection caps requiring cross-coroutine transport sharing.

src/broadcasting/src/BroadcastManager.php

ClearCommand.phpClear release-local proxies +1/-1

Clear release-local proxies
• Moves proxy clearing to bootstrap/cache/aop.
src/cache/src/Console/ClearCommand.php

Aspect.phpPreserve explicit constructor rules +5/-1

Preserve explicit constructor rules
• Allows explicit constructor interception without applying unrelated class-wide aspects.
src/di/src/Aop/Aspect.php

RewriteCollection.phpAlign constructor rewrite exclusions +5/-10

Align constructor rewrite exclusions
• Shares class-wide exclusions with aspect matching while allowing explicit rules.
src/di/src/Aop/RewriteCollection.php

GenerateProxies.phpValidate and isolate proxy publication +105/-46

Validate and isolate proxy publication
• Rejects early-loaded targets and proxies missing required interception. Publishes release-local proxies through dedicated loaders.
src/di/src/Bootstrap/GenerateProxies.php

ClassMapManager.phpIsolate class-map overrides +38/-7

Isolate class-map overrides
• Uses a separately resettable authoritative loader and permits loaded targets only when their canonical source matches.
src/di/src/ClassMap/ClassMapManager.php

FilesystemManager.phpWrap callable S3 credentials +5/-0

Wrap callable S3 credentials
• Protects shared providers before S3 client construction.
src/filesystem/src/FilesystemManager.php

AboutCommand.phpRecognize release-local proxy cache +1/-1

Recognize release-local proxy cache
• Reports generated PHP files in bootstrap/cache/aop accurately.
src/foundation/src/Console/AboutCommand.php

RunTestsInCoroutine.phpEnforce suspended-test deadlines +91/-8

Enforce suspended-test deadlines
• Adds a deadline watcher for yielding tests and children while retaining PHPUnit's busy-loop alarm.
src/foundation/src/Testing/Concerns/RunTestsInCoroutine.php

Factory.phpPreserve named-handler options +9/-7

Preserve named-handler options
• Shares registered transport-option derivation between cached sync and fresh async handlers.
src/http/src/Client/Factory.php

PromiseOperationAspect.phpGuard pending promise operations +55/-0

Guard pending promise operations
• Checks ownership around chaining, waiting, cancellation, settlement, and pending adoption.
src/http/src/Client/Guzzle/Aspects/PromiseOperationAspect.php

TransportOwnershipAspect.phpGuard native transfers +56/-0

Guard native transfers
• Reserves handler ownership before preparation yields and prevents foreign driving of active work.
src/http/src/Client/Guzzle/Aspects/TransportOwnershipAspect.php

CoroutineOwnership.phpTrack Guzzle ownership +162/-0

Track Guzzle ownership
• Uses weak maps to guard pending promises and active handlers while permitting completed or idle reuse.
src/http/src/Client/Guzzle/CoroutineOwnership.php

CoroutineState.phpClean up owner-local work +101/-0

Clean up owner-local work
• Owns a non-copyable queue and active transfers, canceling and draining them on coroutine exit.
src/http/src/Client/Guzzle/CoroutineState.php

CoroutineTaskQueue.phpRoute tasks by coroutine +83/-0

Route tasks by coroutine
• Wraps Guzzle's queue idempotently, preserving outside-coroutine behavior.
src/http/src/Client/Guzzle/CoroutineTaskQueue.php

PendingRequest.phpConfigure isolated async requests +4/-2

Configure isolated async requests
• Selects fresh named-connection handlers for async calls and cached handlers for sync calls.
src/http/src/Client/PendingRequest.php

HttpServiceProvider.phpEnable Guzzle guards in HTTP provider +11/-0

Enable Guzzle guards in HTTP provider
• Registers the queue and aspects without relying on observability packages.
src/http/src/HttpServiceProvider.php

MailManager.phpWrap callable SES credentials +7/-2

Wrap callable SES credentials
• Protects providers before constructing SES v2 clients.
src/mail/src/MailManager.php

SqsConnector.phpWrap callable SQS credentials +2/-1

Wrap callable SQS credentials
• Serializes custom callable providers without changing non-callable credentials.
src/queue/src/Connectors/SqsConnector.php

SerializedCredentialProvider.phpSerialize shared AWS providers +74/-0

Serialize shared AWS providers
• Holds a callable-identity lock through invocation and promise completion, returning caller-specific results and releasing it on failure.
src/support/src/Aws/SerializedCredentialProvider.php

Composer.phpFind the application Composer loader +4/-6

Find the application Composer loader
• Avoids mistaking prepended proxy loaders for the vendor-backed application loader.
src/support/src/Composer.php

Refactor (4) +49 / -39

AspectCollector.phpCentralize aspect registration +29/-0

Centralize aspect registration
• Shares default-rule extraction between providers and test proxy preparation.
src/di/src/Aop/AspectCollector.php

AspectManager.phpCache compiled aspect chains +16/-21

Cache compiled aspect chains
• Stores immutable per-method continuations instead of aspect-name lists.
src/di/src/Aop/AspectManager.php

InteractsWithAop.phpPass targets into manual AOP dispatch +2/-1

Pass targets into manual AOP dispatch
• Supplies instances directly and null for static methods.
src/foundation/src/Testing/Concerns/InteractsWithAop.php

ServiceProvider.phpDelegate aspect registration +2/-17

Delegate aspect registration
• Removes duplicated default-rule reflection and clarifies override semantics.
src/support/src/ServiceProvider.php

Documentation (17) +471 / -6

2026-10-09-guzzle-coroutine-ownership.mdDocument ownership design +190/-0

Document ownership design
• Records lifecycle guarantees, architecture, validation, and rejected approaches.
docs/plans/2026-10-09-guzzle-coroutine-ownership.md

todo.mdUpdate ownership planning +1/-0

Update ownership planning
• Adds the Guzzle ownership work to project planning.
docs/todo.md

README.mdExplain broadcast connection limits +2/-0

Explain broadcast connection limits
• Documents the unsafe connection-cap options and recommends bounded concurrency.
src/broadcasting/README.md

aop.mdExplain safe proxy generation +6/-2

Explain safe proxy generation
• Documents release-local files, early-load failures, constructor rules, and test preparation.
src/docs/aop.md

broadcasting.mdExplain coroutine-safe broadcasting +4/-0

Explain coroutine-safe broadcasting
• Documents async reuse, custom handlers, and alternatives to connection caps.
src/docs/broadcasting.md

filesystem.mdDocument S3 credential serialization +2/-0

Document S3 credential serialization
• Explains callable-provider serialization and safe SDK memoization.
src/docs/filesystem.md

http-client.mdDocument Guzzle ownership +24/-3

Document Guzzle ownership
• Shows owner-local pending work and clarifies transport settings on isolated async connections.
src/docs/http-client.md

mail.mdDocument SES credential providers +2/-0

Document SES credential providers
• Explains serialization, memoization, tenant isolation, and pooling fingerprints.
src/docs/mail.md

packages.mdClarify repeated class-map overrides +1/-1

Clarify repeated class-map overrides
• Documents same-source registration and different-source rejection.
src/docs/packages.md

porting-from-laravel.mdAdd coroutine-safe migration guidance +4/-0

Add coroutine-safe migration guidance
• Covers Guzzle pending work, lazy clients, and broadcast concurrency limits.
src/docs/porting-from-laravel.md

queues.mdDocument SQS provider serialization +2/-0

Document SQS provider serialization
• Explains shared callables, optional memoization, and tenant-sensitive credentials.
src/docs/queues.md

testing.mdWarn about coroutine output assertions +3/-0

Warn about coroutine output assertions
• Describes PHPUnit output-capture limitations and the non-coroutine workaround.
src/docs/testing.md

README.mdState Guzzle ownership rule +2/-0

State Guzzle ownership rule
• Advises completing pending work in its creating coroutine.
src/http/README.md

Http.phpAnnotate new handler API +1/-0

Annotate new handler API
• Adds facade documentation for named-connection handler creation.
src/support/src/Facades/Http.php

README.mdDocument benchmark reproduction +51/-0

Document benchmark reproduction
• Explains workloads, commands, controls, and interpretation.
tests/Benchmarks/GuzzleOwnership/README.md

results.mdReport measured costs +145/-0

Report measured costs
• Records CPU, startup, memory, compatibility, and measurement limitations.
tests/Benchmarks/GuzzleOwnership/results.md

README.mdDocument ownership test execution +31/-0

Document ownership test execution
• Provides focused regression and compatibility instructions.
tests/Http/Client/Guzzle/README.md

Other (53) +3389 / -198

tests.ymlAdd Guzzle 7 compatibility CI +37/-0

Add Guzzle 7 compatibility CI
• Adds a focused job for Guzzle 7, promises 2, ownership, and affected consumers.
.github/workflows/tests.yml

.php-cs-fixer.phpDisable unsafe return-assignment formatting +2/-3

Disable unsafe return-assignment formatting
• Prevents rewriting assignments captured by reference in Guzzle wait callbacks.
.php-cs-fixer.php

composer.jsonDeclare broadcast transport dependencies +3/-0

Declare broadcast transport dependencies
• Adds direct promises, context, and HTTP-message dependencies.
src/broadcasting/composer.json

composer.jsonRemove unused AOP pipeline dependency +0/-1

Remove unused AOP pipeline dependency
• Drops the dedicated DI pipeline dependency.
src/di/composer.json

composer.jsonDeclare HTTP DI dependency +1/-0

Declare HTTP DI dependency
• Adds the direct dependency needed by ownership aspects.
src/http/composer.json

composer.jsonDeclare credential-wrapper dependencies +2/-0

Declare credential-wrapper dependencies
• Adds direct coroutine and engine package requirements.
src/support/composer.json

testbench.yamlPurge local proxies +1/-1

Purge local proxies
• Changes skeleton cleanup to bootstrap/cache/aop.
src/testbench/testbench.yaml

composer.jsonDeclare promises test dependency +1/-0

Declare promises test dependency
• Adds the direct requirement used by PHPUnit queue setup.
src/testing/composer.json

AfterEachTestExtension.phpPrepare Guzzle proxies before discovery +58/-0

Prepare Guzzle proxies before discovery
• Seeds a safe test queue and generates framework proxies without activating optional instrumentation.
src/testing/src/PHPUnit/AfterEachTestExtension.php

AfterEachTestSubscriber.phpReset Guzzle test state +8/-0

Reset Guzzle test state
• Cancels test transfers and discards abandoned outside-coroutine callbacks.
src/testing/src/PHPUnit/AfterEachTestSubscriber.php

NoopAspect.phpProvide dispatch measurement control +19/-0

Provide dispatch measurement control
• Adds a pass-through aspect for isolating AOP cost.
tests/Benchmarks/GuzzleOwnership/Fixtures/NoopAspect.php

server.phpProvide loopback benchmark origin +24/-0

Provide loopback benchmark origin
• Serves requests separately so server CPU is excluded.
tests/Benchmarks/GuzzleOwnership/Fixtures/server.php

benchmark.phpMeasure outgoing-path overhead +314/-0

Measure outgoing-path overhead
• Adds configurable HTTP, promise, AWS, Pusher, and cancellation measurements.
tests/Benchmarks/GuzzleOwnership/benchmark.php

PusherClientTest.phpRegress broadcasting handler isolation +361/-0

Regress broadcasting handler isolation
• Covers concurrent callers, fan-out, reuse, configuration, custom handlers, and rejected caps.
tests/Broadcasting/PusherClientTest.php

ClearCommandTest.phpCheck new proxy clear path +1/-1

Check new proxy clear path
• Expects bootstrap/cache/aop removal.
tests/Cache/ClearCommandTest.php

AspectManagerTest.phpTest compiled-chain cache +11/-14

Test compiled-chain cache
• Updates cache assertions for closures and null misses.
tests/Di/Aop/AspectManagerTest.php

AspectTest.phpTest constructor targeting +48/-0

Test constructor targeting
• Covers explicit constructors alongside class and wildcard rules.
tests/Di/Aop/AspectTest.php

ProceedingJoinPointTest.phpTest explicit join-point target +10/-39

Test explicit join-point target
• Updates object and static-call instance assertions.
tests/Di/Aop/ProceedingJoinPointTest.php

ProxyCallVisitorTest.phpTest generated instance dispatch +80/-1

Test generated instance dispatch
• Checks constructor selection and correct instance or null forwarding.
tests/Di/Aop/ProxyCallVisitorTest.php

ProxyDispatcherTest.phpRegress cached-chain semantics +204/-18

Regress cached-chain semantics
• Tests rebinding, concurrency, nesting, short-circuiting, exceptions, and retention.
tests/Di/Aop/ProxyDispatcherTest.php

GenerateProxiesTest.phpRegress proxy bootstrap lifecycles +270/-20

Regress proxy bootstrap lifecycles
• Covers early loading, rule coverage, release caches, loaders, and replacement cleanup.
tests/Di/Bootstrap/GenerateProxiesTest.php

ClassMapManagerTest.phpTest override loader resets +73/-16

Test override loader resets
• Verifies map isolation, cleanup, same-source repeats, and different-source rejection.
tests/Di/ClassMap/ClassMapManagerTest.php

ServiceProviderDiTest.phpAdapt provider DI tests +23/-20

Adapt provider DI tests
• Checks delegated aspect registration and isolated override behavior.
tests/Di/ServiceProviderDiTest.php

CoroutineTest.phpAccount for test deadline watcher +3/-1

Account for test deadline watcher
• Adjusts coroutine-list expectations during tests.
tests/Engine/CoroutineTest.php

LoadEnvironmentVariablesTest.phpKeep output capture synchronous +3/-0

Keep output capture synchronous
• Runs output-sensitive bootstrap assertions outside a coroutine.
tests/Foundation/Bootstrap/LoadEnvironmentVariablesTest.php

ApplicationBootstrapTest.phpAdjust bootstrap test setup +0/-1

Adjust bootstrap test setup
• Removes an assumption superseded by early proxy preparation.
tests/Foundation/Testing/ApplicationBootstrapTest.php

InteractsWithAopTest.phpTest static AOP helper calls +16/-1

Test static AOP helper calls
• Checks that static interceptions expose no target instance.
tests/Foundation/Testing/Concerns/InteractsWithAopTest.php

WithCachedStateTest.phpAdjust cached-state setup +0/-1

Adjust cached-state setup
• Removes an obsolete proxy-lifecycle assumption.
tests/Foundation/Testing/WithCachedStateTest.php

BootstrapTest.phpTest clean-process ownership boot +91/-0

Test clean-process ownership boot
• Covers cached boot, early-load errors, shutdown, and transfer retention.
tests/Http/Client/Guzzle/BootstrapTest.php

CoroutineIsolationTest.phpTest callback context isolation +81/-0

Test callback context isolation
• Checks request and database-transaction ownership across yielding callbacks.
tests/Http/Client/Guzzle/CoroutineIsolationTest.php

CoroutineTaskQueueNonCoroutineTest.phpTest outside queue behavior +70/-0

Test outside queue behavior
• Covers idempotent setup and safe test reset.
tests/Http/Client/Guzzle/CoroutineTaskQueueNonCoroutineTest.php

CoroutineTaskQueueTest.phpTest owner-local callbacks +103/-0

Test owner-local callbacks
• Covers yielding drains, child isolation, ordering, and exit cleanup.
tests/Http/Client/Guzzle/CoroutineTaskQueueTest.php

bootstrap.phpExercise isolated proxy boots +95/-0

Exercise isolated proxy boots
• Provides early-load, repeat-boot, shutdown, and transfer-lifetime modes.
tests/Http/Client/Guzzle/Fixtures/bootstrap.php

run-guzzle-promise-tests.phpRun upstream promise suites +64/-0

Run upstream promise suites
• Validates supported promise families against generated interception.
tests/Http/Client/Guzzle/Fixtures/run-guzzle-promise-tests.php

PromiseOwnershipTest.phpRegress promise ownership +142/-0

Regress promise ownership
• Covers pending operations, completed sharing, adoption, and failed self-resolution.
tests/Http/Client/Guzzle/PromiseOwnershipTest.php

TransportOwnershipTest.phpRegress native-transfer ownership +410/-0

Regress native-transfer ownership
• Covers foreign driving, yielding preparation, cancellation, owner exit, and reuse.
tests/Http/Client/Guzzle/TransportOwnershipTest.php

HttpConnectionTest.phpTest async connection settings +53/-11

Test async connection settings
• Verifies transport options survive isolated handlers and preset overrides.
tests/Http/HttpConnectionTest.php

AboutCommandTest.phpCheck updated AOP status +2/-2

Check updated AOP status
• Expects release-local generated proxy detection.
tests/Integration/Foundation/Console/AboutCommandTest.php

DatabaseStoreTest.phpFreeze SQLite limiter clock +12/-5

Freeze SQLite limiter clock
• Keeps contract decisions deterministic despite I/O delays.
tests/Integration/RateLimiter/Database/Sqlite/DatabaseStoreTest.php

QueueSqsConnectorTest.phpTest wrapped SQS callables +6/-4

Test wrapped SQS callables
• Checks callable resolution and credential-object pass-through.
tests/Queue/QueueSqsConnectorTest.php

SwooleStoreTest.phpStabilize Swoole limiter tests +5/-0

Stabilize Swoole limiter tests
• Freezes the clock for timing-sensitive contracts.
tests/RateLimiter/SwooleStoreTest.php

WorkerArrayStoreTest.phpStabilize worker-array limiter tests +6/-0

Stabilize worker-array limiter tests
• Freezes the clock for timing-sensitive contracts.
tests/RateLimiter/WorkerArrayStoreTest.php

ResponseFactoryTest.phpKeep output assertions outside coroutines +4/-3

Keep output assertions outside coroutines
• Runs output-sensitive response tests synchronously.
tests/Routing/ResponseFactoryTest.php

AuthenticatesRequestsTest.phpExercise real digest negotiation +14/-21

Exercise real digest negotiation
• Replaces sender-option inspection with a loopback challenge across Guzzle versions.
tests/Saloon/Feature/AuthenticatesRequestsTest.php

AwsCredentialConsumerTest.phpRegress pooled AWS consumers +129/-0

Regress pooled AWS consumers
• Checks SQS, S3, and SES with shared providers and caller-specific credentials.
tests/Support/AwsCredentialConsumerTest.php

SerializedCredentialProviderNonCoroutineTest.phpTest non-coroutine provider calls +34/-0

Test non-coroutine provider calls
• Confirms providers complete outside coroutines.
tests/Support/SerializedCredentialProviderNonCoroutineTest.php

SerializedCredentialProviderTest.phpRegress credential-provider locking +330/-0

Regress credential-provider locking
• Covers callable aliases, per-caller results, failures, cancellation, and memoization.
tests/Support/SerializedCredentialProviderTest.php

SupportComposerTest.phpTest application loader selection +17/-0

Test application loader selection
• Checks discovery with a prepended auxiliary loader.
tests/Support/SupportComposerTest.php

PurgeSkeletonCommandTest.phpCheck local proxy purging +2/-2

Check local proxy purging
• Updates Testbench artifact assertions to bootstrap/cache/aop.
tests/Testbench/Foundation/Console/PurgeSkeletonCommandTest.php

AfterEachTestExtensionTest.phpTest early proxy and queue setup +20/-0

Test early proxy and queue setup
• Checks optional instrumentation remains disabled and callbacks cannot leak.
tests/Testing/PHPUnit/AfterEachTestExtensionTest.php

GuzzleCleanupFixture.phpProvide callback-leak fixture +36/-0

Provide callback-leak fixture
• Leaves a callback pending and verifies subsequent cleanup.
tests/Testing/PHPUnit/Fixtures/GuzzleCleanupFixture.php

TimeLimitFixture.phpProvide deadline scenarios +37/-0

Provide deadline scenarios
• Covers busy work, sleeping work, children, and completed tests.
tests/Testing/PHPUnit/Fixtures/TimeLimitFixture.php

TimeLimitTest.phpTest coroutine timeout results +53/-12

Test coroutine timeout results
• Checks named failures for timed-out work and success for completed work.
tests/Testing/PHPUnit/TimeLimitTest.php

greptile-apps · 2026-10-09T15:04:37Z

Confidence Score: 4/5

[High impact] Rewrites AOP interception and Guzzle concurrency handling across the framework.

The PR needs the previously reported replacement-loader priority issue fixed before merging.

Findings

Old proxies hide replacements ▶

Summary

This PR keeps pending Guzzle work in its owning coroutine, updates affected AWS and broadcasting clients, and reduces shared AOP work.

Changes since the previous review allow credential-provider callbacks to reenter in the same coroutine.
Instrumentation tests now call generated proxies directly.
Notification snapshots use offsetExists() for object membership.
No new actionable issues were found.

_{Reviews (2) · Last reviewed commit: "Avoid deprecated object storage lookups ..." · Reviewed by Greptile}

greptile-apps · 2026-10-09T15:04:40Z

+        $ordinaryProxies = array_diff_key($proxies, $replacements);
+
+        if ($ordinaryProxies !== []) {
+            if (static::$proxyLoader === null) {
+                static::$proxyLoader = new ClassLoader;
+                static::$proxyLoader->setClassMapAuthoritative(true);
+                static::$proxyLoader->register(true);
+            }
+
+            static::$proxyLoader->addClassMap($ordinaryProxies);
+        }
+
+        ClassMapManager::applyProxies($proxies);


Old proxies hide replacements

If the first application registers any replacement before proxy generation, the ordinary proxy loader is placed ahead of the replacement loader. Refreshing the application without test teardown, then replacing an unloaded class that already has an ordinary proxy, updates only the lower-priority replacement loader. The old proxy still wins, so PHP loads the original implementation instead of the registered replacement.

Remove overlapping ordinary entries or restore replacement-loader priority when publishing replacements. Add a test that refreshes the application without calling flushState() between boots.

The proposed ordering does not occur through the supported bootstrap paths. Production registers provider overrides before its single proxy-generation pass, so the generated ordinary and replacement maps are disjoint. The shared PHPUnit extension creates the persistent proxy loader before application providers can register overrides; the override loader therefore stays ahead of it during refreshApplication(), and test cleanup resets overrides between tests. The isolated two-generation reproduction omitted that test bootstrap. No additional loader reordering is needed for these paths.

That context resolves the concern. The problematic ordering requires registering a replacement after the ordinary proxy loader has already been published; the supported production bootstrap does not do that, and the PHPUnit bootstrap establishes the replacement loader’s priority before refreshes. Since the isolated reproduction omitted that test bootstrap, this is not an actionable issue for the supported paths. No additional loader reordering is needed.

qodo-free-for-open-source-projects · 2026-10-09T15:05:29Z

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Adopted transfers escape owner cleanup 🐞 Bug ☼ Reliability

Description

PromiseOperationAspect::process() calls CoroutineOwnership::settled() after resolve() or
reject() even when the transfer promise has adopted another pending promise. If a tracked transfer
adopts pending work, this removes it from owner-exit cancellation and releases the multi-handler's
ownership reservation while that work remains unfinished.

Code

src/http/src/Client/Guzzle/Aspects/PromiseOperationAspect.php[51]
+        CoroutineOwnership::settled($promise);

Evidence
The aspect records pending adoption but still unconditionally settles the tracked promise.
Settlement removes it from both the owner's cancellation list and the handler's active reservation;
the promise tests establish that an adopted chain can remain dependent on pending work after
resolution.
src/http/src/Client/Guzzle/Aspects/PromiseOperationAspect.php[37-51]
src/http/src/Client/Guzzle/CoroutineOwnership.php[125-149]
src/http/src/Client/Guzzle/CoroutineState.php[78-99]
tests/Http/Client/Guzzle/PromiseOwnershipTest.php[103-125]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A tracked transfer that adopts a pending promise is removed from transfer and handler ownership tracking before the adopted work finishes.
## Fix Focus Areas
- src/http/src/Client/Guzzle/Aspects/PromiseOperationAspect.php[37-53]
- src/http/src/Client/Guzzle/CoroutineOwnership.php[125-149]
## Recommended Fix
Release transfer tracking only once the promise and its adopted work are complete. Preserve owner-exit cancellation and the handler reservation while adopted work is pending, and add a regression test for that path.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. ~~Documented test fallback for early-loaded aspect targets now fails at boot~~ ✓ Resolved

Description

GenerateProxies::ensureLoadedProxyCoversRules throws InvalidDefinitionException whenever a
matched target is already loaded without ProxyMarker. The InteractsWithAop trait still describes
that exact case as supported: "load order may mean the original class was loaded before proxy
generation, so the test must use callWithAspects() instead". Now, when a provider registers an
aspect after an earlier test has loaded the target, the application fails during bootstrap, so the
test never reaches isAopProxied()/callWithAspects() and the unchanged guidance sends readers
down a path that no longer works.

Code

src/di/src/Bootstrap/GenerateProxies.php[R98-103]

+        if (! in_array(ProxyMarker::class, $reflection->getTraitNames(), true)) {
+            throw new InvalidDefinitionException(
+                "The AOP target [{$class}] was loaded before proxy generation. "
+                . 'Bind a factory in register() and instantiate it after proxy generation, during boot() or later.'
+            );
+        }

Evidence

ProxyManager adds every class-map target that matches a rule to the proxies list. It does not
check whether the class is already loaded. generate() then calls ensureLoadedProxyCoversRules
for each proxy, and that method throws for any loaded class without ProxyMarker. The unchanged
docblocks on InteractsWithAop (isAopProxied and the trait description) still tell test authors
that this load-order situation is expected and that they should handle it with callWithAspects().
The PR changed the bootstrap behaviour but did not update that trait's guidance.

src/di/src/Bootstrap/GenerateProxies.php[90-103]
src/foundation/src/Testing/Concerns/InteractsWithAop.php[27-42]
src/di/src/Aop/ProxyManager.php[36-45]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
After this change, application bootstrap throws when a matched AOP target class is loaded before proxy generation. `InteractsWithAop` still tells test authors that this load order is expected and can be handled with `callWithAspects()`, but that path can no longer be reached when the aspect is registered by a provider.
## Fix Focus Areas
- src/foundation/src/Testing/Concerns/InteractsWithAop.php[12-42]
- src/di/src/Bootstrap/GenerateProxies.php[90-103]
## Recommended Fix
Rewrite the `InteractsWithAop` trait and `isAopProxied()` docblocks to say that `callWithAspects()` only applies to aspects registered directly with `AspectCollector` after bootstrap. Aspects registered by providers on classes that are already loaded now fail during boot. Optionally, add a test-only escape hatch (for example, a flag that skips the loaded-target check in Testbench) if this fallback is meant to stay supported.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Tip of the day

💡 Did you know, you can show, collapse, or hide each part of a finding: code, evidence, and all

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Waiting for memoized credentials drains Guzzle callbacks. A callback starting another SDK request could wait for a provider lock held by the same coroutine and deadlock. Keep cross-coroutine serialization while allowing nested calls by the native owner. Only the outermost frame acquires and releases the lock, and every invocation still waits for its own completed result. Clear the bounded owner map during normal release and test cleanup. Cover nested requests through two framework-created S3 clients using callable aliases, cached credentials, refreshes and subsequent use from another coroutine. The regression hangs with reentry disabled. Full framework, Testbench, package integration, formatting and static analysis checks pass. Clarify that existing credential benchmarks predate this correction.

The early-loaded target test correctly triggered its bootstrap exception in CI, but PHP wrote it to standard output while the assertion read standard error. Set the subprocess error destination explicitly. Remove obsolete manual-interception fallbacks from Sentry and Telescope tests now that the PHPUnit extension generates their Guzzle proxies before discovery. Correct the testing helper guidance without weakening bootstrap validation. Bootstrap, Sentry and Telescope tests pass, along with formatting and static analysis. The full framework and package-mode suites also pass.

Use SplObjectStorage::offsetExists() for both shared-reference lookups instead of contains(), which is deprecated in PHP 8.5. The replacement is available on PHP 8.4 and preserves object membership semantics. Verified on PHP 8.4, including stored null values. Notification transport serialization tests and the full framework suite pass.

binaryfire added 18 commits October 9, 2026 09:47

Link the PHPUnit output buffering proposal

2cea8b2

Record upstream PR #7040 alongside issue #7039 so the coroutine test integration follow-up can track the proposed fix.

greptile-apps Bot reviewed Oct 9, 2026

View reviewed changes

qodo-free-for-open-source-projects Bot reviewed Oct 9, 2026

View reviewed changes

Comment thread src/http/src/Client/Guzzle/Aspects/PromiseOperationAspect.php

Comment thread src/di/src/Bootstrap/GenerateProxies.php

binaryfire added 3 commits October 9, 2026 15:43

binaryfire merged commit c71b3c1 into 0.4 Oct 9, 2026
54 checks passed

Navigation Menu

Repository navigation

Make Guzzle coroutine-safe and reduce AOP overhead - #60

Make Guzzle coroutine-safe and reduce AOP overhead#60
binaryfire merged 21 commits into
0.4from
fix/guzzle-coroutine-ownership

binaryfire commented Oct 9, 2026 •

edited by macroscopeapp Bot

Loading

Uh oh!

coderabbitai Bot commented Oct 9, 2026 •

edited

Loading

Review skipped

Uh oh!

qodo-free-for-open-source-projects Bot commented Oct 9, 2026

Uh oh!

greptile-apps Bot commented Oct 9, 2026 •

edited

Loading

Uh oh!

greptile-apps Bot Oct 9, 2026

Uh oh!

binaryfire Oct 9, 2026

Uh oh!

greptile-apps Bot Oct 9, 2026

Uh oh!

qodo-free-for-open-source-projects Bot commented Oct 9, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

binaryfire commented Oct 9, 2026 • edited by macroscopeapp Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Keep Guzzle callbacks and transfers in their owning coroutine

Why AOP is needed

Correct shared SDK and HTTP consumers

Reduce shared AOP work

Make class-map overrides safe across application boots

Approaches considered and rejected

AOP performance improvements

Ownership overhead across outgoing paths

Memory bounds and capacity planning

Measurement conditions and validation

Make Guzzle coroutine-safe and reduce AOP dispatch overhead

Uh oh!

coderabbitai Bot commented Oct 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Uh oh!

qodo-free-for-open-source-projects Bot commented Oct 9, 2026

PR Summary by Qodo

Uh oh!

greptile-apps Bot commented Oct 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Confidence Score: 4/5

Findings

Summary

Uh oh!

greptile-apps Bot Oct 9, 2026

Choose a reason for hiding this comment

Uh oh!

binaryfire Oct 9, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Oct 9, 2026

Choose a reason for hiding this comment

Uh oh!

qodo-free-for-open-source-projects Bot commented Oct 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

binaryfire commented Oct 9, 2026 •

edited by macroscopeapp Bot

Loading

coderabbitai Bot commented Oct 9, 2026 •

edited

Loading

greptile-apps Bot commented Oct 9, 2026 •

edited

Loading

qodo-free-for-open-source-projects Bot commented Oct 9, 2026 •

edited

Loading