Fix JSON correctness and package metadata discovery - #14
Open
binaryfire wants to merge 13 commits into
Open
Conversation
Treat the public depth argument as a maximum number of nested containers and translate it to PHP's distinct decode and validation depth unit. Preserve caller flags, forward throwing behavior to Jsonable values, use json_validate for predicates, and align framework response test readers with the shared contract. Add boundary, native-error, and flag-forwarding coverage.
Raise native decode depths where Collections consumes JSON produced at the framework's 512-container limit. Keep the dependency direction intact rather than coupling Collections back to Support. Cover collection decoding, Jsonable item serialization, and Arr conversion at the supported boundary and one level beyond it.
Use PHP's native 513 decode depth in both filesystem JSON readers so documents written with 512 nested containers remain readable. Retain the existing flags-controlled error behavior and missing-file contract, with regressions for maximum depth, overflow, malformed input, and throwing mode.
Route Composer file reads and writes through the shared JSON contract so maximum-depth metadata can be read after it is written. Encode before inspecting file mode or replacing bytes, ensuring over-depth or otherwise invalid callback results fail without changing the original file. Cover the supported boundary and byte-for-byte failure preservation.
Align maintenance data, HTTP request and response payloads, JSON sessions, XML normalization, and Inertia test data with the shared nesting contract. Values encoded at 512 nested containers now decode through their owning boundary, while one-level-over values fail at encoding or validation instead of becoming null or unrelated type errors. Existing output shapes and non-throwing session recovery remain unchanged.
Move the serialized-closure command to Concurrency and centralize response-envelope decoding, remote exception reconstruction, binary result handling, and malformed transport errors in SerializedClosureResult. Delegate ProcessDriver and Testbench process results to the shared decoder, declare their direct package dependencies, and move the command, fixture, and process tests to the owning package. Preserve raw non-closure output and transport-specific encoding behavior.
Route request JSON casts and both validator execution paths through the shared JSON contract, removing the unused request encoder and the dead PHP-version fallback. Malformed, empty, and over-depth JSON strings now fail consistently in interpreted and compiled validation before array, collection, object, or JSON casting. Correct the public example and cover the normal validated form-request path.
Give Eloquent's codec matching write and read depth limits, keep contextual model errors for failed encodes, and validate decoded shapes before constructing first-party JSON class casts. Use the existing JSON attribute encoder for path assignments and let valid values replace malformed readable originals without swallowing decryption failures. Cover primitive, encrypted, enum, collection, fluent, data-object, custom codec, and cross-engine repair behavior.
Enable native throwing JSON encoding in base, MySQL, MariaDB, PostgreSQL, and SQLite binding preparation so recursion, non-finite values, and depth failures cannot reach query execution as false. Keep each grammar's existing encoding flags and binding shapes, tighten an adjacent PostgreSQL comparison, and exercise the protected preparation methods directly across all supported grammar families.
Make database show and table commands raise the native JSON error at serialization time instead of passing false into Symfony output. Add focused probes for valid output and non-finite metadata so command rendering preserves its existing format while failures retain their real cause.
Store entries through one readable codec, purge only top-level fields that exceed the entry envelope, and keep exception visibility updates and replacement inserts atomic in deterministic family order. Normalize diagnostic objects with fail-loud encoding, parse application responses once, and unify client request and response masking before size checks. Structured JSON and form bodies can no longer fall through to raw storage with configured secrets, while opaque and explicit text payload behavior remains unchanged. Cover maximum-depth storage, field recovery, failure ordering, exception family state, updates, structured and raw redaction, response parsing, and watcher normalization.
Distinguish missing Composer metadata from malformed or structurally invalid metadata during framework and Testbench package discovery. Validate package names, versions, and extra.hypervel containers only when they are consumed, preserving wildcard and package-specific ignore semantics. Share focused package-name and Hypervel-extra readers without adding a parser abstraction, keep protected formatting parity, and fail before publishing a replacement manifest. Cover root and installed metadata, ignored packages, cache preservation, test-state registration, and subprocess startup diagnostics.
Record the final depth, storage, redaction, Eloquent repair, serialized transport, and package metadata contracts implemented by this branch. Include the verified native JSON behavior, ownership boundaries, anti-overengineering constraints, file map, testing matrix, performance expectations, compatibility notes, and primary references needed to maintain the changes.
📝 WalkthroughWalkthroughThis change centralizes JSON depth and error handling across Hypervel. It updates framework, Eloquent, Telescope, concurrency, validation, and package discovery paths. It adds extensive boundary and failure tests for JSON encoding, decoding, redaction, serialized closures, and Composer metadata. ChangesJSON contract and framework round trips
Eloquent JSON casts and database encoding
Serialized closure result ownership
Telescope JSON handling
Package metadata validation
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Request
participant Json
participant Validation
participant Telescope
Request->>Json: decode bounded JSON payload
Json->>Validation: validate JSON structure and depth
Validation-->>Request: accept or reject input
Request->>Telescope: record structured request and response data
Telescope->>Json: encode masked payload
Json-->>Telescope: encoded content or JSON exception
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Collaborator
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
Greptile SummaryThis PR standardizes framework-owned JSON nesting and failure behavior while making package metadata discovery fail loudly for malformed consumed structures.
Confidence Score: 5/5The PR appears safe to merge; no concrete changed-code defect remained after tracing the principal JSON, persistence, redaction, transport, and discovery paths. The changed boundaries consistently pair framework-owned encoding with readable decoding, preserve intentional external-input behavior, fail before invalid persistence or discovery, and add focused coverage for the new contracts.
|
| Filename | Overview |
|---|---|
| src/support/src/Json.php | Defines the shared nested-container contract, preserves caller flags, and translates public decode/validation depth to PHP’s native unit. |
| src/concurrency/src/SerializedClosureResult.php | Centralizes serialized-closure envelope validation, remote exception reconstruction, strict base64 decoding, and guarded unserialization. |
| src/database/src/Eloquent/Casts/Json.php | Aligns Eloquent’s default encode/decode depth while retaining custom codecs and contextual writer-owned encoding failures. |
| src/database/src/Eloquent/Concerns/HasAttributes.php | Reuses contextual JSON encoding for path assignment and permits valid values to replace malformed readable JSON. |
| src/telescope/src/Storage/DatabaseEntriesRepository.php | Adds readable entry encoding, top-level depth recovery, and atomic exception visibility-update/insert transactions. |
| src/telescope/src/Watchers/ClientRequestWatcher.php | Masks structured payloads before measuring them and prevents malformed declared JSON or form bodies from falling through to raw storage. |
| src/foundation/src/PackageManifest.php | Validates consumed Composer metadata while retaining missing-file and configured-ignore recovery behavior. |
| src/testbench/src/Foundation/PackageManifest.php | Applies the same focused package metadata checks to Testbench root discovery without weakening its array boundary. |
| src/validation/src/Concerns/ValidatesAttributes.php | Routes JSON validation through the shared maximum-nesting contract. |
| src/foundation/src/Http/Traits/HasCasts.php | Makes request JSON casts use the shared throwing decoder and removes the unused encoder helper. |
Reviews (1): Last reviewed commit: "docs: add JSON correctness implementatio..." | Re-trigger Greptile
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
src/concurrency/src/SerializedClosureResult.php (2)
76-83: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueConfirm the trust boundary for
unserialize.Static analysis flags
unserializeon line 77 as deserialization of untrusted data. The value comes from the child process envelope, so the input is trusted only while process stdout is framework-controlled.allowed_classescannot be restricted here, because concurrent tasks legitimately return objects. Document the trust assumption in the class docblock so future callers do not pass external output intodecode().🤖 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/concurrency/src/SerializedClosureResult.php` around lines 76 - 83, Document the trust boundary for unserialize in the SerializedClosureResult class docblock: decode() must receive only framework-controlled child-process envelope output, not external or user-provided data. Explicitly note that concurrent task results may contain objects and therefore allowed_classes cannot be restricted; leave the existing decode logic unchanged.Source: Linters/SAST tools
43-65: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winValidate the exception class before you instantiate it.
Line 55 instantiates any class name found in the envelope, with envelope-supplied arguments. If the class is not a
Throwable, the constructor still runs, and only then does line 60 reject the object. Constructors of unrelated classes can have side effects.Check the class name first. This also removes the need for the post-construction
instanceofcheck.🛡️ Proposed guard
$exceptionClass = $payload['exception'] ?? RuntimeException::class; $message = $payload['message'] ?? 'Serialized closure execution failed.'; $parameters = $payload['parameters'] ?? ['message' => $message]; + if (! is_a($exceptionClass, Throwable::class, true)) { + throw new RuntimeException($message); + } + try { $exception = new $exceptionClass(...$parameters); } catch (Throwable $constructionException) { throw new RuntimeException($message, previous: $constructionException); } - - if (! $exception instanceof Throwable) { - throw new RuntimeException($message); - } throw $exception;Note:
tests/Concurrency/SerializedClosureResultTest.phplines 207-234 assert that a missing class produces a non-null previous exception, and thatstdClassproduces aRuntimeExceptionwith the transported message. With this guard, both cases take theis_abranch and carry no previous exception. UpdatetestItContainsUnavailableExceptionClassesDuringReconstructionaccordingly if you apply the change.🤖 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/concurrency/src/SerializedClosureResult.php` around lines 43 - 65, Validate the envelope’s exception class with is_a(..., Throwable::class, true) before constructing it in SerializedClosureResult; for invalid or unavailable classes, throw RuntimeException with the transported message and do not instantiate them. Remove the post-construction instanceof check, and update testItContainsUnavailableExceptionClassesDuringReconstruction to expect no previous exception for missing classes and stdClass.Source: Linters/SAST tools
tests/Concurrency/SerializedClosureResultTest.php (2)
244-260: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a rejection test for parameters that exceed the transport depth.
The suite proves that 510 containers reconstruct. It does not prove that the decoder rejects a deeper envelope. Add a case with 511 containers to lock the boundary at the decoder, matching
testItDegradesExceptionParametersBeyondTheTransportDepthintests/Concurrency/Console/InvokeSerializedClosureCommandTest.php.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/Concurrency/SerializedClosureResultTest.php` around lines 244 - 260, Add a test alongside testItReconstructsTheMaximumExceptionParameterDepth using nestedValue(511), then assert decodePayload rejects or degrades the exception parameters according to the existing transport-depth behavior, matching testItDegradesExceptionParametersBeyondTheTransportDepth. Keep the 510-container reconstruction test unchanged to lock the boundary.
47-66: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMove
$this->fail()outside the matchingtryblocks.PHPUnit\Framework\AssertionFailedErrorextendsRuntimeException, so handlers that catchRuntimeExceptionorExceptionswallow the failure and report misleading diagnostics. Apply this to all listed blocks except lines 146-164, whosecatch (ErrorException)does not catch PHPUnit assertion failures.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/Concurrency/SerializedClosureResultTest.php` around lines 47 - 66, Move each $this->fail() call outside the try blocks that catch RuntimeException or Exception in SerializedClosureResultTest::testItRejectsInvalidResponseEnvelopes and the listed blocks in InvokeSerializedClosureCommandTest.php (lines 231-237); retain the existing catch assertions, and make no change to lines 146-164 because ErrorException does not catch PHPUnit assertion failures.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/database/src/Eloquent/Casts/AsArrayObject.php`:
- Around line 35-41: Translate JsonException during persistence encoding into
JsonEncodingException::forAttribute using the exception message, replacing
ineffective false-result checks. Apply this in AsArrayObject.php#L35-L41,
AsCollection.php#L65-L71, AsDataObject.php#L61-L67,
AsEncryptedArrayObject.php#L37-L43 before encryption,
AsEncryptedCollection.php#L67-L73 before encryption, and
AsEnumArrayObject.php#L69-L75; preserve each cast’s existing persistence flow
after successful encoding.
In `@src/foundation/src/Http/Traits/HasCasts.php`:
- Around line 388-390: Restore the protected asJson helper in the HasCasts trait
and implement it by delegating to Json::encode, preserving the existing
protected extension surface for classes using the trait.
In `@src/telescope/src/Storage/DatabaseEntriesRepository.php`:
- Around line 183-214: The occurrence count and row construction currently
happen before the transaction, allowing concurrent stores to use the same count.
Move the per-family counting and row construction into the transaction in the
repository method containing countExceptionOccurences, and acquire family locks
in deterministic order before counting so concurrent stores serialize correctly
while preserving existing display-flag updates and inserts; add a regression
test covering concurrent stores for one family.
In `@tests/Testing/TestResponseTest.php`:
- Around line 493-505: Update
testDumpDecodesJsonAsObjectsAndPreservesInvalidBytes to capture the previous
VarDumper handler returned by VarDumper::setHandler before installing the test
callback, then restore that captured handler in the finally block instead of
setting it to null.
---
Nitpick comments:
In `@src/concurrency/src/SerializedClosureResult.php`:
- Around line 76-83: Document the trust boundary for unserialize in the
SerializedClosureResult class docblock: decode() must receive only
framework-controlled child-process envelope output, not external or
user-provided data. Explicitly note that concurrent task results may contain
objects and therefore allowed_classes cannot be restricted; leave the existing
decode logic unchanged.
- Around line 43-65: Validate the envelope’s exception class with is_a(...,
Throwable::class, true) before constructing it in SerializedClosureResult; for
invalid or unavailable classes, throw RuntimeException with the transported
message and do not instantiate them. Remove the post-construction instanceof
check, and update testItContainsUnavailableExceptionClassesDuringReconstruction
to expect no previous exception for missing classes and stdClass.
In `@tests/Concurrency/SerializedClosureResultTest.php`:
- Around line 244-260: Add a test alongside
testItReconstructsTheMaximumExceptionParameterDepth using nestedValue(511), then
assert decodePayload rejects or degrades the exception parameters according to
the existing transport-depth behavior, matching
testItDegradesExceptionParametersBeyondTheTransportDepth. Keep the 510-container
reconstruction test unchanged to lock the boundary.
- Around line 47-66: Move each $this->fail() call outside the try blocks that
catch RuntimeException or Exception in
SerializedClosureResultTest::testItRejectsInvalidResponseEnvelopes and the
listed blocks in InvokeSerializedClosureCommandTest.php (lines 231-237); retain
the existing catch assertions, and make no change to lines 146-164 because
ErrorException does not catch PHPUnit assertion failures.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 4a913411-c432-4c95-90a0-2f14a189dea2
📒 Files selected for processing (96)
docs/plans/2026-08-11-0932-json-correctness-and-package-metadata.mdsrc/collections/src/Arr.phpsrc/collections/src/Traits/EnumeratesValues.phpsrc/concurrency/composer.jsonsrc/concurrency/src/Console/InvokeSerializedClosureCommand.phpsrc/concurrency/src/ProcessDriver.phpsrc/concurrency/src/SerializedClosureResult.phpsrc/database/src/Console/ShowCommand.phpsrc/database/src/Console/TableCommand.phpsrc/database/src/Eloquent/Casts/AsArrayObject.phpsrc/database/src/Eloquent/Casts/AsCollection.phpsrc/database/src/Eloquent/Casts/AsDataObject.phpsrc/database/src/Eloquent/Casts/AsEncryptedArrayObject.phpsrc/database/src/Eloquent/Casts/AsEncryptedCollection.phpsrc/database/src/Eloquent/Casts/AsEnumArrayObject.phpsrc/database/src/Eloquent/Casts/AsEnumCollection.phpsrc/database/src/Eloquent/Casts/AsFluent.phpsrc/database/src/Eloquent/Casts/Json.phpsrc/database/src/Eloquent/Concerns/HasAttributes.phpsrc/database/src/Query/Grammars/Grammar.phpsrc/database/src/Query/Grammars/MySqlGrammar.phpsrc/database/src/Query/Grammars/PostgresGrammar.phpsrc/database/src/Query/Grammars/SQLiteGrammar.phpsrc/docs/validation.mdsrc/filesystem/src/Filesystem.phpsrc/filesystem/src/FilesystemAdapter.phpsrc/foundation/src/FileBasedMaintenanceMode.phpsrc/foundation/src/Http/Traits/HasCasts.phpsrc/foundation/src/PackageManifest.phpsrc/foundation/src/Providers/FoundationServiceProvider.phpsrc/http/src/Client/Request.phpsrc/http/src/JsonResponse.phpsrc/inertia/src/Testing/AssertableInertia.phpsrc/session/src/Store.phpsrc/support/src/Composer.phpsrc/support/src/Json.phpsrc/support/src/Str.phpsrc/support/src/Xml.phpsrc/telescope/src/ExtractProperties.phpsrc/telescope/src/Storage/DatabaseEntriesRepository.phpsrc/telescope/src/Watchers/ClientRequestWatcher.phpsrc/telescope/src/Watchers/EventWatcher.phpsrc/telescope/src/Watchers/ModelWatcher.phpsrc/telescope/src/Watchers/RequestWatcher.phpsrc/testbench/composer.jsonsrc/testbench/src/Foundation/PackageManifest.phpsrc/testbench/src/Foundation/Process/ProcessResult.phpsrc/testing/src/AssertableJsonString.phpsrc/testing/src/TestResponse.phpsrc/validation/src/Concerns/ValidatesAttributes.phpsrc/validation/src/PlanExecutor.phptests/Concurrency/ConcurrencyTest.phptests/Concurrency/Console/InvokeSerializedClosureCommandTest.phptests/Concurrency/Fixtures/ConcurrentProcessExceptionFixtures.phptests/Concurrency/PackageMetadataTest.phptests/Concurrency/SerializedClosureResultTest.phptests/Database/DatabaseConsoleJsonTest.phptests/Database/DatabaseEloquentJsonCastTest.phptests/Database/DatabaseMariaDbQueryGrammarTest.phptests/Database/DatabaseMySqlQueryGrammarTest.phptests/Database/DatabasePostgresQueryGrammarTest.phptests/Database/DatabaseQueryGrammarTest.phptests/Database/DatabaseSQLiteQueryGrammarTest.phptests/Filesystem/FilesystemAdapterTest.phptests/Filesystem/FilesystemTest.phptests/Foundation/FoundationFileBasedMaintenanceModeTest.phptests/Foundation/FoundationPackageManifestTest.phptests/Foundation/Http/CustomCastingTest.phptests/Http/HttpClientTest.phptests/Http/HttpJsonResponseTest.phptests/Inertia/Testing/AssertableInertiaTest.phptests/Integration/Database/EloquentModelEncryptedCastingTest.phptests/Integration/Database/EloquentModelJsonCastingTest.phptests/Session/SessionStoreTest.phptests/Support/ComposerFileTest.phptests/Support/JsonTest.phptests/Support/SupportArrTest.phptests/Support/SupportCollectionTest.phptests/Support/SupportStrTest.phptests/Support/SupportStringableTest.phptests/Support/XmlTest.phptests/Telescope/ExtractPropertiesTest.phptests/Telescope/Storage/DatabaseEntriesRepositoryTest.phptests/Telescope/Watchers/ClientRequestWatcherTest.phptests/Telescope/Watchers/EventWatcherTest.phptests/Telescope/Watchers/ModelWatcherTest.phptests/Telescope/Watchers/RequestWatchersTest.phptests/Testbench/Foundation/Fixtures/PackageManifest/build-manifest.phptests/Testbench/Foundation/PackageManifestPackageTesterTest.phptests/Testbench/Foundation/PackageManifestTest.phptests/Testbench/Foundation/Process/ProcessResultTest.phptests/Testbench/PackageMetadataTest.phptests/Testing/PHPUnit/TestStateRegistrarsTest.phptests/Testing/TestResponseTest.phptests/Validation/ValidationPlanExecutorTest.phptests/Validation/ValidationValidatorTest.php
Comment on lines
+35
to
+41
| $encoded = Json::encode($value); | ||
|
|
||
| if ($encoded === false) { | ||
| throw JsonEncodingException::forAttribute($model, $key, json_last_error_msg()); | ||
| } | ||
|
|
||
| return [$key => $encoded]; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Translate the thrown JSON exception before persistence.
Json::encode() returns string and throws JsonException. The === false checks never run. Encoding failures therefore leak JsonException instead of the required model and attribute-specific JsonEncodingException.
src/database/src/Eloquent/Casts/AsArrayObject.php#L35-L41: CatchJsonExceptionand throwJsonEncodingException::forAttribute($model, $key, $exception->getMessage()).src/database/src/Eloquent/Casts/AsCollection.php#L65-L71: CatchJsonExceptionand throwJsonEncodingException::forAttribute($model, $key, $exception->getMessage()).src/database/src/Eloquent/Casts/AsDataObject.php#L61-L67: CatchJsonExceptionand throwJsonEncodingException::forAttribute($model, $key, $exception->getMessage()).src/database/src/Eloquent/Casts/AsEncryptedArrayObject.php#L37-L43: CatchJsonExceptionbefore encryption and throw the contextual exception.src/database/src/Eloquent/Casts/AsEncryptedCollection.php#L67-L73: CatchJsonExceptionbefore encryption and throw the contextual exception.src/database/src/Eloquent/Casts/AsEnumArrayObject.php#L69-L75: CatchJsonExceptionand throw the contextual exception.
📍 Affects 6 files
src/database/src/Eloquent/Casts/AsArrayObject.php#L35-L41(this comment)src/database/src/Eloquent/Casts/AsCollection.php#L65-L71src/database/src/Eloquent/Casts/AsDataObject.php#L61-L67src/database/src/Eloquent/Casts/AsEncryptedArrayObject.php#L37-L43src/database/src/Eloquent/Casts/AsEncryptedCollection.php#L67-L73src/database/src/Eloquent/Casts/AsEnumArrayObject.php#L69-L75
🤖 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/database/src/Eloquent/Casts/AsArrayObject.php` around lines 35 - 41,
Translate JsonException during persistence encoding into
JsonEncodingException::forAttribute using the exception message, replacing
ineffective false-result checks. Apply this in AsArrayObject.php#L35-L41,
AsCollection.php#L65-L71, AsDataObject.php#L61-L67,
AsEncryptedArrayObject.php#L37-L43 before encryption,
AsEncryptedCollection.php#L67-L73 before encryption, and
AsEnumArrayObject.php#L69-L75; preserve each cast’s existing persistence flow
after successful encoding.
Comment on lines
+388
to
+390
| public function fromJson(string $value, bool $asObject = false): mixed | ||
| { | ||
| return json_decode($value, ! $asObject); | ||
| return Json::decode($value, ! $asObject); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Restore the protected asJson helper.
The change removes asJson from this trait. A class that uses HasCasts can call this protected method. That class will fail at runtime after this change.
Keep the helper and delegate its implementation to Json::encode. This preserves the protected extension surface while using the shared JSON contract.
Proposed fix
+ protected function asJson(mixed $value): string
+ {
+ return Json::encode($value);
+ }
+
public function fromJson(string $value, bool $asObject = false): mixed📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| public function fromJson(string $value, bool $asObject = false): mixed | |
| { | |
| return json_decode($value, ! $asObject); | |
| return Json::decode($value, ! $asObject); | |
| protected function asJson(mixed $value): string | |
| { | |
| return Json::encode($value); | |
| } | |
| public function fromJson(string $value, bool $asObject = false): mixed | |
| { | |
| return Json::decode($value, ! $asObject); |
🤖 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/foundation/src/Http/Traits/HasCasts.php` around lines 388 - 390, Restore
the protected asJson helper in the HasCasts trait and implement it by delegating
to Json::encode, preserving the existing protected extension surface for classes
using the trait.
Comment on lines
+183
to
+214
| $families | ||
| ->each(function ($family, $familyHash) use (&$occurrences, &$lastUuids): void { | ||
| $occurrences[$familyHash] = $this->countExceptionOccurences($family->first()); | ||
| $lastUuids[$familyHash] = $family->last()->uuid; | ||
|
|
||
| $this->table('telescope_entries') | ||
| ->where('type', EntryType::EXCEPTION) | ||
| ->where('family_hash', $familyHash) | ||
| ->where('should_display_on_index', true) | ||
| ->update(['should_display_on_index' => false]); | ||
| }); | ||
|
|
||
| $this->table('telescope_entries')->insert($chunked->map(function ($exception) use (&$occurrences, $lastUuids) { | ||
| $rows = $chunked->map(function ($exception) use (&$occurrences, $lastUuids) { | ||
| $familyHash = $exception->familyHash(); | ||
| ++$occurrences[$familyHash]; | ||
|
|
||
| return array_merge($exception->toArray(), [ | ||
| 'family_hash' => $familyHash, | ||
| 'should_display_on_index' => $exception->uuid === $lastUuids[$familyHash], | ||
| 'content' => json_encode( | ||
| array_merge($exception->content, ['occurrences' => $occurrences[$familyHash]]), | ||
| JSON_INVALID_UTF8_SUBSTITUTE | ||
| 'content' => $this->encodeContent( | ||
| array_merge($exception->content, ['occurrences' => $occurrences[$familyHash]]) | ||
| ), | ||
| ]); | ||
| })->toArray()); | ||
| })->toArray(); | ||
|
|
||
| $connection = DB::connection($this->connection); | ||
|
|
||
| $connection->transaction(function () use ($connection, $families, $rows): void { | ||
| $families->each(function ($family, $familyHash) use ($connection): void { | ||
| $connection->table('telescope_entries') | ||
| ->where('type', EntryType::EXCEPTION) | ||
| ->where('family_hash', $familyHash) | ||
| ->where('should_display_on_index', true) | ||
| ->update(['should_display_on_index' => false]); | ||
| }); | ||
|
|
||
| $connection->table('telescope_entries')->insert($rows); | ||
| }); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Calculate exception occurrences inside a per-family transaction.
Lines 183-200 calculate each family count before Line 204 starts the transaction. Two concurrent stores of the same family can both read occurrence n and store entries with occurrence n + 1.
Start the count and row construction inside a transaction that serializes each family. Keep the deterministic family ordering when acquiring locks. Add a concurrent-store regression test.
🤖 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/telescope/src/Storage/DatabaseEntriesRepository.php` around lines 183 -
214, The occurrence count and row construction currently happen before the
transaction, allowing concurrent stores to use the same count. Move the
per-family counting and row construction into the transaction in the repository
method containing countExceptionOccurences, and acquire family locks in
deterministic order before counting so concurrent stores serialize correctly
while preserving existing display-flag updates and inserts; add a regression
test covering concurrent stores for one family.
Comment on lines
+493
to
+505
| public function testDumpDecodesJsonAsObjectsAndPreservesInvalidBytes(): void | ||
| { | ||
| $dumped = []; | ||
| VarDumper::setHandler(function (mixed $value) use (&$dumped): void { | ||
| $dumped[] = $value; | ||
| }); | ||
|
|
||
| try { | ||
| TestResponse::fromBaseResponse(new Response('{"nested":{"value":1}}'))->dump(); | ||
| TestResponse::fromBaseResponse(new Response('{invalid'))->dump(); | ||
| } finally { | ||
| VarDumper::setHandler(null); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
For Symfony VarDumper 8.1, does VarDumper::setHandler() return the previously configured handler so callers can restore it?
💡 Result:
Yes, in Symfony VarDumper 8.1, the VarDumper::setHandler method returns the previously configured handler (or null if none was set), allowing callers to store and restore it [1][2]. The implementation of the setHandler method in the VarDumper component explicitly captures the current handler before updating it: public static function setHandler(?callable $callable):?callable { $prevHandler = self::$handler; // Prevent replacing the handler with expected format as soon as the env var was set: if (isset($_SERVER['VAR_DUMPER_FORMAT'])) { return $prevHandler; } self::$handler = $callable; return $prevHandler; } As shown above, the method returns $prevHandler, which contains the previously registered callable, enabling the restoration of the prior behavior if needed [1][2].
Citations:
- 1: https://github.com/symfony/symfony/blob/8.1/src/Symfony/Component/VarDumper/VarDumper.php
- 2: https://github.com/symfony/var-dumper/blob/8.1/VarDumper.php
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- test context ---'
sed -n '480,515p' tests/Testing/TestResponseTest.php
printf '%s\n' '--- VarDumper dependency declarations ---'
rg -n -S 'symfony/var-dumper|VarDumper::setHandler' composer.json composer.lock tests src 2>/dev/null | head -120Repository: hypervel/components-backup
Length of output: 4321
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- existing handler restoration pattern ---'
sed -n '95,200p' tests/Telescope/Watchers/DumpWatcherTest.php
printf '%s\n' '--- VarDumper API/version metadata ---'
rg -n -S '"name": "symfony/var-dumper"|"version": "v?8\.1|setHandler' composer.lock vendor/symfony/var-dumper/VarDumper.php 2>/dev/null | head -80 || trueRepository: hypervel/components-backup
Length of output: 3470
Restore the previous VarDumper handler.
VarDumper::setHandler() returns the previous handler. Store it before installing the test handler, then restore it in finally.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/Testing/TestResponseTest.php` around lines 493 - 505, Update
testDumpDecodesJsonAsObjectsAndPreservesInvalidBytes to capture the previous
VarDumper handler returned by VarDumper::setHandler before installing the test
callback, then restore that captured handler in the finally block instead of
setting it to null.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
This PR makes JSON nesting and failure behavior consistent across framework-owned storage, transport, validation, diagnostics, and package discovery.
The main rule is simple: a value accepted at Hypervel's public maximum of 512 nested containers must remain readable by the matching framework boundary. PHP uses different depth units for encoding and decoding, so a value encoded with depth 512 requires native decode depth 513. Several framework paths used 512 for both and could turn valid data into null, false, an empty result, or an unrelated type error.
This change defines that contract once in Support Json and applies it only where Hypervel owns both sides of the round trip. External input and protocol-specific readers keep their existing contracts.
What changed
Shared JSON behavior
Concurrency transport
Request validation
Eloquent and database boundaries
Telescope
Package discovery
Compatibility and cost
Public and protected Laravel-style surfaces are preserved. Explicit native depth arguments on APIs that already expose native PHP semantics remain native. Eloquent custom codecs, stored empty-string handling, filesystem flags, session recovery, raw process output, and opaque Telescope payload behavior remain intact.
Normal JSON reads add one branch and integer increment. Telescope storage still encodes once on success; field-by-field recovery runs only after a depth error. Structured payloads still mask and encode once. The exception transaction is limited to exception chunks. The change adds no cache, registry, retry loop, container lookup, worker state, or general successful-path preflight.
Malformed package metadata now fails loudly by design. Missing metadata remains supported.
Testing
The branch includes focused regressions for every changed boundary, including depth limits, native flags and errors, Eloquent custom and encrypted casts, repair behavior, query grammars, Telescope storage and redaction, serialized closure envelopes, and package discovery.
Verification includes the full Components fix pipeline, Testbench and dogfood suites, focused package suites, and the SQLite, PostgreSQL, MySQL, and MariaDB integration matrices.
Summary by CodeRabbit
New Features
Bug Fixes