Skip to content

Harden coroutine runtime and resource lifecycle - #440

Merged
binaryfire merged 28 commits into
0.4from
audit/coroutine-runtime-lifecycle
Jul 17, 2026
Merged

Harden coroutine runtime and resource lifecycle#440
binaryfire merged 28 commits into
0.4from
audit/coroutine-runtime-lifecycle

Conversation

@binaryfire

@binaryfire binaryfire commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator

Summary

This change hardens the low-level runtime used for coroutine context, task creation, process concurrency, Engine I/O, coordinator timers, and pool teardown.

The main changes are:

  • make coroutine context snapshots transactional and explicitly targetable;
  • roll back coroutine bookkeeping when task creation fails;
  • preserve lock, waiter, caller, and filesystem ownership across exceptional paths;
  • remove Engine portability code for server implementations Hypervel does not support;
  • correct stream, HTTP/2, and WebSocket boundary behavior;
  • make process result transport binary-safe and exception reconstruction failure-safe;
  • release cleared Coordinator timers and Pool-owned frequency timers deterministically;
  • remove destructor cleanup that could never run while timer callbacks retained their owners.

For more details, see: docs/plans/2026-07-12-framework-coroutine-state-lifecycle-audit-ledger.md

Context and coroutine lifecycle

CoroutineContext now captures a complete snapshot before spawning or installing it elsewhere. Replication failure cannot leave partially copied state in a child, and callers can capture selected keys from the current coroutine or an explicit source coroutine through one documented API.

Coroutine creation now treats bookkeeping as a transaction. Capacity tokens, waiter counts, caller replacements, mutex ownership, and forked-task tracking are restored when native creation fails. Forked tasks remain visible to their owner until all scheduled work is complete.

Filesystem locks now preserve coroutine ownership instead of allowing another coroutine sharing the same wrapper to unlock or replace an active lock.

Engine

The Engine package is now explicitly Swoole-only. The removed HTTP server factories, response emitter abstractions, and multi-engine stream wrappers were portability carryover from Hyperf and had no valid Hypervel consumer.

The remaining Swoole boundaries now:

  • preserve PHP stream read, seek, EOF, and close semantics;
  • reject failed HTTP/2 connections before publishing a client;
  • keep WebSocket frame and connection state truthful after native failures;
  • avoid stale or duplicated HTTP server state;
  • retain only contracts used by the supported runtime.

This reduces the Engine surface while fixing real native-boundary behavior.

Process concurrency

Process results are transported without losing binary serialized payloads. Decoders reject malformed envelopes, tolerate appended gzip output where the subprocess protocol permits it, and contain parent-only class loading or constructor failures.

Exception reconstruction now preserves supported falsey and typed constructor values without reading inaccessible object state. Exceptions that cannot be reconstructed safely degrade to a RuntimeException carrying the original message instead of causing a second parent-side failure.

The public timeout option is carried through the concurrency contract and facade for Laravel parity. The Process driver applies it; Coroutine and Sync retain their existing execution behavior.

Coordinator and Pool teardown

Coordinator timers retain the exact coroutine ID they own and distinguish framework-owned waits from callbacks that are already running. Clearing a timer releases a coroutine blocked in Coordinator without injecting cancellation into user callback code.

Pool shutdown has a narrow optional capability for frequency strategies that own resources. ConstantFrequency uses its existing clear() method, and cleanup failures are reported without preventing channel closure or idle connection teardown.

The destructors on ConstantFrequency, KeepaliveConnection, DbPool, and RedisPool were removed. Their callbacks retain the owning object while live, so those destructors could not provide cleanup. Explicit close() paths are now the sole lifecycle owners.

Compatibility and performance

Laravel-facing APIs and existing call sites remain compatible. This restores Laravel process-timeout parity and adds documented Hypervel-specific context and teardown capabilities.

The removed Engine APIs were unused Hypervel-specific multi-server abstractions. Hypervel supports Swoole only.

There is no new normal request or job overhead. Added work is limited to the operation that needs it:

  • context copying allocates only when a snapshot is requested;
  • process encoding and reconstruction run only for explicit process concurrency;
  • timer registration retains two small bookkeeping entries;
  • Pool performs one capability check during explicit shutdown;
  • database and Redis destructor changes are net deletions.

