Skip to content

Harden process, server process, and filesystem lifecycles#442

Merged
binaryfire merged 31 commits into
0.4from
audit/process-and-filesystem-lifecycles
Jul 20, 2026
Merged

Harden process, server process, and filesystem lifecycles#442
binaryfire merged 31 commits into
0.4from
audit/process-and-filesystem-lifecycles

Conversation

@binaryfire

@binaryfire binaryfire commented Jul 20, 2026

Copy link
Copy Markdown
Collaborator

Summary

This PR completes the lifecycle work across Process, Server Process, and Filesystem. These packages meet at several native boundaries—child processes, Swoole process handles, file descriptors, locks, remote streams, and HTTP response finalization—where ambiguous ownership can become a leak, a hang, or retained memory in a long-lived worker.

The implementation makes those ownership boundaries explicit. Creation either commits completely or rolls back. Cleanup runs exhaustively while preserving the earliest failure. Lazy resources remain owned until their actual terminal event. Native false results are handled at the boundary instead of escaping into stricter PHP contracts.

Process

Process fakes now preserve every supported result value, including falsy values, and empty pipelines fail before execution. Real and fake invoked-process callbacks share the same callable normalization and terminal-failure behavior, so a callback exception cannot leave the process in a misleading state.

Invoked processes and process pools now complete callback and teardown paths consistently. The public contracts, facade surface, tests, README provenance, and application guide were updated with the implementation. The repository guidance also now records the workflow for incremental upstream updates: originating pull requests establish the complete change surface, while the current upstream branch remains the source of truth for the code and tests being ported.

Server processes

Custom server processes are now configured through server.processes and resolved through the container. This keeps the public shape familiar without importing Laravel's unrelated Process package concepts or retaining Hyperf-specific configuration baggage. The package remains intentionally Swoole-only.

Native process construction, listener setup, socket export, IPC payload handling, and teardown now have explicit success and failure paths. Partially created processes are cleaned up, transferred sockets are exported once, and listener/channel ownership is released deterministically. Dead abstractions and an unused exception path were removed rather than preserved behind compatibility shims.

A new Boost guide documents how to define, register, configure, and communicate with a server process, including the process boundary and serialization constraints that matter to application authors.

Filesystem and streaming

Local filesystem operations now check the native operation that owns the result. This covers reads, writes, replacement, links, deletion, metadata, MIME detection, permissions, temporary files, and stream closure. Partial and zero-byte writes are handled explicitly, and multi-step cleanup preserves the first failure without skipping later independent work.

Filesystem locking now uses checked native locks directly. Blocking acquisition uses an adaptive 1–100 ms Swoole-friendly wait instead of fixed worker-local polling, reducing wakeups during sustained contention while retaining immediate LOCK_NB behavior. The tradeoff is bounded and explicit: a lone waiter behind a long-held lock may observe release up to roughly 100 ms later.

S3 and Google Cloud Storage reads are lazy by default. Their stream paths use Guzzle's PHP stream handler under Swoole's hooked I/O, which keeps first-byte delivery prompt and memory bounded without adding a custom cURL/channel bridge. Ordinary read and get behavior is unchanged. Applications can set stream_reads to false when connection reuse is more important than lazy delivery; lazy streams use Connection: close, so workloads dominated by many small streamed objects may incur additional connection setup.

HTTP iterable and event streams now write chunks directly through the Swoole response bridge and stop their producer as soon as the client write fails. Ordinary streamed callbacks retain Symfony's output-buffer behavior, with failure-safe buffer cleanup. Request finalization was made exhaustive across response sending, termination callbacks, middleware, request events, duration handlers, and coroutine context removal. Already committed streamed responses remain committed when later finalization fails.

The adapter, scoped, and pooled filesystem surfaces were synchronized with the current public behavior, including assertEmpty, safe append/prepend semantics, complete JSON values, HTTP file support, truthful metadata, and encoded signed-route path segments. Stream leases are released by explicit close ownership rather than destructor timing.

Compatibility and cost

The Laravel-facing Process and Filesystem APIs remain compatible. Server Process is Hypervel's Swoole-specific package, but its configuration and application-facing usage are now Laravel-shaped. No abstraction for unsupported servers was added.

The ordinary non-streaming request path and ordinary cloud reads do not gain new locks, retries, registries, or allocations. The lock backoff reduces polling work under contention, lazy remote streams bound memory, and single socket export removes repeated native work. The implementation deliberately avoids a resource state machine, configurable retry machinery, a custom cURL bridge, detached stream wrappers, and speculative image APIs.

Validation

