Harden Pipeline, Bus, and Core lifecycles#443
Merged
Conversation
Fail named Hub dispatches with Laravel's explicit InvalidArgumentException instead of falling through an undefined array key into an incidental TypeError. Keep Hypervel's exact null and empty-string default selection ahead of the guard so the valid pipeline name "0" remains addressable. Port the current upstream Hub coverage and merge the zero-name regression into the dedicated test class.
Bind both Pipeline::class and the facade key through one transient factory so the mutable concrete cannot enter the container's worker-lifetime auto-singleton cache. Preserve the existing facade lifecycle while restoring Laravel-style freshness for direct concrete resolution. Add a bounded, deterministic sibling-coroutine regression that proves passables and finalizers cannot overwrite one another during interleaved execution.
Add the repository-required void return types to the existing Pipeline and facade test methods without widening the change into closure or fixture typing churn. Remove the duplicate Hub zero-name case now that Hub behavior lives in its current upstream-aligned dedicated test class. Existing Pipeline, macro, facade freshness, and enum transaction coverage remains unchanged.
Require hypervel/database directly because Pipeline's public withinTransaction capability resolves and invokes the database manager. This makes the package contract truthful instead of relying on Support's transitive dependency. Database is already mandatory in the current Hypervel package graph, so the metadata correction adds no installed or runtime weight.
Replace the discarded Event::dispatched query with an actual dispatch assertion and compare ConnectionEvent::connectionName with the expected configured name. The enum, string, and default-connection cases now fail when Pipeline selects the wrong database instead of passing vacuously. Use strict result assertions and complete the package test method return types at the same boundary.
Identify the current Laravel 13.x Pipeline source as the package's porting reference, matching the concise format used by sibling Hypervel package READMEs. Keeping the upstream location at the package boundary makes future incremental parity work discoverable without adding unnecessary package documentation.
Record pipeline-01 through pipeline-05, their verified ownership boundaries, accepted implementation, current Laravel discovery sources, regression coverage, validation results, and final performance and overengineering assessment. Mark Pipeline complete and route the next audit to Bus with the exact queue-12, support-02, and shared-Pipeline invariants that must be revalidated. Keep rejected context, locking, cloning, registry, and contract-binding machinery explicit so future audits do not reopen disproven designs.
Replace the request-wide context bracket with a lazy WeakMap keyed by the exact unique job object. Record ownership only after the unique lock is acquired, consume it once during payload creation, and let weak keys prevent worker-lifetime retention. Carry the metadata safely through deferred, after-response, synchronous, background, and after-commit dispatch paths while restoring log context on both success and failure. Make after-response selection reversible at the pending-dispatch boundary. Declare the direct Foundation and Queue log dependencies, register deterministic test-state cleanup, remove the superseded trait, and cover exact identity, weak retention, delayed payload creation, and failure cleanup.
Add grouped bulk dispatch to the dispatcher and queueing contract, and bind the QueueingDispatcher alias through the same fakeable base dispatcher used by the public Bus surface. Bring the preparation contract, facade annotations, Bus fake, and pending-batch fake in line with the current upstream behavior, including boolean-or-void preparation results, bulk routing, array batch assertions, sync and after-response assertions, nullable fake returns, and correctly serialized batched jobs at the record boundary. Keep provider wiring on typed batching configuration and add focused unit, integration, and Testbench coverage for the public contracts, fake behavior, dispatch preparation, and corrected upstream regression cases.
Complete the batch lifecycle with guarded started, finished, and canceled events, cancellation-aware job handling, finished-batch support, preserved routing metadata, and correct zero-valued identifiers and cursors. Make database count updates and callback refreshes tolerate batches deleted or pruned during execution. Resolve pooled connections per operation instead of mutating a worker-shared repository, expose the read-only connection name, and tighten repository and configuration types. Keep fake repositories, Horizon responses, and console output consistent with the real lifecycle, and add regression coverage for event gates, cancellation, missing rows, connection ownership, callback state, pagination boundaries, and exhaustive worker-global test cleanup.
Use one owner cache read, fail open when the lock has already expired, and establish the maximum-wait anchor with an atomic cache add so concurrent first attempts cannot overwrite one another. Guard the optional JobDebounced event with hasListeners while preserving EventFake assertions, avoiding event construction and dispatch on the normal no-listener queue hot path. Add deterministic concurrent-anchor, cache-read-count, missing-lock, and event-gating coverage across the debounce lock and queued handler paths.
Document bulk dispatch, dispatch preparation, reversible after-response selection, batch lifecycle events, finished batches, and the matching Bus fake assertions in the task-first queue guide. Call out that bulk dispatch goes directly to the selected queue driver and therefore does not run preparation, unique-job, or debounce lifecycles; jobs using those features must be dispatched individually. Record the package provenance and the intentional DynamoDB and mutable batch-connection differences so future upstream updates preserve Hypervel pooled-connection safety.
Record the final bus-01 through bus-19 findings, accepted fixes, important rejected concerns, public API decisions, hot-path assessment, regression coverage, full validation results, and completed review state. Mark Bus complete, add the shared Bus dependency and revalidation routes, advance the active package to Core, and preserve the verified 71-package checklist consistency.
Reload the default low-level logger after worker configuration is rebuilt so replacement workers publish validated levels and format before startup logging and readiness. Make line interpolation PSR-safe, escape dynamic Symfony Console values without formatting ordinary messages, and emit raw resilient JSON with valid fallbacks and informative context normalization. Precompute enabled levels for constant-time filtering while preserving configured custom levels and PSR invalid-level errors. Add focused coverage for reload atomicity and ordering, custom logger ownership, safe context conversion, markup and percent handling, malformed JSON inputs, throwing serializers, and level and format validation.
Select the native task callback shape from task_enable_coroutine, task_object, and the presence-sensitive task_use_object alias exactly as Swoole does. Route task completion through the native Task in object mode and through the Server in legacy mode, and ship an explicit disabled coroutine-task default. Reject truthy event_object settings at the global and per-port mutation boundaries because that native mode changes callback signatures beyond Hypervel positional lifecycle adapters. Preserve explicit false, identify the offending port, and cover construction and later supported configuration mutation. Add focused task, server configuration, and shipped-default tests for every supported mode and precedence boundary.
Narrow receive-event payloads to the native string type exposed by the sole callback path. Delete the empty server-start hook, unused framework exception, and unbound console logger. These Hyperf-era public classes had no consumers or supported behavior and implied extension alternatives the framework does not provide.
Declare the Swoole extension as a direct Core requirement because lifecycle callbacks type and call its native classes. Remove the unused Coroutine package edge and record the Hyperf framework source so future maintenance uses the correct architectural reference.
Explain the low-level stdout logger configuration, supported line and JSON formats, and worker-start reload behavior without conflating it with the application logging stack. Document that Swoole event_object is incompatible with Hypervel lifecycle callbacks and direct users toward the framework lifecycle events. Remove the stale response-emitter claim from shipped configuration guidance.
Record core-01 through core-08 with their verified failure boundaries, accepted implementations, rejected complexity, public configuration decisions, regression coverage, performance assessment, full validation results, and completed independent review. Mark Core complete, add the shared Core revalidation routes for Foundation and Server, and advance the active package to Foundation with every exact inherited ledger decision required after compaction. Preserve the verified 71-package checklist parity and keep the final assessment explicit that the work adds no workaround, stale surface, retained request state, hot-path regression, or unresolved finding.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThe PR updates Pipeline, Bus, queue, Core, Foundation, server, logging, documentation, and test lifecycle behavior. It adds dispatch and batch contracts, coroutine-safe state handling, Swoole validation, resilient stdout logging, dependency metadata, and regression coverage. ChangesPipeline lifecycle and validation
Bus dispatch and batch lifecycle
Core lifecycle, logging, and server configuration
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant PendingDispatch
participant UniqueJobPayloadContext
participant Queue
participant Context
PendingDispatch->>UniqueJobPayloadContext: register unique job metadata
Queue->>UniqueJobPayloadContext: consume metadata for the exact job
Queue->>Context: scope payload hooks with hidden metadata
Context-->>Queue: return payload-hook result
sequenceDiagram
participant SwooleServer
participant TaskCallback
participant EventDispatcher
participant Server
SwooleServer->>TaskCallback: invoke task callback arguments
TaskCallback->>EventDispatcher: dispatch OnTask with native or legacy task data
EventDispatcher-->>TaskCallback: return task result
TaskCallback->>Server: finish result for legacy task mode
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Greptile SummaryThis PR hardens Pipeline, Bus, and Core components for Swoole worker-lifetime correctness, addressing mutable-builder singleton leakage, delayed-payload ownership, batch concurrency, and native callback signatures.
Confidence Score: 5/5Safe to merge — all changes address documented Swoole worker-lifetime defects with no regressions detected across the changed paths. The changes are structurally sound: transient Pipeline binding eliminates the singleton cache, WeakMap unique-job context ties metadata to the exact job object, per-call getConnection() removes the mutable worker-shared connection setter, and null-return batch counts guard concurrent deletion without altering the happy path. The isFirstJobProcessed formula is arithmetically correct across success, failure, and concurrent-processing scenarios. The StdoutLogger rewrite validates atomically at boot, falls back gracefully on JSON encoding failures, and is PSR-3 compliant. No new hot-path locking, cross-coroutine shared state, or unchecked null dereferences were introduced. No files require special attention; BatchFake non-nullable return overrides are pre-existing covariant narrowing already noted in a prior review thread. Important Files Changed
Reviews (3): Last reviewed commit: "docs(audit): record collision-safe bulk ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
tests/Testbench/Integrations/DispatchJobTest.php (1)
16-17: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueFix typo in the test method name.
Consider renaming this test method to
itCanTriggerExpectedJobsoritTriggersExpectedJobsfor grammatical correctness.📝 Proposed rename
#[Test] - public function itCanTriggersExpectedJobs(): void + public function itCanTriggerExpectedJobs(): void🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/Testbench/Integrations/DispatchJobTest.php` around lines 16 - 17, Rename the test method itCanTriggersExpectedJobs to a grammatically correct name, preferably itCanTriggerExpectedJobs, and update any references to the method if present.tests/Core/StdoutLoggerTest.php (1)
409-423: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove throwing stub classes into
Fixtures/.
ThrowingStdoutLoggerStringableandThrowingStdoutLoggerJsonSerializableare standalone test support classes defined inline in the test file, while this same test already relies ontests/Core/Fixtures/TestObject.phpfor the analogousTestObjectfixture.♻️ Suggested move
-class ThrowingStdoutLoggerStringable implements Stringable -{ - public function __toString(): string - { - throw new RuntimeException('String conversion failed.'); - } -} - -class ThrowingStdoutLoggerJsonSerializable implements JsonSerializable -{ - public function jsonSerialize(): mixed - { - throw new RuntimeException('JSON serialization failed.'); - } -}Move both classes to
tests/Core/Fixtures/ThrowingStdoutLoggerStringable.phpandtests/Core/Fixtures/ThrowingStdoutLoggerJsonSerializable.php, and import them in this test file.As per path instructions,
tests/**/*.php: "Put standalone test support files inFixtures/, use test-specific namespaces only for collision-prone generic helper classes".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/Core/StdoutLoggerTest.php` around lines 409 - 423, Move ThrowingStdoutLoggerStringable and ThrowingStdoutLoggerJsonSerializable out of the test file into separate files under Core/Fixtures, preserving their existing namespaces and behavior. Remove the inline class definitions and import both fixture classes where the test uses them, matching the existing TestObject fixture pattern.Source: Path instructions
src/bus/README.md (1)
8-8: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueFix markdown heading style to resolve linter warning.
The static analysis tool
markdownlint-cli2expects a Setext-style heading here based on the document's conventions.♻️ Proposed fix
-## Differences From Laravel +Differences From Laravel +------------------------🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/bus/README.md` at line 8, Update the “Differences From Laravel” heading in the README to use Setext-style Markdown syntax, matching the document’s existing heading conventions and satisfying markdownlint-cli2.Source: Linters/SAST tools
src/bus/src/Batch.php (1)
138-153: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract a helper for the repeated "resolve dispatcher → hasListeners → dispatch" pattern.
The
Container::getInstance()->bound(Dispatcher::class)→hasListeners()→dispatch()sequence is now duplicated four times (BatchStarted ×2, BatchFinished, BatchCanceled). A small private helper would reduce duplication and avoid resolving the dispatcher twice per call inrecordSuccessfulJob/recordFailedJob.♻️ Proposed helper extraction
+ /** + * Dispatch the given event class if listeners are registered for it. + */ + protected function dispatchIfListening(string $eventClass, object $event): void + { + $container = Container::getInstance(); + + if (! $container->bound(Dispatcher::class)) { + return; + } + + $events = $container->make(Dispatcher::class); + + if ($events->hasListeners($eventClass)) { + $events->dispatch($event); + } + }Then each call site collapses to, e.g.:
- $container = Container::getInstance(); - - if ($container->bound(Dispatcher::class)) { - $events = $container->make(Dispatcher::class); - - if ($events->hasListeners(BatchStarted::class)) { - $events->dispatch(new BatchStarted($this)); - } - } + $this->dispatchIfListening(BatchStarted::class, new BatchStarted($this));Also applies to: 164-169, 252-269, 334-347
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/bus/src/Batch.php` around lines 138 - 153, Extract the repeated dispatcher resolution and event delivery logic into a private helper on Batch that accepts the event to dispatch. Have the helper check the container binding, resolve Dispatcher once, verify listeners, and dispatch the event; replace the duplicated BatchStarted, BatchFinished, and BatchCanceled blocks in recordSuccessfulJob and recordFailedJob with calls to this helper while preserving existing conditions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/bus/src/Dispatcher.php`:
- Around line 118-153: Update Dispatcher::bulk() to validate the instance
returned by queueResolver before invoking bulk() on it. Require a valid Queue
implementation and throw the same structured RuntimeException used by
dispatchToQueue() when validation fails, preserving the existing grouped
dispatch behavior for valid queues.
In `@src/core/src/Bootstrap/TaskCallback.php`:
- Around line 18-27: Update the config lookup in TaskCallback::__construct so
config->boolean() for OPTION_TASK_ENABLE_COROUTINE receives false as its default
value, preventing missing settings from producing an exception while preserving
the existing OR logic with taskObject.
---
Nitpick comments:
In `@src/bus/README.md`:
- Line 8: Update the “Differences From Laravel” heading in the README to use
Setext-style Markdown syntax, matching the document’s existing heading
conventions and satisfying markdownlint-cli2.
In `@src/bus/src/Batch.php`:
- Around line 138-153: Extract the repeated dispatcher resolution and event
delivery logic into a private helper on Batch that accepts the event to
dispatch. Have the helper check the container binding, resolve Dispatcher once,
verify listeners, and dispatch the event; replace the duplicated BatchStarted,
BatchFinished, and BatchCanceled blocks in recordSuccessfulJob and
recordFailedJob with calls to this helper while preserving existing conditions.
In `@tests/Core/StdoutLoggerTest.php`:
- Around line 409-423: Move ThrowingStdoutLoggerStringable and
ThrowingStdoutLoggerJsonSerializable out of the test file into separate files
under Core/Fixtures, preserving their existing namespaces and behavior. Remove
the inline class definitions and import both fixture classes where the test uses
them, matching the existing TestObject fixture pattern.
In `@tests/Testbench/Integrations/DispatchJobTest.php`:
- Around line 16-17: Rename the test method itCanTriggersExpectedJobs to a
grammatically correct name, preferably itCanTriggerExpectedJobs, and update any
references to the method if present.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 9d34b1d9-12e6-4ec7-aa79-dbbb90ca7553
📒 Files selected for processing (77)
docs/plans/2026-07-12-framework-coroutine-state-lifecycle-audit-ledger.mddocs/plans/2026-07-12-framework-coroutine-state-lifecycle-audit.mdsrc/boost/docs/deployment.mdsrc/boost/docs/logging.mdsrc/boost/docs/queues.mdsrc/bus/README.mdsrc/bus/src/Batch.phpsrc/bus/src/BatchRepository.phpsrc/bus/src/Batchable.phpsrc/bus/src/BusServiceProvider.phpsrc/bus/src/DatabaseBatchRepository.phpsrc/bus/src/DebounceLock.phpsrc/bus/src/Dispatcher.phpsrc/bus/src/Events/BatchCanceled.phpsrc/bus/src/Events/BatchStarted.phpsrc/bus/src/PendingBatch.phpsrc/bus/src/UniqueJobPayloadContext.phpsrc/contracts/src/Bus/QueueingDispatcher.phpsrc/contracts/src/Queue/PreparesForDispatch.phpsrc/core/README.mdsrc/core/composer.jsonsrc/core/src/Bootstrap/ServerStartCallback.phpsrc/core/src/Bootstrap/TaskCallback.phpsrc/core/src/Bootstrap/WorkerStartCallback.phpsrc/core/src/Events/OnReceive.phpsrc/core/src/Exceptions/NotImplementedException.phpsrc/core/src/Logger/ConsoleLogger.phpsrc/core/src/Logger/StdoutLogger.phpsrc/foundation/composer.jsonsrc/foundation/config/app.phpsrc/foundation/config/server.phpsrc/foundation/src/Bus/PendingDispatch.phpsrc/foundation/src/Queue/InteractsWithUniqueJobs.phpsrc/horizon/src/Http/Controllers/BatchesController.phpsrc/pipeline/README.mdsrc/pipeline/composer.jsonsrc/pipeline/src/Hub.phpsrc/pipeline/src/PipelineServiceProvider.phpsrc/queue/composer.jsonsrc/queue/src/CallQueuedHandler.phpsrc/queue/src/Console/BatchesTableCommand.phpsrc/queue/src/Queue.phpsrc/server/src/Port.phpsrc/server/src/ServerConfig.phpsrc/support/src/Facades/Bus.phpsrc/support/src/Testing/Fakes/BatchFake.phpsrc/support/src/Testing/Fakes/BatchRepositoryFake.phpsrc/support/src/Testing/Fakes/BusFake.phpsrc/support/src/Testing/Fakes/PendingBatchFake.phpsrc/testing/src/PHPUnit/AfterEachTestSubscriber.phptests/Bus/BusBatchTest.phptests/Bus/BusBatchableTest.phptests/Bus/BusDebounceLockTest.phptests/Bus/BusDispatcherTest.phptests/Bus/BusPendingBatchTest.phptests/Bus/BusPendingDispatchTest.phptests/Bus/UniqueJobPayloadContextTest.phptests/Core/Bootstrap/TaskCallbackTest.phptests/Core/Bootstrap/WorkerStartCallbackTest.phptests/Core/StdoutLoggerTest.phptests/Foundation/FoundationConfigTest.phptests/Integration/Horizon/Controller/BatchesControllerTest.phptests/Integration/Pipeline/PipelineTransactionTest.phptests/Integration/Queue/CallQueuedHandlerTest.phptests/Integration/Queue/DebouncedJobTest.phptests/Integration/Queue/JobDispatchingTest.phptests/Integration/Queue/PreparesForDispatchTest.phptests/Log/ContextQueueTest.phptests/Pipeline/CoroutineIsolationTest.phptests/Pipeline/HubTest.phptests/Pipeline/PipelineFacadeTest.phptests/Pipeline/PipelineTest.phptests/Queue/CallQueuedHandlerTest.phptests/Server/ServerConfigTest.phptests/Support/PendingBatchFakeTest.phptests/Support/SupportTestingBusFakeTest.phptests/Testbench/Integrations/DispatchJobTest.php
💤 Files with no reviewable changes (4)
- src/core/src/Exceptions/NotImplementedException.php
- src/core/src/Bootstrap/ServerStartCallback.php
- src/core/src/Logger/ConsoleLogger.php
- src/foundation/src/Queue/InteractsWithUniqueJobs.php
Group bulk jobs by nested connection and queue dimensions instead of flattening both names through a delimiter. This prevents distinct supported routes from colliding when either name contains a colon while preserving each route's original values and per-route job order. Add a focused regression that proves the previously colliding (a:b, c) and (a, b:c) routes are resolved and dispatched independently. The implementation uses only native array grouping and avoids encoded keys, explicit hashing, or additional ordering machinery.
Rename the touched Bus dispatch integration test to use the grammatically correct method name. This keeps the existing behavior and coverage unchanged while removing a pre-existing typo from a test already modified by the current work unit.
Amend the existing Bus audit finding with the supported colon-bearing route collision, the nested connection and queue grouping decision, and its focused regression coverage. Remove the superseded rejection of bulk-key hashing, document the precise performance and complexity boundary, refresh validation evidence, and record the completed follow-up review without creating a duplicate finding.
Collaborator
Author
|
@greptile-apps @coderabbitai Follow-up is pushed. I made one behavioral correction:
The remaining findings do not warrant code changes:
The focused regressions and the full |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
ast-grep outline src/bus/src/Dispatcher.php --items all --type function --match bulk
ast-grep outline tests/Bus/BusDispatcherTest.php --items all --type function --match 'testDispatchBulk|bulk'
ast-grep outline tests/Testbench/Integrations/DispatchJobTest.php --items all --type function --match 'itCanTrigger'
fd -a '2026-07-12-framework-coroutine-state-lifecycle-audit-ledger.md' docs
printf '\n--- Dispatcher::bulk implementation ---\n'
rg -n -A100 -B8 'function\s+bulk\s*\(' src/bus/src/Dispatcher.php
printf '\n--- bulk regression coverage ---\n'
rg -n -A180 -B8 'function\s+testDispatchBulk\s*\(' tests/Bus/BusDispatcherTest.php
printf '\n--- Testbench method name ---\n'
rg -n -A5 -B3 'function\s+itCanTrigger' tests/Testbench/Integrations/DispatchJobTest.php
printf '\n--- Bus audit follow-up references ---\n'
rg -n -i -A8 -B5 'delimiter|collision|bulk|misrout|nested' docs/plans/2026-07-12-framework-coroutine-state-lifecycle-audit-ledger.mdLength of output: 105396 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- Dispatcher resolver definitions and route resolution ---'
rg -n -A28 -B8 'queueResolver|resolve(Connection|Queue)FromQueueRoute|getAttributeValue' src/bus/src/Dispatcher.php
printf '%s\n' '--- Queue bulk signatures and routing boundaries ---'
rg -n -A24 -B8 'function\s+bulk\s*\(' src/queue src/contracts src/support src/bus -g '*.php'
printf '%s\n' '--- Explicit empty connection/queue handling in relevant production code/tests ---'
rg -n -i -C3 "(onConnection\(''\)|onQueue\(''\)|connection\s*===?\s*''|queue\s*===?\s*''|connection\s*==\s*''|queue\s*==\s*'')" \
src/bus src/queue tests/Bus tests/Queue tests/Integration/Queue -g '*.php'Length of output: 43014
The remaining points are consistent with the implementation boundaries and existing coverage. I did not execute tests in this sandbox, but the source-level follow-up is sound. If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR completes the Pipeline, Bus, and Core lifecycle audit work units.
The common thread is ownership in a long-lived Swoole worker. Mutable builders must not become worker singletons, delayed payload metadata must stay attached to the exact job that owns it, batch state must tolerate concurrent deletion, and native callback configuration must match the signatures Hypervel actually registers.
Pipeline
Pipeline is a mutable per-operation builder. Resolving the concrete class previously placed it in Hypervel's auto-singleton cache, so independently resolved builders could overwrite each other's passable, pipes, transaction, and finalizer state across coroutines.
This change:
Pipeline::classand the canonicalpipelinekey through one transient factory;"0"name;withinTransaction();The only new cost is the fresh allocation required when callers directly resolve the mutable concrete builder. No context lookup, lock, clone, registry, or per-dispatch Bus workaround was added.
Bus
The Bus work updates the package to current Laravel behavior while correcting worker-lifetime and delayed-serialization defects that do not exist in a request-per-process runtime.
Dispatcher and fake behavior now includes:
QueueingDispatcheralias resolution;bool|voidpreparation contracts;afterResponse(bool)selection;Batch handling now:
"0"values;Unique-job payload metadata no longer lives in a request-wide bracket that can be stranded by preparation, lock, serialization, or dispatch failures. A lazy
WeakMapsidecar now associates acquired-lock metadata with the exact job object and consumes it only when that object's queue payload is created. This keeps after-response and after-commit serialization correct, lets abandoned jobs release metadata naturally, and scopes Laravel interoperability keys to the payload that owns them.Debounce handling uses one owner read, atomically creates the original max-wait anchor, and skips optional event work when no listener exists. The atomic first write affects only anchor creation and removes a cache read from the normal owner path.
Intentional Hypervel differences are documented: DynamoDB batch storage is unsupported, and the mutable database batch connection setter is omitted because it is unsafe on a worker singleton. Bulk dispatch documentation also states that bulk jobs bypass preparation, unique-job, and debounce lifecycles.
Core
Core now treats worker bootstrap and native Swoole callback settings as explicit lifecycle boundaries.
The low-level stdout logger:
Task callbacks now select the exact Swoole signature from
task_enable_coroutine,task_object, and the presence-sensitivetask_use_objectalias, then finish through the native owner for that mode. The shipped coroutine-task default is explicit.Truthy
event_objectsettings are rejected at both global and per-port mutation boundaries. That option changes numerous native callback signatures and is incompatible with Hypervel's positional lifecycle bridge. Explicitfalseremains valid, and per-port failures identify the offending port.The receive event now exposes its native string payload. Three unused Hyperf-era public classes were removed, Core declares its direct Swoole requirement, the false Coroutine dependency is gone, and package provenance and user documentation are current.
Performance and complexity
The changes keep new work at the boundaries that own it:
WeakMaplookup occurs only while unique registrations exist.There are no new hot-path locks, retries, polling loops, container lookups, coroutine context registries, wrapper jobs, or retained request objects.
Validation
The full repository quality gate passes, including formatting, both PHPStan configurations, the parallel component suite, the Testbench package contract suite, and the dogfood package suite. Focused regression coverage exercises deterministic coroutine isolation, delayed payload ownership, batch deletion races, debounce atomicity, worker configuration reloads, native callback modes, failure cleanup, fakes, contracts, and documentation-facing behavior.
Composer metadata validation, stale-reference scans, package checklist parity, and diff integrity checks also pass.
Summary by CodeRabbit
New Features
Bug Fixes
event_objectsettings.Documentation