No polling loop, retry framework, generic cancellation registry, reflection-based lifecycle detection, or per-timer channel was introduced.

Testing

The regression coverage exercises:

  • context replication failure and explicit source capture;
  • coroutine creation rollback across every affected primitive;
  • lock ownership and waiter completion;
  • Engine stream, HTTP/2, and WebSocket failures;
  • binary process results, malformed envelopes, gzip suffixes, reporting failures, and exception reconstruction;
  • Coordinator clear, clear-all, re-entry, self-clear, callback-yield, and spawn-failure schedules;
  • Pool timer release and exhaustive shutdown after frequency cleanup failure;
  • explicit heartbeat teardown in Pool, Database, and Redis.

The full formatter, static-analysis, parallel test, Testbench contract, and Testbench dogfood gates pass.

Summary by CodeRabbit

  • New Features
    • Added per-task timeouts for concurrency operations via the process driver (other drivers accept the parameter for compatibility).
    • Improved coroutine context snapshot/capture and isolation for forked/concurrent tasks.
    • Enhanced process task result transport, including lossless binary payload support.
  • Bug Fixes
    • Improved WebSocket upgrade, messaging, cleanup, and stream detachment behavior.
    • Ensured timers/pools shut down and clear deterministically to prevent lingering state.
  • Documentation
    • Updated concurrency and coroutine context guidance, including timeout behavior.
  • Breaking Changes
    • Removed/changed several engine HTTP/response and WebSocket frame APIs—update integrations accordingly.
  • Tests
    • Expanded lifecycle, timeout, and context replication robustness coverage.

Model stream detachment explicitly so size, readability, and writability remain truthful after the underlying contents are released.

Reject negative reads without mutating the buffer, preserve zero-length reads, maintain the remaining size directly, and return PSR-compatible metadata results instead of throwing an unrelated implementation exception.

Add focused regressions for size tracking, invalid reads, metadata, and every operation that must fail after detachment.
Check the native Swoole HTTP/2 connect result during client construction and convert a false result into the existing HttpClientException contract with the native error details.

This prevents an unusable client from escaping construction and failing later at a less useful boundary. Add a deterministic regression covering immediate connection refusal.
Return native Swoole push results instead of reporting unconditional success, reject failed upgrades at construction, and clear connection-local callbacks and handles through one exception-safe lifecycle boundary.

Simplify Frame and its contract to represent only capabilities Swoole actually supports: compute payload length from the payload, expose the native boolean mask flag, remove the ineffective masking-key and payload-length mutators, and drop the ignored serialization argument.

Expand regressions to cover both native push branches, upgrade failure, cleanup after callback failure, computed payload length, immutable mask changes, and the mask bit emitted on the wire.
Delete the unused HTTP server, factory, response emitter, event stream, and their contracts that were inherited from Hyperf's interchangeable Swoole and Swow architecture.

Hypervel permanently targets Swoole, and its live HTTP, WebSocket, and Reverb servers already use the canonical Swoole request and response bridges in hypervel/http-server. Remove the dead provider binding, direct package dependencies, obsolete regression, and lifecycle-plan claims that applied only to the deleted server.

Document Engine's upstream origin and the intentional Swoole-only architecture so future ports do not restore the portability layer.
Document the verified Engine defects, the owner-approved Swoole-only simplification, rejected speculative complexity, implementation boundaries, regression coverage, validation, API impact, and performance assessment.

Mark Engine complete in the framework-wide audit and advance the active package to Coroutine. No cross-package revalidation remains from this work unit.
Introduce a keys-first captureFrom API that snapshots and replicates context in the calling coroutine before any destination is modified. Keep copyFrom compatible while routing installation through setMany so explicit destinations remain isolated from non-coroutine fallback storage.

Add direct coverage for current, filtered, explicit, dead, and non-coroutine source behavior, and prove that replication failures cannot partially overwrite the destination. Refresh the log context comments to describe the generalized replication boundary.
Capture parent context synchronously before spawning so replication failures remain caller-visible and concurrency capacity or wait counts can roll back cleanly. Remove duplicate Concurrent exception reporting and make the high-level coroutine reporter resilient when container or handler infrastructure also fails.