composer fix completed successfully:

  • PHP CS Fixer: 5,574 files checked, no changes required
  • PHPStan: 3,107 primary files and 15 secondary files, no errors
  • Framework suite: 23,280 tests, 66,328 assertions, 1,600 skipped
  • Testbench contract suite: 346 tests, 1,029 assertions, 3 skipped
  • Testbench dogfood suite: 4 tests, 7 assertions
  • Package/checklist parity: 71/71
  • git diff --check: clean

Summary by CodeRabbit

  • New Features
    • Added support for custom server processes and documented server process setup/IPC.
    • Introduced iterable streamed responses to better stop output after write failures.
    • Added filesystem assertEmpty() and broader JSON result handling; file uploads now accept additional file types.
  • Bug Fixes
    • Improved exception/termination robustness across request lifecycle, streaming responses, and process pools.
    • Enhanced filesystem locking, replace-in-file validation, deletion failure reporting, and upload failure handling.
  • Documentation
    • Expanded guidance for process/server-process APIs and streaming behavior (including lazy streaming defaults for S3/GCS).

Normalize constructor-seeded fake sequences through the same conversion boundary used by pushed values so supported strings and output arrays always resolve to valid process results.

Treat only truly empty fake output as empty, preserving the supported string value zero, and narrow the public sequence types to the values the factory accepts.

Document fake controls as test-only worker state so long-lived factory mutations have an explicit lifecycle boundary.
Fail at the Process pipeline boundary when its builder produces no commands instead of allowing collection reduction to violate the declared process-result contract.

Keep empty process pools valid; only pipelines require a final process whose result can be returned.
Guard synchronous and asynchronous output callbacks at the owning process boundary. Stop the exact child when user output handling fails, contain teardown failure, and preserve the original exception without adding callback-free allocations.

Make pool startup transactional and pool waiting and stopping exhaustive. Retain exact child handles, preserve result keys, clean every owned sibling after failure, and keep the earliest failure primary.

Complete timeout and nullable-wait behavior across the invoked-process contract and fake, align fake PID, output, callable, readiness, and terminal-state semantics with real processes, and preserve supported falsy command and input values.

Restore the current Laravel Macroable surface on real invoked processes and register authoritative test cleanup for its worker-static registry. Publish precise environment and fake-sequence facade types and document asynchronous ownership boundaries.
Add deterministic real-process coverage for callback failures, exact-PID teardown, stop-time callback re-entry, nullable waits, timeout checks, and contract-level stopping.

Exercise transactional pool startup, exhaustive sibling cleanup, first-failure preservation, keyed and empty results, and descriptive empty-pipeline failures.

Cover fake readiness, handler restoration, callable normalization, terminal PID and output behavior, falsy values, seeded sequences, invoked-process macros, and authoritative static-state cleanup.
Identify the current Laravel Process source branch used as the package parity reference so future ports and audits begin from the correct upstream boundary.

Retain the existing package introduction and DeepWiki link while restoring the missing trailing newline.
Explain the shell boundary for string commands, recommend argv arrays for dynamic values and direct signals, and show escaped environment placeholders when shell features are required.

Document result and output callback failures, graceful stop arguments, wait callbacks, active asynchronous timeout enforcement, coroutine ownership, and transactional pool cleanup.

Describe invoked-process macros as boot-only worker-static extensions and state the deliberate real-versus-fake macro boundary without exposing internal cleanup machinery.
Record the verified Process findings, final ownership boundaries, rejected speculative mechanisms, originating Laravel pull-request trace, compatibility result, and performance assessment.

Capture focused and full-suite validation, independent review completion, and Concurrency revalidation against transactional pool startup, exhaustive cleanup, empty pools, and keyed results.

Mark Process complete, retain the process-02 cross-package revalidation record, and route the next audit work to Server Process.
Require originating implementation and documentation pull requests to establish the reason for an upstream change and the complete file surface affected when it was introduced.

Use historical pull-request diffs only for discovery, then port source, tests, and documentation from the current upstream default or development branch so follow-up fixes are retained.

Search current upstream for the added symbols, apply the established Hypervel adaptations, and add proportionate user documentation when upstream omitted it.
Replace the audit-local copy of incremental porting instructions with the authoritative AGENTS.md workflow so the two sources cannot drift.

Retain the audit-specific requirement to record originating implementation and documentation pull-request surfaces together with the current-branch result in the investigation and ledger.
Check native process registration and socket export at their owning boundaries, clean up rejected process pipes, and publish only successfully attached handles.

Make listener ownership and callback teardown exception-safe while preserving the earliest failure, retaining valid falsy IPC payloads, and applying fakeable restart backoff before rethrowing.

Remove unreachable exception surface and add deterministic coverage for registration rollback, socket export, channel closure, teardown precedence, IPC fidelity, and resource release.
Make server.processes the single declarative configuration path and ship its discoverable default alongside the other Swoole server settings.

