Repository navigation
Sync Inertia and Slack updates and fix worker lifecycle issues - #61
Conversation
Ports inertiajs/inertia-laravel #915, #888 and #904. They share the
middleware tests, the Inertia config file and the frontend
documentation, so they land together.
#915: Inertia answers fragment redirects and Inertia::location() with a
409 response that tells the client to perform the redirect itself.
Deferred callbacks only run after responses below 400, so callbacks
registered with defer() were skipped even though the request
succeeded. A new EnsureDeferredCallbacksRun middleware, prepended to
the global middleware stack by the service provider, marks the pending
callbacks to always run on these responses. Asset version mismatch
reloads and failed responses still skip them. The middleware's own
per-request cost is within benchmark noise.
#888: The new store_previous_url option makes the Inertia middleware
store the URL and route name of Inertia visits as the session's
previous location, which the session middleware skips for AJAX
requests. Only GET visits are stored; prefetch requests, precognitive
requests and partial reloads of the rendered component are excluded.
Applications may override shouldStoreCurrentUrl() to change which
visits are stored. Upstream's Laravel 11 compatibility checks are not
ported.
#904: The new preserve_big_integers option, or preserveBigIntegers()
on a single response, sends integers outside JavaScript's safe range
in props and flash data as {"$bigint": "..."} markers and flags the
page, so the Inertia client (3.8 or later) revives them as native
BigInt values. It is disabled by default.
Hypervel adaptations:
- Hypervel encodes the root view's page JSON with JSON_THROW_ON_ERROR,
so the self-referencing object and pure enum tests expect that JSON
error where upstream renders false.
- frontend.md gains Previous URL and Big Integers sections, adapted
from inertiajs/docs cf513d8ffc.
Upstream reference: inertiajs/inertia-laravel 3.x at 4da52b72da.
Validation: upstream tests ported to MiddlewareTest,
InertiaServiceProviderTest, PropsResolverTest and ResponseFactoryTest,
plus the Inertia suite and PHPStan.
Ports inertiajs/inertia-laravel #917. inertia:stop-ssr fails when the SSR server is not running, which breaks deployment scripts that stop the server before it has ever started. With --graceful, the command reports that the server is not running and exits successfully instead. Only an unreachable server counts as not running. When something answers on the SSR URL with an unhealthy response, the command still fails, with or without --graceful, as upstream's tests require. Hypervel adaptation: the command already checks health before shutting the server down, because the HTTP client cannot tell a refused connection from the official SSR server closing the connection without replying to /shutdown. A new HttpGateway::checkHealth() returns the health result and throws ConnectionException when the server cannot be reached, so the command can tell an unreachable server from an unhealthy response. isHealthy() wraps it and still returns false for both. vite.md documents stopping the SSR server and the --graceful option, adapted from inertiajs/docs cf513d8ffc. Upstream reference: inertiajs/inertia-laravel 3.x at 4da52b72da. Validation: upstream tests ported to StopSsrTest, plus the Inertia suite and PHPStan.
Ports inertiajs/inertia-laravel #918. When a page preserves big
integers, its props and flash data carry {"$bigint": "..."} markers, so
assertInertia() and inertiaProps() saw the markers rather than the
integers passed to the response. AssertableInertia now turns the
markers back into integers when the page sets preserveBigIntegers, so
tests assert against the original values. Pages without the flag keep
any marker-shaped data the application sends itself.
Upstream reference: inertiajs/inertia-laravel 3.x at 4da52b72da.
Validation: upstream tests ported to AssertableInertiaTest, plus the
Inertia suite and PHPStan.
Give prepareMockEndpoint() the required method title, describing the given or example middleware it registers.
AGENTS.md requires a title docblock on every method except test* methods and a native return type on test methods, but most older tests omit them. Agents follow the surrounding code over the written rule, so new tests keep reproducing the gap until review catches it. Record the owner's request for one sweep of each, about 8,400 untitled methods in 1,600 files and 5,600 untyped test methods in 490 files, followed by an automated check that keeps the code and the rule aligned.
Bring the isolated Inertia changes onto the current framework baseline before reconciling Slack. Keep the pending test title and return-type follow-ups, while retaining 0.4's removal of the completed exception-message migration task.
PHPUnit deprecates expectExceptionMessage(), which matches any substring. Replace the eight remaining calls with the explicit alternatives, keeping what each test meant to check. Use expectExceptionMessageIs() for the notification serialization and streaming timeout messages, which the tests expect in full. Keep partial matching with expectExceptionMessageIsOrContains() where the expected text is part of a longer message: the Inertia view wrapper, the two database read pool messages and the Curl host fragments. Validation: each changed test file passes on its own.
Compare the Block Kit contracts, blocks, composites, elements and default ID generation with laravel/slack-notification-channel 3.x at 7b7c3e220a4f7a45cd88d65069f28d4fb4b67281, together with their unit tests. Every remaining source difference is a deliberate Hypervel fix or adaptation: static return types, Slack limits counted in characters, filters that keep '0' values, UTF-8-safe truncation, verbatim select option values, the select placeholder and option count limits, select action IDs seeded from their text, and generated IDs capped at 255 characters. Correct inherited wording: imperative titles for TextObject::markdown() and ConfirmObject::danger(), the PlainTextOnlyTextObject::$text docblock copied from $type, and the SectionBlock error, which now asks for at least one field rather than one block. Type the remaining callbacks. Every upstream unit test case and assertion is present. Rename the upstream-derived test methods to the camelCase form of their upstream names so later syncs can match them, which also fixes a header text length test that was named after the block ID. Remove two UsersSelectElementTest cases already covered by StaticSelectElementTest, and add titles to SelectOptionTest's data providers. Validation: the Slack suite (208 tests), the Horizon LongWaitDetected test, composer lint:fix and composer analyse pass.
Compare the webhook and Web API channels, messages, router, provider, tests and documentation with laravel/slack-notification-channel 3.x (7b7c3e220a4f7a45cd88d65069f28d4fb4b67281) and Laravel docs 13.x (226b0649c77e1d6fe739a20e1da654e94ccc718f). Fixes: - SlackAttachment::field() used is_callable(), so titles that name PHP functions, such as "Date" or "Count", were called instead of used as titles. Only closures are now treated as field builders. - Webhook sends used a Guzzle client with no request timeout, so a stalled host held the send indefinitely. The provider now gives the webhook channel 10-second connect and 30-second request timeouts; applications can replace the client and attachment messages can override options with http(). Upstream alignment: - Restore the legacy message docblock, the nullable callback ID default and the inlined Web API URL; type the remaining callbacks. - Rename upstream-derived tests to upstream's names and order, restore the router's channel-string case, the Block Kit Builder URL assertions and the shared helper's Content-Type check, and reuse the shared notifiable fixture in the router tests. Documentation: - Restore Laravel's installation and Slack App wording, document the webhook URL and false routes with an Incoming Webhooks section, and label the scopes, token and HTTP connection as Web API settings. - Record the verbatim select option value difference in the README and set the Slack sync checkpoint. Validated with the Slack suite, Horizon's LongWaitDetectedTest, composer lint:fix and composer analyse.
Add Engine followed by the Hyperf monorepo with the owner-selected November 1, 2025 history boundary. Record selective runtime review scope and source mappings while preserving Hypervel architecture and Laravel-style APIs. Consult Engine Contract only when an Engine change requires it.
Bring the unpublished Inertia and Slack improvements onto the current framework base before adding independent framework fixes. Preserve both branches' Testing TODO entries and retain the updated HTTP streaming tests with their exact exception assertions. Validated with the affected Inertia, Slack, notifications, HTTP streaming and database read-pool tests: 1,139 tests, 3,922 assertions and six skips. The branch remains at 66 changed files against 0.4.
When a coroutine test threw while child coroutines it started were still blocked, the runner waited for those children until the test's time limit and then reported only "This test was aborted after N seconds". The assertion failure or error that caused the problem was lost. If the test method returned or threw before the deadline and its children are what reached the limit, the runner now rethrows the test's own exception. Failures and errors are reported with their original message. An exception the test expected would otherwise make PHPUnit pass the test, so the runner keeps it and a post-condition fails the test, naming the leftover children with the expected exception as the cause. Skipped and incomplete tests never reach post-conditions, so they keep the time-limit abort. Passing tests that leave children running still time out, and children are not cancelled earlier than before. The time-limit fixture covers failures, errors, expected exceptions and assertion failures, and skipped and incomplete tests, each with children still running at the limit, and checks that tearDown() still runs. Expected-exception tests whose children finish in time still pass, with the children completing normally. Verified with TimeLimitTest, composer lint:fix, composer analyse and the full composer test:parallel suite, plus mutation checks that disabling the rethrow or the skipped/incomplete exclusion fails the new cases.
Fortify derived its passkey settings from the application URL and key, and Horizon filled a missing name from the application name. Both wrote the results into configuration while the server booted, and the configuration tracker replayed those values literally into every worker. A worker whose environment changed the URL, key or application name kept the server's values. Horizon also selected its Redis connection once from the boot-time configuration. Run each through configureUsing() so every worker derives it again from its rebuilt configuration. Values the application sets explicitly still take precedence. Add regression tests that replay the boot-time mutations into rebuilt configurations. Each fails before the fix. Formatting, static analysis and the Fortify and Horizon suites pass.
Cache repositories dispatch events by default. A store opts out with "events" set to false, and the failover repository turns off its outer events because its backing stores dispatch them. When the cache watcher was enabled, Telescope instead set "events" to true on every configured store while the application registered. That re-enabled stores the application had deliberately quieted and recorded every failover operation twice. The change was also boot-time configuration that workers did not derive again, and it relied on a static flag. Register the watcher's listeners directly and leave store configuration alone. Remove the static flag and its test-state cleanup. Add regression tests for a failover store recording each operation once and for a store with events disabled recording nothing. Both fail before the fix. The disabled-watcher test now asserts that no cache entries are recorded instead of inspecting configuration. Formatting, static analysis and the Telescope suite pass.
Each FTP and SFTP adapter holds one open connection. Under Swoole's coroutine hooks, two coroutines using the same disk at once read or write the same socket, and Swoole kills the worker with "Socket#N has already been bound to another coroutine". Separate adapters do not interfere. Add both drivers to the whole-driver pooled drivers, so each operation borrows a disk with its own connection. The existing pool proxy already keeps streams leased until closed, loads listings inside the borrow and blocks raw access outside it. Purging the pools destroys idle disks, and the adapters' destructors close their connections. The filesystem docs explain the pooling and the per-worker connection count, the README records the difference from Laravel, and the porting guide now covers raw access on every pooled disk, including S3 and GCS. Validated against local FTP and SFTP servers: eight concurrent reads through a shared disk crash the worker before this change and all succeed after it, and purging closes every connection. The filesystem suite, lint and static analysis pass.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
PR Summary by QodoSync Inertia and Slack behavior; fix pooled disks and worker lifecycle
AI Description
Diagram
High-Level Assessment
Files changed (83)
|
Code Review by Qodo
1.
|
|
| protected function encodeBigIntegersInArray(array $value, ?SplObjectStorage $seen): array | ||
| { | ||
| foreach ($value as $key => $nested) { | ||
| $value[$key] = $this->encodeBigIntegers($nested, $seen); | ||
| } |
There was a problem hiding this comment.
With preserveBigIntegers() enabled, a self-referencing array inside a plain-object prop or flash data makes encodeBigIntegersInArray() recurse endlessly. For example, $data['self'] = &$data passed inside (object) ['data' => $data] reaches this loop without being walked by PropsResolver. SplObjectStorage only guards objects, so the response exhausts memory or the stack instead of letting json_encode() reject the cycle.
Add a cycle or depth guard, with tests for arrays inside objects and flash data.
There was a problem hiding this comment.
Leaving this unchanged: reference-cyclic arrays are not supported JSON props. The existing props resolver recursively processes arrays before the encoder even when bigint preservation is disabled. Object backreferences remain separately handled and tested. Adding another traversal mechanism here would not make cyclic arrays supported.
There was a problem hiding this comment.
That is true for a cyclic array supplied directly as a prop: PropsResolver::resolveProps() walks that array before this encoder runs. It does not cover the reported case, though. When the array is held by a plain object ((object) ['data' => $data]), resolveProps() leaves the object untouched because it only recurses when is_array($value) is true.
With preservation enabled, encodeBigIntegers() then unwraps the object via get_object_vars() and enters encodeBigIntegersInArray(). Since arrays have no cycle guard, the self-reference recurses indefinitely before json_encode() can reject it. Thus the opt-in path changes a prompt serialization failure into memory or stack exhaustion for object-contained cyclic arrays. The existing SplObjectStorage guard does not help because the cycle is through an array, not an object.
There was a problem hiding this comment.
Thanks for correcting that path: an array inside a plain object bypasses the props resolver's array traversal. That comparison does not apply to your example. The disposition remains unchanged because an explicitly reference-cyclic array is unsupported JSON input, whether used directly or wrapped in an object. We aren't adding a separate cycle-tracking mechanism for that unsupported input.
There was a problem hiding this comment.
Understood. Given that reference-cyclic arrays are intentionally unsupported JSON input, regardless of whether they are nested directly or inside a plain object, I’m closing this finding without requesting a change.
Return pooled connections as soon as a stream is fully buffered, including bounded ranges over temporary streams. Keep live and unmarked decorated streams borrowed until close, without copying buffers or changing range seeking and size semantics. Share the ownership decision across whole-driver and client pools. Preserve live S3 read-through coverage and document when downloads release their connections. Filesystem, Inertia, JWT and object-pool suites, formatting and static analysis pass.
An empty Inertia response without a Referer could redirect back to itself after previous-location tracking overwrote the session URL. Store the visit after constructing the redirect, while preserving the original controller response for partial-reload detection and application eligibility overrides. Add a regression for empty visits without a referrer. Middleware tests, formatting and static analysis pass.
Treat destination-policy failures as failed health checks while keeping connection failures distinct for graceful shutdown. A blocked health check must not report a successful stop. Cover both policy exception types through health and shutdown commands, and assert captured Artisan output as well as raw output. Command tests and affected suites pass.
Avoid repeating reflection over each object class hierarchy when preserving big integers. Retain one boolean per encountered class, never payload values or object instances, and clear the map through the existing response test cleanup. Extend the arbitrary-object test to verify changed values across responses. Clarify opt-in examples and the reserved client marker. Encoder prototype measurements show 26–29% savings for DTO payloads with a fixed 392-byte increase for the tested one- and two-class maps. Props tests, affected suites, formatting and static analysis pass.
Lcobucci JWT 5.6.1 reports malformed registered dates through InvalidTokenStructure rather than TypeError. Assert the public token exception, message and retained Throwable cause without depending on an internal exception class. All malformed-date cases pass with both dependency versions 5.6.0 and 5.6.1. Production decoding behavior is unchanged.
Upstream Updates
inertia:stop-ssr --graceful. An unreachable SSR server can be treated as already stopped, while an unhealthy response or a health check blocked by destination policy still fails. Preserve Hypervel's health check before shutdown and document the option.Additional Hypervel Fixes
events: falseand let failover stores use their backing stores' events, avoiding duplicate cache entries.DateandCountas strings instead of invoking PHP functions with those names. Document webhook URL routing and attachment messages for compatible services separately from Slack's Web API setup.The affected suites, formatting and static analysis pass. FTP and SFTP were also checked against local servers: concurrent reads succeed with separate pooled connections, and purging the pool closes them.
Note
Add Inertia BigInt preservation and previous-URL storage, pool FTP/SFTP disks, and fix stream lease handling
preserve_big_integersconfig (envINERTIA_PRESERVE_BIG_INTEGERS) and a per-responsepreserveBigIntegers()opt-in/out in Response.php using the newPreservesBigIntegerstrait.AssertableInertiadecodes the markers back to integers for assertions.store_previous_urlsetting. Only session-backed GET AJAX route visits are stored; prefetches, Precognition requests, and matching partial reloads are excluded.EnsureDeferredCallbacksRunmiddleware, prepended to the global HTTP kernel stack, so deferred callbacks run on 409 client-side redirects.releaseOrWrapStream: buffered reads release the driver lease immediately; live streams keep the lease viaLeasedStreamuntil closed.inertia:stop-ssrgains a--gracefuloption and succeeds when the SSR server is unreachable;SlackAttachment.fieldno longer invokes callable string titles (e.g.Date) as callbacks andcallbackIdnow defaults to null;SlackWebhookChannelgets a bounded HTTP client (10s connect, 30s total); Telescope no longer enables cache-store events viaCacheWatcher; the coroutine test runner preserves the original failure when a child coroutine hits the time limit.Macroscope summarized 2913864.