Correct run() callable-array handling and restore the exact prior Swoole hook flags in a finally block. Add deterministic regressions for reporting failure, context replication failure, balanced Concurrent and Parallel bookkeeping, callable tuples, and hook restoration.
Construct the replacement instance and its channel before publishing either one. A failed factory now leaves the previously published caller intact instead of closing healthy state before a replacement exists.

Cover both successful replacement cleanup and factory failure so the channel transaction cannot regress.
Remove Locker and Mutex entries when their channels are released instead of retaining null tombstones for every dynamic key. Narrow the maps to their truthful channel-only type and keep flushState responsible for closing only live channels.

Add regressions that inspect released-key removal and prevent unbounded worker-lifetime map growth from returning.
Give WaitConcurrent::fork() the same wait-group ownership contract as create(): reserve before spawning, finish in the child finally block, and roll the reservation back when synchronous context capture or coroutine creation fails.

Cover ordinary fork completion and synchronous failure so wait() cannot return while forked work remains active or hang on a stranded count.
Record whether each atomic filesystem boundary actually acquired its coroutine lock and release it only in that case. Cancellation while waiting can no longer unlock another coroutine's live critical section.

Exercise both Filesystem and LockableFile with deterministic owner/waiter scheduling to prove canceled waiters never release a gate they do not own.
Snapshot and replicate the current context before allocating the result channel or creating the child coroutine. Replication failures now surface immediately instead of being reported in a child and misdiagnosed as a waiter timeout.

Keep context installation inside the child and add a focused regression proving the original replication exception reaches the caller.
Treat an absent source descriptor as a no-op when copying WebSocket context, matching the framework coroutine-context contract instead of reading an unchecked storage offset.

Add a regression that preserves the destination context when the requested source has already disappeared.
Document captureFrom as the low-level keys-first API for taking a replicated coroutine context snapshot before a child exists. Clarify current and explicit source selection, transfer-only semantics, and the higher-level fork and copyContext alternatives.

Update the general context guide to describe invocation-time snapshots accurately and record the Coroutine package's Hyperf upstream reference.
Capture the verified coroutine, context, lock-ownership, Waiter, and WebSocket findings with their final implementation, regression coverage, performance assessment, and review outcome.

Mark the Coroutine package complete, route the next audit slice to Concurrency, and retain the cross-package revalidation links needed by the remaining package audits.
Encode successful serialized values as base64 so arbitrary binary results remain valid JSON across process boundaries.

Build failure envelopes from structurally reconstructible constructor state, preserve falsey and floating values, contain reporting and encoding failures, and fall back to a stable RuntimeException response when the original exception cannot be transported safely.

Add focused coverage for binary results, malformed state, constructor shapes, UTF-8 failures, reporter failures, and exact environment restoration.
Decode process responses with strict JSON and base64 handling, ignore appended gzip output, reconstruct transported failures from named constructor state, and contain invalid or non-Throwable parent-side classes as RuntimeException.

Restore the current Laravel optional timeout contract across the facade, driver contract, and all implementations while applying the timeout only to process tasks, matching Laravel behavior.

Cover binary results, malformed transport, reconstruction failures, falsey constructor values, timeout propagation, and unchanged coroutine and sync semantics.
Keep Testbench remote-closure decoding aligned with the framework process protocol: trim appended gzip bytes, decode JSON and base64 strictly, reconstruct named exception state, and contain invalid parent-side classes without masking the transported message.

Document the full throwable boundary and add direct decoder coverage plus a real subprocess round trip for binary values.
Describe process-only timeouts and the exact copied-context contract, including shared ordinary object references and ReplicableContext behavior.

Record the package provenance and material Hypervel differences, and mark Laravel fork-driver support as intentionally omitted because Swoole coroutines are the native execution model.
Capture the verified transport, reconstruction, timeout, documentation, and test-isolation findings together with the rejected complexity, owner-approved costs, validation, and final review outcome.

Mark Concurrency complete, retain Foundation and Testbench revalidation dependencies, and route the next package audit to Coordinator.
Track the coroutine owned by each timer registration and distinguish framework-owned coordinator waits from user callbacks that are actively running or yielding. Clearing a timer can now release a blocked coroutine immediately without injecting cancellation into callback code.

Guard coroutine ID publication against synchronous completion and creation-hook re-entry, preserve timer statistics across every exit, and declare Coordinator's direct Coroutine and PSR logger dependencies. Record the package's Hyperf provenance and the intentional clear behavior at the source boundary.