Preserve programmatically registered process instances, remove the duplicate root configuration dependency, and keep registration behavior deterministic across class and instance inputs.

Register lifecycle logging as passive observation so reporting failures cannot prevent active Signal setup or teardown, with focused coverage for configuration and listener ordering.
Use mocked Swoole process handles for ProcessCollector assertions so pure static-registry tests do not consume native process resources.

Remove the redundant ProcessTest coverage now subsumed by the deterministic AbstractProcess lifecycle suite, reducing parallel resource pressure without weakening supported behavior coverage.
Add the package provenance and a concise authoring guide covering process definitions, declarative and programmatic registration, and container resolution.

Document lifecycle events, Signal integration, restart behavior, serialized IPC fidelity, trusted-data expectations, and server-owned native handle boundaries.

Align the package description with the established server-process terminology.
Record the verified Server Process findings, accepted corrections, rejected speculative machinery, regression coverage, validation results, API outcome, and performance assessment.

Mark server-process complete, carry its Foundation configuration dependency forward, and route the audit to filesystem with every required shared finding named explicitly.
Document how application and package authors define, configure, and register custom Swoole server processes, including process options, lifecycle events, signals, and boot-time ownership boundaries.

Add source-backed IPC send and receive examples covering collector scope, complete datagram writes, falsy payloads, message sizing, and trusted-data requirements. Link the guide beside the Process facade documentation so the two distinct process APIs are easy to discover.
Replace the worker-local Locker layer with checked native file locks so blocking operations use Swoole's adaptive scheduling while nonblocking calls fail immediately.

Make LockableFile creation, metadata, partial writes, flushing, truncation, unlocking, and closing failure-safe with exact resource ownership and earliest-failure precedence. Align local Filesystem native return types, metadata warning handling, recursive deletion, links, replacement, modes, and facade metadata with reachable runtime behavior.

Restore FileStore's native nonblocking lock semantics, redact atomic replacement contents at the facade boundary, remove the obsolete worker-gate regression, and add deterministic coverage for contention, cancellation, native failures, partial writes, cleanup, and metadata results.
Prevent append and prepend from overwriting an existing object after a failed read, and make putFileAs validate and close its owned source stream on every success and failure path.

Publish the complete decoded JSON union, accept the full Laravel file input family, and narrow Storage manager and assertion metadata to their real contracts. Port Laravel's assertEmpty testing API across direct, pooled, and scoped disks without adding an implementation-specific method to the generic Filesystem contract.

Add regression coverage for unreadable objects, missing upload sources, exceptional stream cleanup, decoded scalar JSON values, assertion failures, scoped prefixes, and pooled lease release.
Enable streamed reads by default for pooled S3 and Google Cloud Storage disks so readStream and readStreamRange expose data lazily instead of spooling complete remote objects before consumers can proceed.

Keep ordinary read and get operations on their existing transport path, pass the option through both SDK and Flysystem layers, and retain stream_reads=false as an explicit opt-out for workloads that prioritize connection reuse across many small streams.

Add socket-pair regressions that prove lazy consumption and coroutine scheduler progress, plus manager coverage for both defaults and per-disk opt-outs.
Encode each signed local-storage path segment while preserving directory separators so URI delimiters cannot alter the signed route or hide an expired signature.

Treat failed upload persistence as an internal server error instead of reporting a successful empty response, and rely on the merged filesystem configuration rather than a drifting call-site default when registering served disks.

Cover delimiter-bearing and nested download and upload paths, expired signatures, failed storage writes, and cleanup of every generated fixture.
Retain generator-backed response chunks in a dedicated StreamedResponse subtype so the Swoole bridge can write them directly and stop advancing their producer as soon as the peer rejects a write.

Emit each server-sent event as one complete chunk, support every callable generator shape, and preserve Symfony's buffered callback path for ordinary streamed responses. Make that fallback capture write failures without throwing from the output handler, clean only removable buffers, still end the response, and preserve the earliest failure.

Add deterministic coverage for lazy one-shot iteration, callback replacement, disconnect termination, generator cleanup, SSE encoding, callable normalization, write and end failure precedence, and ordinary callback compatibility.
Run request events, response sending, kernel termination, application callbacks, terminable middleware, duration handlers, and context removal through independent cleanup boundaries while preserving the earliest failure.

Treat a response already committed for streaming as authoritative when later application code fails, avoiding an invalid second render and ensuring HTTP Server events and termination receive the same response and original throwable.

Add regression coverage for every failing finalization phase, continued cleanup after each failure, committed-response preservation, duration-handler isolation, middleware traversal, and unconditional request context removal.
Fall back to application/octet-stream when MIME detection fails so file responses never publish an invalid Content-Type value.

Remove LeasedStream destructor cleanup and keep native pooled release on the explicit stream_close and construction-rollback boundaries, avoiding unsafe native teardown during PHP object destruction.

Prove failed response writes stop further reads and close the stream, missing MIME data uses the binary fallback, and explicit stream resource destruction releases each lease exactly once.
Update environment command and file-session mocks to model Filesystem::put as the native int-or-false boundary instead of returning booleans or null that strict production code cannot produce.

Correct the shared environment spy so get returns file contents and put returns the written byte count, keeping the tests faithful to the real caller and callee contracts without adding source-side guards for unreachable mock states.
Run the join_paths tests through Hypervel's framework test base so they receive the same coroutine and state-cleanup behavior as the rest of the suite.

Add complete native return types to test methods, generator providers, and Stringable fixtures while preserving the Unix and Windows path matrix unchanged.
Document that S3 and Google Cloud Storage readStream operations are lazy by default, including the bounded-memory benefit, the unchanged behavior of ordinary reads, the connection-reuse tradeoff, and the stream_reads opt-out.

Add assertEmpty examples alongside the existing filesystem testing assertions and align the documented Google Cloud Storage configuration with the framework default.
Record the verified native I/O, adapter, streaming, response, and finalization findings together with their accepted designs, rejected complexity, implementation result, regression coverage, performance tradeoffs, and signed-off validation.

Close the carried Filesystem, Coroutine, DI, Encryption, and Support revalidations, remove stale pending-audit language, and mark Filesystem complete in the authoritative package checklist.

Advance the routing index to Pipeline with support-02 as its required prior decision while preserving the remaining Cache, HTTP, Foundation, and HTTP Server revalidation routes.
@coderabbitai

coderabbitai Bot commented Jul 20, 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: ec0fbf59-d15a-4132-aa42-d9201d843731

📥 Commits

Reviewing files that changed from the base of the PR and between 06c93b8 and 1054be6.

📒 Files selected for processing (6)
  • docs/plans/2026-07-12-framework-coroutine-state-lifecycle-audit-ledger.md
  • src/boost/docs/filesystem.md
  • src/filesystem/src/Filesystem.php
  • src/routing/src/ResponseFactory.php
  • tests/Filesystem/FilesystemTest.php
  • tests/Routing/ResponseFactoryTest.php
🚧 Files skipped from review as they are similar to previous changes (5)
  • tests/Routing/ResponseFactoryTest.php
  • src/routing/src/ResponseFactory.php
  • tests/Filesystem/FilesystemTest.php
  • docs/plans/2026-07-12-framework-coroutine-state-lifecycle-audit-ledger.md
  • src/filesystem/src/Filesystem.php

📝 Walkthrough

Walkthrough

This PR hardens filesystem I/O, process and server-process lifecycles, HTTP streaming, termination cleanup, public contracts, documentation, and test coverage. It also updates audit ledgers and framework state flushing for newly introduced static process state.

Changes

Framework lifecycle and infrastructure

Layer / File(s) Summary
Filesystem contracts and I/O hardening
src/contracts/..., src/filesystem/..., src/foundation/config/filesystems.php, src/support/src/Facades/File.php
Filesystem locking, metadata, deletion, upload, URL encoding, streaming defaults, return types, and assertion APIs are updated.
Process callbacks and pool lifecycle
src/process/..., src/contracts/src/Process/InvokedProcess.php
Output callback failures stop processes, pools roll back or clean up siblings, timeout enforcement is exposed, and falsy command/input values are preserved.
Custom server processes
src/server-process/..., src/foundation/config/server.php
Server-process registration, lifecycle cleanup, socket export failures, IPC payload handling, and event observation are revised.
HTTP streaming and finalization
src/http/..., src/http-server/..., src/routing/src/ResponseFactory.php, src/foundation/src/Http/Kernel.php
Iterable streaming, SSE formatting, write-failure handling, committed-response preservation, and exhaustive termination are implemented.
Documentation and audit records
AGENTS.md, docs/plans/..., src/boost/docs/..., src/server-process/README.md, src/process/README.md
Upstream-update guidance, audit status, process APIs, server processes, and filesystem streaming/testing behavior are documented.

Validation

Layer / File(s) Summary
Filesystem and streaming tests
tests/Filesystem/*, tests/Http/*, tests/HttpServer/*, tests/Routing/*
Coverage verifies native I/O failures, lock ownership, streaming defaults, iterable responses, response termination, scoped assertions, and signed paths.
Process and server-process tests
tests/Process/*, tests/ServerProcess/*
Coverage verifies callback failures, pool cleanup, falsy values, IPC handling, process registration, teardown, and static-state cleanup.
Integration and contract tests
tests/Integration/*, tests/Foundation/*, tests/Encryption/*, tests/Cache/*, tests/Session/*
Integration coverage verifies upload failures, URI-delimited paths, termination behavior, sensitive parameters, cache locking, and updated filesystem return values.

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

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title matches the PR’s main theme: hardening process, server process, and filesystem lifecycle behavior.
Docstring Coverage ✅ Passed Docstring coverage is 98.11% which is sufficient. The required threshold is 80.00%.
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/process-and-filesystem-lifecycles

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.

@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

🧹 Nitpick comments (2)
src/routing/src/ResponseFactory.php (1)

118-135: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Multi-line string payloads will break SSE framing.

If $message is (or resolves to) a raw string containing \n, only the first line is treated as data:; subsequent lines are sent unprefixed and will be misinterpreted as new SSE fields by the client. Per the SSE spec, each line of the payload needs its own data: prefix.

🐛 Suggested fix
         if (! is_string($message) && ! is_numeric($message)) {
             $message = Js::encode($message);
         }

-        return "event: {$event}\ndata: {$message}\n\n";
+        $data = implode("\n", array_map(
+            static fn (string $line): string => "data: {$line}",
+            explode("\n", (string) $message),
+        ));
+
+        return "event: {$event}\n{$data}\n\n";
🤖 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/routing/src/ResponseFactory.php` around lines 118 - 135, Update
formatStreamedEvent to split string payloads on newline boundaries and prefix
every resulting line with data: before appending the terminating blank line.
Preserve the existing event selection, StreamedEvent data extraction, and
encoding behavior for non-string/non-numeric messages.
src/filesystem/src/LocalFilesystemAdapter.php (1)

75-75: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Extract the shared signed-path encoder.
Both methods repeat strtr(rawurlencode($path), ['%2F' => '/']); a small private helper would keep the signed URL path handling in one place.

🤖 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/filesystem/src/LocalFilesystemAdapter.php` at line 75, Extract the
repeated strtr(rawurlencode($path), ['%2F' => '/']) logic from the affected
methods into a private helper in LocalFilesystemAdapter, then call that helper
wherever the signed path is encoded. Preserve the existing encoding behavior
exactly.
🤖 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 779: Escape the pipe in the `Socket|false` type within the
`server-process-02` Markdown table cell so the row retains its intended
six-column structure.

In `@src/boost/docs/filesystem.md`:
- Around line 941-943: The Pest example at src/boost/docs/filesystem.md lines
941-943 and the PHPUnit example at src/boost/docs/filesystem.md lines 980-982
assert that the disk is empty after adding photo1.jpg and photo2.jpg. Update
both examples to use a fresh fake disk before assertEmpty(), or remove the
contradictory assertEmpty() assertions while preserving the intended filesystem
examples.

In `@src/filesystem/src/Filesystem.php`:
- Around line 448-453: Update Filesystem::mimeType() so the finfo_file() call
does not emit its native warning for missing or unreadable paths, while
preserving its false return value and existing behavior when finfo_open() fails.

In `@tests/Filesystem/FilesystemTest.php`:
- Around line 322-335: Update
testDeleteDirectoryReportsFailureAfterAttemptingEveryEntry to compare
RecordingDeleteFilesystem::$deleted with the expected file paths without
depending on iteration order, while retaining the assertFalse result check.

---

Nitpick comments:
In `@src/filesystem/src/LocalFilesystemAdapter.php`:
- Line 75: Extract the repeated strtr(rawurlencode($path), ['%2F' => '/']) logic
from the affected methods into a private helper in LocalFilesystemAdapter, then
call that helper wherever the signed path is encoded. Preserve the existing
encoding behavior exactly.

In `@src/routing/src/ResponseFactory.php`:
- Around line 118-135: Update formatStreamedEvent to split string payloads on
newline boundaries and prefix every resulting line with data: before appending
the terminating blank line. Preserve the existing event selection, StreamedEvent
data extraction, and encoding behavior for non-string/non-numeric messages.
🪄 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: 829ee7c0-056e-41f4-a690-c4cdbe175686

📥 Commits

Reviewing files that changed from the base of the PR and between 52f357b and 06c93b8.

📒 Files selected for processing (89)
  • AGENTS.md
  • docs/plans/2026-07-12-framework-coroutine-state-lifecycle-audit-ledger.md
  • docs/plans/2026-07-12-framework-coroutine-state-lifecycle-audit.md
  • src/boost/docs/documentation.md
  • src/boost/docs/filesystem.md
  • src/boost/docs/processes.md
  • src/boost/docs/server-process.md
  • src/cache/src/FileStore.php
  • src/contracts/src/Filesystem/Filesystem.php
  • src/contracts/src/Process/InvokedProcess.php
  • src/filesystem/src/AwsS3V3Adapter.php
  • src/filesystem/src/Concerns/InteractsWithPooledFilesystem.php
  • src/filesystem/src/FileResponseBuilder.php
  • src/filesystem/src/Filesystem.php
  • src/filesystem/src/FilesystemAdapter.php
  • src/filesystem/src/FilesystemManager.php
  • src/filesystem/src/FilesystemServiceProvider.php
  • src/filesystem/src/GoogleCloudStorageAdapter.php
  • src/filesystem/src/LeasedStream.php
  • src/filesystem/src/LocalFilesystemAdapter.php
  • src/filesystem/src/LockableFile.php
  • src/filesystem/src/ReceiveFile.php
  • src/filesystem/src/ScopedFilesystemProxy.php
  • src/foundation/config/filesystems.php
  • src/foundation/config/server.php
  • src/foundation/src/Application.php
  • src/foundation/src/Http/Kernel.php
  • src/http-server/src/ResponseBridge.php
  • src/http-server/src/Server.php
  • src/http/src/IterableStreamedResponse.php
  • src/process/README.md
  • src/process/src/Concerns/GuardsProcessOutput.php
  • src/process/src/Factory.php
  • src/process/src/FakeInvokedProcess.php
  • src/process/src/FakeProcessResult.php
  • src/process/src/FakeProcessSequence.php
  • src/process/src/InvokedProcess.php
  • src/process/src/InvokedProcessPool.php
  • src/process/src/PendingProcess.php
  • src/process/src/Pipe.php
  • src/process/src/Pool.php
  • src/routing/src/ResponseFactory.php
  • src/server-process/README.md
  • src/server-process/composer.json
  • src/server-process/src/AbstractProcess.php
  • src/server-process/src/Exceptions/ServerInvalidException.php
  • src/server-process/src/Exceptions/SocketAcceptException.php
  • src/server-process/src/Listeners/BootProcessListener.php
  • src/server-process/src/ServerProcessServiceProvider.php
  • src/support/src/Facades/File.php
  • src/support/src/Facades/Process.php
  • src/support/src/Facades/Storage.php
  • src/testing/src/PHPUnit/AfterEachTestSubscriber.php
  • tests/Cache/CacheFileStoreTest.php
  • tests/Encryption/SensitiveParameterTest.php
  • tests/Filesystem/AwsS3V3AdapterTest.php
  • tests/Filesystem/CoroutineLockOwnershipTest.php
  • tests/Filesystem/FileResponseBuilderTest.php
  • tests/Filesystem/FilesystemAdapterTest.php
  • tests/Filesystem/FilesystemManagerTest.php
  • tests/Filesystem/FilesystemPoolProxyTest.php
  • tests/Filesystem/FilesystemTest.php
  • tests/Filesystem/GoogleCloudStorageAdapterTest.php
  • tests/Filesystem/JoinPathsHelperTest.php
  • tests/Filesystem/LeasedStreamTest.php
  • tests/Filesystem/LockableFileTest.php
  • tests/Filesystem/ScopedFilesystemProxyTest.php
  • tests/Foundation/FoundationApplicationTest.php
  • tests/Foundation/Http/KernelTest.php
  • tests/Http/IterableStreamedResponseTest.php
  • tests/HttpServer/ResponseBridgeTest.php
  • tests/HttpServer/ServerTest.php
  • tests/Integration/Console/EnvironmentDecryptCommandTest.php
  • tests/Integration/Console/EnvironmentEncryptCommandTest.php
  • tests/Integration/Filesystem/ReceiveFileTest.php
  • tests/Integration/Filesystem/ServeFileTest.php
  • tests/Process/ProcessTest.php
  • tests/Routing/ResponseFactoryTest.php
  • tests/ServerProcess/AbstractProcessTest.php
  • tests/ServerProcess/BootProcessListenerTest.php
  • tests/ServerProcess/ExceptionTest.php
  • tests/ServerProcess/Fixtures/ListenableProcess.php
  • tests/ServerProcess/ListenCreationFailureTest.php
  • tests/ServerProcess/ListenMethodTest.php
  • tests/ServerProcess/ProcessCollectorTest.php
  • tests/ServerProcess/ProcessTest.php
  • tests/ServerProcess/ServerProcessServiceProviderTest.php
  • tests/Session/FileSessionHandlerTest.php
  • tests/Testing/PHPUnit/AfterEachTestSubscriberTest.php
💤 Files with no reviewable changes (5)
  • src/server-process/src/Exceptions/ServerInvalidException.php
  • tests/ServerProcess/ProcessTest.php
  • tests/Filesystem/CoroutineLockOwnershipTest.php
  • src/server-process/src/Exceptions/SocketAcceptException.php
  • src/filesystem/src/LeasedStream.php

Comment thread docs/plans/2026-07-12-framework-coroutine-state-lifecycle-audit-ledger.md Outdated
Comment thread src/boost/docs/filesystem.md Outdated
Comment thread src/filesystem/src/Filesystem.php
Comment thread tests/Filesystem/FilesystemTest.php
@greptile-apps

greptile-apps Bot commented Jul 20, 2026

Copy link
Copy Markdown

Greptile Summary

This PR hardens ownership boundaries across Process, Server Process, and Filesystem by making construction atomic (rollback on partial failure), cleanup exhaustive (preserve-first-error across all finalization steps), and native-result handling explicit at each boundary rather than letting false escape into stricter PHP contracts.

  • Process: Pool and Pipe now validate before starting any process and stop already-started siblings on construction failure; InvokedProcessPool::wait() stops surviving siblings when any single wait fails; FakeInvokedProcess gains an outputHandlerFailed flag and corrected empty-output behavior.
  • Server Process: AbstractProcess::bind() finalizes with preserve-first-error across event dispatch, channel push, timer teardown, and restart sleep; listen() closes the $quit channel on all coroutine exit paths.
  • Filesystem and streaming: LockableFile checks every native call with preserve-first-error in close(); IterableStreamedResponse retains the iterable so the Swoole bridge can stop production on write failure; Kernel::terminate() and Application::terminate() run all steps exhaustively.

Confidence Score: 4/5

The changes are well-scoped and mechanically consistent, but the static $failed closure op-array sharing in GuardsProcessOutput remains unaddressed from the prior review thread, and the blocking flock replacement of the coroutine-yielding Locker wrapper needs Swoole hook verification.

The exhaustive-finalization and preserve-first-error patterns are applied correctly throughout. The IterableStreamedResponse fast-path is correct. The main open question is whether Swoole hooks cover flock; if not, blocking getExclusiveLock(block: true) calls will stall the entire worker thread under contention.

src/process/src/Concerns/GuardsProcessOutput.php (static closure variable semantics) and src/filesystem/src/LockableFile.php (blocking flock under Swoole without a verified coroutine hook) warrant a second look before merging.

Important Files Changed

Filename Overview
src/http/src/IterableStreamedResponse.php New class retaining iterable chunks for Swoole's direct-write path; setChunks correctly stores both field and closure, streamTo stops production on write failure, clearChunks prevents double delivery
src/process/src/Concerns/GuardsProcessOutput.php New trait guarding process output callbacks; static $failed variable shared across all closure instances from the same op-array (previously flagged) can cause output suppression after any single process fails
src/filesystem/src/LockableFile.php Replaces atomic()/Locker coroutine-safe wrappers with checked native flock; exhaustive error checking on all native calls; close() uses preserve-first-error pattern
src/filesystem/src/Filesystem.php Removes Locker-based atomic wrapper; put() uses file_put_contents with LOCK_EX; replaceInFile adds byte-count validation; deleteDirectory propagates partial-failure status
src/foundation/src/Http/Kernel.php terminate() made exhaustive with preserve-first-error across all cleanup steps; handle() moves RequestHandled dispatch inside try-catch with streamed-response awareness
src/http-server/src/ResponseBridge.php sendStreamedContent delegates to sendIterableContent for IterableStreamedResponse; OB handler tracks write failures via $writable flag; both paths call swooleResponse->end() in finally
src/server-process/src/AbstractProcess.php bind() makes finalization exhaustive with preserve-first-error; listen() closes $quit channel on all exit paths including coroutine-create failure; getListenSocket() returns Socket
src/process/src/InvokedProcessPool.php stop() handles inspection exceptions without abandoning terminal cleanup; wait() stops surviving siblings on any single wait failure to prevent pool-level hang
src/routing/src/ResponseFactory.php eventStream() yields formatted SSE chunks via IterableStreamedResponse; formatStreamedEvent() added with CRLF normalization; stream() wraps generator callbacks in IterableStreamedResponse
src/process/src/FakeInvokedProcess.php outputHandlerFailed flag added to match real process behavior; waitUntil() saves/restores prior output handler in finally; id() returns null when process has no remaining iterations
src/process/src/Pool.php start() validates all processes before any are started; partial-construction catch stops already-started siblings and rethrows the creation failure
src/filesystem/src/LeasedStream.php __destruct() removed in favor of explicit close ownership; stream_close() via PHP resource GC is now the only finalization path
src/http-server/src/Server.php onRequest() finalization made exhaustive; already-streamed responses preserved instead of replaced with a synthetic 500 when Kernel::handle() throws

Reviews (2): Last reviewed commit: "docs(audit): record filesystem review co..." | Re-trigger Greptile

Comment thread src/process/src/Concerns/GuardsProcessOutput.php
Suppress warnings at both fileinfo native boundaries and return the declared false result when the MIME database or file probe fails.

Cover the missing-file failure path and keep exhaustive recursive deletion coverage independent of FilesystemIterator ordering without weakening the asserted entries.
Normalize CRLF and carriage-return boundaries before prefixing each logical payload line with an SSE data field. Preserve the single-line fast path with one native scan and no normalization allocation.

Add exact-byte regression coverage for CRLF, CR, and LF delimiters so future formatter changes cannot silently emit invalid continuation lines.
Demonstrate disk emptiness before uploads and use a distinct empty directory for directory-level assertions in both Pest and PHPUnit examples.

Keep the upstream documentation structure while removing contradictory states that would mislead application and package authors.
Amend the completed Filesystem work unit with the verified MIME boundary, multiline SSE framing, deterministic regression coverage, and coherent documentation examples.

Repair the Markdown union table cell, record the exact performance and Laravel-parity effects, and update validation and review evidence to the final green state.
@binaryfire

Copy link
Copy Markdown
Collaborator Author

@coderabbitai @greptile-apps Followed up on every review finding, including the nitpicks and outside-diff item.

Changes landed:

  • Filesystem::mimeType() now suppresses and checks both finfo_open() and finfo_file(); either native failure returns the declared false result. Added a missing-file regression.
  • Recursive deletion coverage now compares the exact attempted paths without assuming FilesystemIterator order.
  • The filesystem examples assert an empty disk before writes and use a distinct empty directory for assertDirectoryEmpty().
  • The audit table now escapes Socket\|false and the work-unit record reflects the final behavior and validation.
  • SSE payloads normalize CRLF before lone CR, then prefix every logical line with data: . The single-line path remains one native scan; exact regression coverage includes CRLF, CR, and LF.

Three items were intentionally left unchanged:

  • Closure function statics are per closure instance on the supported PHP 8.4.23 runtime. A direct probe and sequential real callback-failure tests both confirm there is no cross-process contamination.
  • Symfony StreamedResponse::setChunks() assigns its callback property directly; it does not dispatch through the overridden setCallback(). The retained iterable remains available, and the direct streamTo() coverage passes.
  • The signed-path encoding stays as two short current-Laravel expressions. A private helper would add indirection and upstream divergence without owning additional behavior.

Validation is green: PHP CS Fixer, both PHPStan configurations, 23,282 component tests, 346 Testbench contract tests, 4 dogfood tests, focused filesystem/routing/streaming coverage, and git diff --check.

@greptile-apps

greptile-apps Bot commented Jul 20, 2026

Copy link
Copy Markdown

I need to walk back my original claim. The PHP manual documents that separate Closure objects created from the same literal each get their own static variable slot — sharing only occurs when two variables reference the same Closure object. Since each call to guardProcessOutput() produces a new Closure instance, $failed is per-wrapper, not shared across processes.

Your direct probe was right and my original reasoning was wrong. The static $failed is safe as-is. The other four items are all addressed cleanly — this was a thorough follow-up pass.

@coderabbitai

coderabbitai Bot commented Jul 20, 2026

Copy link
Copy Markdown

Tip

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

@binaryfire Thanks for the thorough follow-up. The fixes and regression coverage address the reported findings, and the rationale for the three retained implementations is sound—particularly preserving upstream-aligned signed-path expressions where a helper adds no behavior.

The reported validation coverage is also comprehensive. 🐇✅


✏️ Learnings added
Learnt from: binaryfire
URL: https://github.com/hypervel/components/pull/442

Timestamp: 2026-07-20T07:24:14.606Z
Learning: In the Hypervel Process package on the supported PHP 8.4.23 runtime, function-static variables in output-guard closures are per closure instance; sequential real callback-failure coverage confirms no state contamination between different processes.

Learnt from: binaryfire
URL: https://github.com/hypervel/components/pull/442

Timestamp: 2026-07-20T07:24:14.606Z
Learning: For Hypervel routing streaming, Symfony `StreamedResponse::setChunks()` assigns the callback property directly rather than invoking an overridden `setCallback()`. `IterableStreamedResponse` must retain its iterable chunks for direct `streamTo()` handling, which is covered by regression tests.

Learnt from: binaryfire
URL: https://github.com/hypervel/components/pull/442

Timestamp: 2026-07-20T07:24:14.606Z
Learning: In `src/filesystem/src/LocalFilesystemAdapter.php`, signed-path encoding intentionally remains as the concise current-Laravel `rawurlencode` plus slash-preservation expressions; a private helper would add indirection without owning additional behavior and would diverge from upstream.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

@binaryfire
binaryfire merged commit d27487c into 0.4 Jul 20, 2026
42 of 43 checks passed
@binaryfire
binaryfire deleted the audit/process-and-filesystem-lifecycles branch July 21, 2026 09:09
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