Make coordinator concurrency tests failure-safe by signaling wait groups in finally blocks and asserting from the parent. Add deterministic regressions for clearing one or all timers, callback-yield safety, re-entry, self-clear, and spawn rollback, and update heartbeat tests for the new registration bookkeeping.
Introduce a narrow optional capability for frequency strategies that own resources and have Pool::close() invoke it before closing the channel and draining idle connections. ConstantFrequency participates through its existing clear method, so its timer and captured pool graph are released at the supported deterministic lifecycle boundary.

Contain and report custom strategy cleanup failures through the existing pool reporter while continuing independent channel and connection teardown. This prevents an idempotent close from becoming permanently partial without expanding the general low-frequency contract or affecting borrow and release paths.

Add regressions proving that closing a ConstantFrequency-backed pool releases its timer and stops later ticks, and that a throwing custom cleanup cannot prevent channel closure, connection cleanup, or bookkeeping reset.
Delete destructor cleanup from KeepaliveConnection, DbPool, and RedisPool. Their heartbeat callbacks retain the owning object while a timer is live, so the destructors cannot run when cleanup would be useful and only repeat a no-op after deterministic teardown has already completed.

Keep explicit close paths as the sole resource owners: connection close clears keepalive state, while database and Redis pool close operations clear their heartbeat timers and their callbacks also stop during worker shutdown. No replacement garbage-collection machinery is necessary.
Document the verified Coordinator and Pool lifecycle findings, implemented ownership boundaries, rejected overengineered alternatives, regression coverage, performance and compatibility impact, validation, and final assessment.

Mark Coordinator complete, route the next work unit to Signal, and carry the Pool, Database, and Redis teardown findings into the cross-package dependency index for revalidation during their later full audits.
@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: cf8ea065-4f30-4eeb-9f71-b1ee06e3428c

📥 Commits

Reviewing files that changed from the base of the PR and between 405d9f3 and 9032de0.

📒 Files selected for processing (7)
  • docs/plans/2026-07-10-object-pool-lifecycle-and-client-pooled-filesystems.md
  • docs/plans/2026-07-12-framework-coroutine-state-lifecycle-audit-ledger.md
  • docs/plans/2026-07-15-framework-enum-identifier-contracts.md
  • src/concurrency/src/ProcessDriver.php
  • src/testbench/src/Foundation/Process/ProcessResult.php
  • tests/Integration/Concurrency/ConcurrencyTest.php
  • tests/Testbench/Foundation/Process/ProcessResultTest.php
🚧 Files skipped from review as they are similar to previous changes (1)
  • tests/Testbench/Foundation/Process/ProcessResultTest.php

📝 Walkthrough

Walkthrough

This PR updates coroutine context capture and failure handling, process-driver transport and exception reconstruction, Swoole stream/WebSocket behavior, timer and pool teardown, filesystem lock ownership, and associated tests and documentation. It also removes obsolete HTTP portability and native HTTP overload surfaces.

Changes

Runtime lifecycle and transport

Layer / File(s) Summary
Engine and WebSocket behavior
src/contracts/Engine/..., src/engine/..., tests/Engine/*
Stream detachment, metadata, WebSocket masking, upgrade failure, push-result propagation, cleanup, and HTTP/2 connection validation are updated.
Context and coroutine failure safety
src/context/..., src/coroutine/..., src/foundation/src/Testing/..., src/filesystem/...
Context snapshots and replication are transactional, coroutine bookkeeping is balanced on failures, hook flags are restored, exception logging has a fallback, and lock release requires ownership.
Process concurrency transport
src/concurrency/..., src/foundation/src/Console/..., src/testbench/src/Foundation/Process/...
Process tasks accept timeouts; serialized results use strict base64 transport; gzip suffixes are trimmed; malformed payloads and exception reconstruction failures are contained.
Timer and pool ownership
src/coordinator/..., src/pool/..., src/database/..., src/redis/...
Timers track coroutine IDs and cancel waiting coroutines, while pool frequency and heartbeat cleanup move to explicit shutdown paths.
Validation and documentation
tests/..., docs/plans/..., src/boost/docs/...
Regression coverage, package documentation, audit ledgers, and timeout and identifier contract descriptions are updated.

Estimated code review effort: 5 (Critical) | ~120 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant ProcessDriver
  participant Process
  participant ProcessResult
  Caller->>ProcessDriver: run(tasks, timeout)
  ProcessDriver->>Process: apply timeout and execute task
  Process-->>ProcessDriver: JSON envelope with base64 result
  ProcessDriver->>ProcessResult: decode and reconstruct output
  ProcessResult-->>Caller: value or reconstructed Throwable
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 41.73% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main theme: hardening coroutine runtime behavior and lifecycle cleanup across context, timers, concurrency, and engine resources.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch audit/coroutine-runtime-lifecycle

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.

@greptile-apps

greptile-apps Bot commented Jul 17, 2026

Copy link
Copy Markdown

Greptile Summary

This PR hardens the low-level coroutine, process concurrency, engine I/O, coordinator timer, and pool teardown layers across the framework. The scope is wide (89 files) but the changes are logically coherent: each surface area gets a tightly targeted fix.

  • Coroutine lifecycle: CoroutineContext gains a transactional snapshot API (captureFrom) used by Coroutine::fork() to capture parent context before spawning, eliminating the window where the parent could be gone when copyFrom was called in the child. Filesystem and LockableFile guard against spurious unlocks via a $locked flag. Caller::initInstance eliminates the null window between close and new-channel assignment. Timer clear() calls cancelById only when the timer is blocked in a coordinator wait, and clearAll iterates a key snapshot.
  • Process concurrency: InvokeSerializedClosureCommand base64-encodes results and adds a JSON round-trip guard for exception parameters. ProcessDriver validates the response envelope, strips appended gzip bytes, and reconstructs exceptions with a try-catch fallback. Timeout is threaded through the Driver contract and facade.
  • Engine and pool teardown: WebSocket::upgrade() failure now throws, cleanup moved to finally. HTTP/2 connect() failure throws before publishing a client. ConstantFrequency, DbPool, RedisPool, and KeepaliveConnection destructors removed (the timer callback holds $this, so they could never fire). Pool::close() calls ClearableFrequencyInterface::clear() with error reporting before draining the channel.

Confidence Score: 5/5

Safe to merge — each fix is backed by a test and the rollback/teardown paths are carefully sequenced.

No correctness regressions identified across the reviewed surfaces. Coroutine context capture is now transactional and validated by new tests. Process result decoding is stricter and fails gracefully. Timer cancellation correctly distinguishes the waiting state from a running callback. Destructor removal is justified by object-retention semantics. The only maintenance concern (duplicated decode logic across ProcessDriver and testbench ProcessResult) does not affect runtime correctness.

src/testbench/src/Foundation/Process/ProcessResult.php contains a near-verbatim copy of the decode logic in ProcessDriver; the two should stay in sync.

Important Files Changed

Filename Overview
src/concurrency/src/ProcessDriver.php Adds optional timeout threading through to process pool; replaces fragile JSON decode with validated envelope parsing, gzip-suffix stripping, base64/unserialize decode, and safe exception reconstruction with try-catch fallback.
src/foundation/src/Console/InvokeSerializedClosureCommand.php Result now base64-encoded before serialize; exception encoding uses JSON round-trip check plus UTF-8 class-name guard with two fallback layers, all writing to the authoritative response envelope.
src/coordinator/src/Timer.php Replaces opaque boolean closure-tracking map with coroutine-ID and waiting-state maps; clear() calls cancelById only when the timer is blocked in a coordinator wait; clearAll() iterates a snapshot of keys.
src/context/src/CoroutineContext.php Adds captureFrom() for snapshot-at-call-site context capture; setMany() writes directly to ArrayObject context and throws CoroutineDestroyedException when a specific destroyed coroutine ID is targeted.
src/coroutine/src/Coroutine.php fork() now captures context in the parent via captureFrom() before spawning, eliminating the window where the parent could be gone when copyFrom was called inside the child; printLog wraps container access in try-catch.
src/engine/src/Http/Stream.php Replaces writable flag with detached flag; read/write/getContents/isReadable/isWritable now throw on detached streams; getMetadata returns empty array instead of throwing BadMethodCallException.
src/engine/src/WebSocket/WebSocket.php upgrade() failure now throws instead of silently continuing; start() loop cleanup moved from end-of-loop to a finally block so it runs even when recv() throws.
src/pool/src/Pool.php close() now calls ClearableFrequencyInterface::clear() (with error reporting) before closing the channel, so frequency-owned timers are torn down before idle connection draining.
src/coroutine/src/Channel/Caller.php initInstance() creates and populates the new channel before closing the old one, eliminating the null window between close and assignment.
src/testbench/src/Foundation/Process/ProcessResult.php Mirrors ProcessDriver's gzip-strip, envelope validation, base64 decode, and safe exception reconstruction; identical decode block is duplicated in both files with no shared utility.
src/pool/src/ConstantFrequency.php Implements ClearableFrequencyInterface; __destruct removed because the timer callback holds a reference to $this, preventing GC-triggered cleanup.
src/coroutine/src/Concurrent.php Exception reporting removed from create/fork inner try-catch; exceptions now propagate to Coroutine::create()'s own printLog handler, eliminating duplicated reporting.
src/coroutine/src/functions.php run() saves and restores previous hook flags in a finally block instead of always resetting to 0; single-callable and array-callable paths dispatched explicitly.
src/engine/src/Http/V2/Client.php connect() return value now checked; false result throws HttpClientException before any caller can use a failed connection.
src/filesystem/src/Filesystem.php atomic() tracks whether Locker::lock() succeeded before attempting unlock in the finally block, preventing a spurious unlock on exception before the first lock acquisition.

Reviews (2): Last reviewed commit: "Repair inline types in Markdown tables" | Re-trigger Greptile

Comment thread src/engine/src/Http/Stream.php
Comment thread src/foundation/src/Console/InvokeSerializedClosureCommand.php

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🤖 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 `@docs/plans/2026-07-12-framework-coroutine-state-lifecycle-audit-ledger.md`:
- Line 617: Update the concurrency-04 table cell to escape or encode the pipes
in “CarbonInterval|int|null” so Markdown treats the union type as one cell and
MD056 passes; preserve the documented timeout API text and all other table
columns.

In `@src/concurrency/src/ProcessDriver.php`:
- Around line 72-101: Update the payload handling around the result-processing
logic to require a boolean successful field before branching, and validate that
the envelope contains the expected result data for successful responses.
Deserialize the decoded result with failure detection enabled, rejecting invalid
serialized data instead of treating unserialize() returning false as a
legitimate result while preserving valid false results.

In `@src/coordinator/README.md`:
- Line 6: Update src/coordinator/README.md:6 to add a “Differences From Laravel”
section explaining that Laravel has no direct coordinator equivalent and
documenting Hypervel’s coordinator behavior. Update src/coroutine/README.md:6
with the same required section, describing Hypervel’s Swoole coroutine model and
how it differs from Laravel.

In `@src/engine/README.md`:
- Around line 8-10: Move the Swoole-only portability note from the
“Architecture” section into a section headed exactly “Differences From Laravel”
in the README, preserving its existing content and meaning.
🪄 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: 331fe680-275c-4755-a2be-955574e3f106

📥 Commits

Reviewing files that changed from the base of the PR and between d6db19d and 405d9f3.

📒 Files selected for processing (87)
  • docs/plans/2026-07-12-framework-coroutine-state-lifecycle-audit-ledger.md
  • docs/plans/2026-07-12-framework-coroutine-state-lifecycle-audit.md
  • docs/plans/2026-07-12-test-suite-lifecycle-and-concurrency-robustness.md
  • src/boost/docs/concurrency.md
  • src/boost/docs/context.md
  • src/boost/docs/coroutine-context.md
  • src/concurrency/README.md
  • src/concurrency/src/ConcurrencyManager.php
  • src/concurrency/src/CoroutineDriver.php
  • src/concurrency/src/ProcessDriver.php
  • src/concurrency/src/SyncDriver.php
  • src/context/src/CoroutineContext.php
  • src/contracts/src/Concurrency/Driver.php
  • src/contracts/src/Engine/Http/ServerFactoryInterface.php
  • src/contracts/src/Engine/Http/ServerInterface.php
  • src/contracts/src/Engine/ResponseEmitterInterface.php
  • src/contracts/src/Engine/WebSocket/FrameInterface.php
  • src/coordinator/README.md
  • src/coordinator/composer.json
  • src/coordinator/src/Timer.php
  • src/coroutine/README.md
  • src/coroutine/src/Channel/Caller.php
  • src/coroutine/src/Concurrent.php
  • src/coroutine/src/Coroutine.php
  • src/coroutine/src/Locker.php
  • src/coroutine/src/Mutex.php
  • src/coroutine/src/WaitConcurrent.php
  • src/coroutine/src/functions.php
  • src/database/src/Pool/DbPool.php
  • src/engine/README.md
  • src/engine/composer.json
  • src/engine/src/EngineServiceProvider.php
  • src/engine/src/Http/EventStream.php
  • src/engine/src/Http/Server.php
  • src/engine/src/Http/ServerFactory.php
  • src/engine/src/Http/Stream.php
  • src/engine/src/Http/V2/Client.php
  • src/engine/src/ResponseEmitter.php
  • src/engine/src/WebSocket/Frame.php
  • src/engine/src/WebSocket/Response.php
  • src/engine/src/WebSocket/WebSocket.php
  • src/filesystem/src/Filesystem.php
  • src/filesystem/src/LockableFile.php
  • src/foundation/src/Console/InvokeSerializedClosureCommand.php
  • src/foundation/src/Testing/Coroutine/Waiter.php
  • src/log/src/Context/Repository.php
  • src/pool/src/ClearableFrequencyInterface.php
  • src/pool/src/ConstantFrequency.php
  • src/pool/src/KeepaliveConnection.php
  • src/pool/src/Pool.php
  • src/redis/src/Pool/RedisPool.php
  • src/support/src/Facades/Concurrency.php
  • src/testbench/src/Foundation/Process/ProcessResult.php
  • src/websocket-server/src/Context.php
  • tests/Context/ContextCoroutineTest.php
  • tests/Context/ContextTest.php
  • tests/Context/Fixtures/ThrowingReplicableContext.php
  • tests/Coordinator/CoordinatorManagerTest.php
  • tests/Coordinator/CoordinatorTest.php
  • tests/Coordinator/FunctionTest.php
  • tests/Coordinator/TimerTest.php
  • tests/Coroutine/Channel/CallerTest.php
  • tests/Coroutine/ConcurrentForkTest.php
  • tests/Coroutine/CoroutineCreateFailureTest.php
  • tests/Coroutine/CoroutineNonCoroutineContextTest.php
  • tests/Coroutine/CoroutineTest.php
  • tests/Coroutine/LockerTest.php
  • tests/Coroutine/MutexTest.php
  • tests/Coroutine/ParallelTest.php
  • tests/Coroutine/WaitConcurrentTest.php
  • tests/Engine/Http2ClientTest.php
  • tests/Engine/HttpServerTest.php
  • tests/Engine/StreamTest.php
  • tests/Engine/WebSocketTest.php
  • tests/Filesystem/CoroutineLockOwnershipTest.php
  • tests/Foundation/Console/Fixtures/ConcurrentProcessExceptionFixtures.php
  • tests/Foundation/Console/InvokeSerializedClosureCommandTest.php
  • tests/Foundation/Testing/Coroutine/WaiterTest.php
  • tests/Integration/Concurrency/ConcurrencyTest.php
  • tests/Integration/Database/Sqlite/DbPoolHeartbeatTest.php
  • tests/Log/ContextCoroutineTest.php
  • tests/Pool/HeartbeatConnectionTest.php
  • tests/Pool/PoolTest.php
  • tests/Redis/RedisPoolHeartbeatTest.php
  • tests/Testbench/Foundation/Process/ProcessResultTest.php
  • tests/Testbench/Foundation/Process/RemoteCommandTest.php
  • tests/WebSocketServer/ContextTest.php
💤 Files with no reviewable changes (14)
  • src/contracts/src/Engine/Http/ServerFactoryInterface.php
  • src/engine/src/Http/Server.php
  • src/contracts/src/Engine/Http/ServerInterface.php
  • src/engine/src/EngineServiceProvider.php
  • src/engine/composer.json
  • src/engine/src/Http/ServerFactory.php
  • src/contracts/src/Engine/ResponseEmitterInterface.php
  • tests/Engine/HttpServerTest.php
  • src/engine/src/Http/EventStream.php
  • src/pool/src/KeepaliveConnection.php
  • src/engine/src/ResponseEmitter.php
  • src/database/src/Pool/DbPool.php
  • src/redis/src/Pool/RedisPool.php
  • src/coroutine/src/Concurrent.php

Comment thread docs/plans/2026-07-12-framework-coroutine-state-lifecycle-audit-ledger.md Outdated
Comment thread src/concurrency/src/ProcessDriver.php Outdated
Comment thread src/coordinator/README.md
Comment thread src/engine/README.md
Validate the decoded response shape before either process consumer reads its success state or reconstructs a remote exception. Reject malformed failure metadata with stable consumer-specific errors instead of allowing protocol corruption to surface as unrelated type failures.

Check unserialize failures at the native boundary so valid-base64 garbage cannot masquerade as a legitimate false result. Preserve real serialized false values and the existing arbitrary-object result contract.

Add mirrored Concurrency and Testbench regressions for invalid envelope shapes, malformed failure fields, corrupt serialized bytes, and valid false round trips.
Escape literal union-type pipes in every affected plan table so GitHub's GFM table parser keeps each declaration in its intended cell. The escapes are source-only and render as normal pipe characters inside code spans.

Apply the correction consistently across the object-pool plan, framework audit ledger, and enum-identifier plan rather than leaving the same rendering defect in adjacent rows.
@binaryfire

Copy link
Copy Markdown
Collaborator Author

@greptileai @coderabbitai Thanks for the reviews. I checked every inline and summary finding against the current contracts, upstream implementations, and call paths.

Two corrections landed:

  • Both serialized-process response consumers now validate envelope structure and failure metadata before use. Corrupt serialized results fail cleanly, a legitimate false result still round-trips, and arbitrary object results remain supported by design. Concurrency and Testbench have mirrored regressions.
  • All eleven affected plan-table rows now escape union-type pipes. This was verified against the GitHub GFM renderer, which previously truncated those cells at the first literal pipe.

The remaining suggestions were intentionally left unchanged:

  • Stream::eof() after detach continues to throw. That matches current Guzzle and the PSR-7 detached-stream contract; PSR-7 does not require a detached stream to report EOF.
  • Exception parameter transport keeps its strict JSON identity gate. Reconstructing the original class after objects, enums, nested values, or binary state change type would risk a parent-side type error or silently different exception semantics.
  • Coordinator and Coroutine do not get Differences From Laravel sections because they are Hyperf-derived infrastructure with no Laravel counterparts.
  • Engine keeps the Architecture heading because the note describes removal of Hyperf multi-engine portability, not a Laravel API difference.
  • The generic docstring-coverage threshold was not applied. The changed code follows the repository docblock conventions, and adding inventory-style comments without a concrete documentation gap would create noise.

composer fix is green, including formatting, static analysis, the full parallel suite, Testbench, and package-mode dogfood.

@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

@binaryfire Thanks for the thorough validation and follow-up.

The serialized-process envelope hardening and escaped GFM table pipes address the reported issues. The remaining decisions are also reasonable given the detached-stream contract, exception reconstruction safety requirements, package provenance, and existing documentation conventions.

🐇 ✅

@greptile-apps

greptile-apps Bot commented Jul 17, 2026

Copy link
Copy Markdown

The explanations are clear and well-grounded. A few notes:

Stream::eof() after detach — Acknowledged. My original comment incorrectly attributed return true behavior to Guzzle; if current Guzzle throws on detached eof(), then Hypervel matches the reference implementation rather than deviating from it. The StreamTest coverage makes this unambiguous.

Envelope validation — The hardened consumers with boolean successful check, failure metadata validation, and the serialize(false) identity guard for the unserialize path are exactly right. The mirrored regressions in Concurrency and Testbench are good.

Exception parameter JSON identity gate — The strict gate is the correct choice here. Once json_decode has changed objects to arrays, enums to scalars, or altered nested state, you cannot safely reconstruct the original class. Degrading to RuntimeException with the original message is safer than surfacing a TypeError or silently different semantics in the parent process.

Coordinator/Coroutine Differences From Laravel — The reasoning is sound. A section whose content is only "Laravel has no equivalent" adds boilerplate without useful compatibility guidance, and the Hyperf provenance is already documented.

Engine Architecture heading — Agreed. The note is about removing Hyperf's Swoole/Swow portability layer, not adapting a Laravel API; "Architecture" is the more accurate classification.

The two actionable findings are addressed and the intentional decisions are well-justified. LGTM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant