Skip to content

Fix JSON correctness and package metadata discovery - #14

Closed
binaryfire wants to merge 15 commits into
0.4from
fix/json-correctness
Closed

Fix JSON correctness and package metadata discovery#14
binaryfire wants to merge 15 commits into
0.4from
fix/json-correctness

Conversation

@binaryfire

@binaryfire binaryfire commented Aug 11, 2026

Copy link
Copy Markdown
Collaborator

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

  • Support Json now treats depth as a nested-container limit and translates it for native decode and validation calls.
  • Caller flags are preserved for encode and decode.
  • Jsonable values receive caller flags plus throwing behavior while retaining ownership of their own depth.
  • Str JSON predicates and framework test-response readers use the same validation and decode contract.
  • Collections, Filesystem, Composer files, maintenance data, HTTP JSON responses, outgoing client requests, JSON sessions, XML normalization, and Inertia test data now preserve framework-owned maximum-depth round trips.

Concurrency transport

  • Serialized closure command ownership moves from Foundation to Concurrency.
  • SerializedClosureResult owns envelope validation, remote exception reconstruction, binary result decoding, gzip-marker handling, and unserialization failures.
  • ProcessDriver and Testbench delegate to the same decoder.
  • Package dependencies and tests move with the implementation.

Request validation

  • Request JSON casts and both validator execution paths use the shared JSON contract.
  • Malformed, empty, and over-depth JSON fail consistently before array, collection, object, or JSON casting.
  • The unused request JSON encoder and obsolete validation fallback are removed.
  • The validation documentation now demonstrates JSON-string input for JSON-backed casts.

Eloquent and database boundaries

  • Eloquent JSON encoding and decoding use matching depth limits while preserving model and attribute context on write failures.
  • First-party JSON class casts reject failed encodes before storage or encryption and validate decoded shapes before construction.
  • JSON path assignment uses the existing attribute encoder instead of duplicating its failure handling.
  • A valid assignment can repair malformed readable JSON, while decryption failures remain fail-loud and cannot authorize overwrite.
  • Query grammars for MySQL, MariaDB, PostgreSQL, and SQLite reject invalid JSON bindings before query execution.
  • Database console commands report native JSON errors instead of passing false into output rendering.

Telescope

  • Stored entries use one readable codec.
  • A depth-overflowing top-level field is replaced with Telescope's existing purge marker while unrelated fields remain available.
  • Non-depth encoding failures remain fail-loud.
  • Exception visibility updates and replacement inserts run atomically in deterministic family order.
  • Structured request and response payloads are masked before size checks.
  • JSON and URL-encoded request bodies cannot fall through to raw storage with configured secrets.
  • Opaque bodies and explicit plain text keep their existing representation.
  • Application responses are decoded once, and deep structured responses are purged without making the entry itself unreadable.

Package discovery

  • Missing Composer metadata remains a supported empty state.
  • Malformed syntax and invalid consumed structures now fail package discovery instead of silently publishing an incomplete cache.
  • Root wildcard and package-specific ignore behavior is preserved and runs before metadata that the application chose not to consume.
  • Package names, versions, and extra.hypervel containers produce path-specific errors.
  • Framework and Testbench discovery share the same focused metadata checks without adding a general parser layer.

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

    • Added consistent JSON encoding, decoding, validation, and nesting-depth handling across framework features.
    • JSON failures now surface as clear exceptions instead of silent invalid results.
    • Added stricter package metadata validation and clearer discovery failures.
    • Improved serialized task result handling, including remote exceptions and malformed responses.
    • Telescope now limits, masks, truncates, or purges unsafe and overly deep payloads.
  • Bug Fixes

    • Improved JSON casting, request handling, sessions, filesystem operations, database queries, and maintenance-mode round trips.
    • Preserved valid data when replacing malformed stored JSON.

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.
@coderabbitai

coderabbitai Bot commented Aug 11, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 1bbd5b8b-e1df-44db-974e-59a4ce81d081

📥 Commits

Reviewing files that changed from the base of the PR and between f8fb1d7 and e33ba4d.

📒 Files selected for processing (7)
  • docs/plans/2026-08-11-0932-json-correctness-and-package-metadata.md
  • src/concurrency/src/SerializedClosureResult.php
  • tests/Concurrency/ConcurrencyTest.php
  • tests/Concurrency/Console/InvokeSerializedClosureCommandTest.php
  • tests/Concurrency/SerializedClosureResultTest.php
  • tests/Testbench/Foundation/PackageManifestPackageTesterTest.php
  • tests/Testbench/Foundation/Process/ProcessResultTest.php
🚧 Files skipped from review as they are similar to previous changes (5)
  • src/concurrency/src/SerializedClosureResult.php
  • tests/Testbench/Foundation/Process/ProcessResultTest.php
  • tests/Testbench/Foundation/PackageManifestPackageTesterTest.php
  • docs/plans/2026-08-11-0932-json-correctness-and-package-metadata.md
  • tests/Concurrency/Console/InvokeSerializedClosureCommandTest.php

📝 Walkthrough

Walkthrough

This change centralizes JSON depth and error handling across Hypervel. It updates framework, Eloquent, Telescope, concurrency, validation, and package discovery paths. It adds boundary and failure tests for JSON handling, serialized closures, redaction, and Composer metadata.

Changes

JSON contract and framework round trips

Layer / File(s) Summary
Shared JSON contract and consumers
src/support/src/Json.php, src/collections/..., src/filesystem/..., src/http/..., src/session/..., src/validation/..., src/testing/..., src/inertia/...
Json defines shared depth conversion, throwing encoding, decoding, and validation. Framework consumers use the shared helper and support the configured nesting boundary.
JSON boundary tests and documentation
tests/Support/..., tests/Filesystem/..., tests/Http/..., tests/Session/..., tests/Validation/..., tests/Testing/..., src/docs/validation.md
Tests cover maximum depth, over-depth input, malformed JSON, flags, invalid UTF-8, and round trips. Validation documentation uses JSON casts and rules.

Eloquent JSON casts and database encoding

Layer / File(s) Summary
Eloquent cast contracts and repair behavior
src/database/src/Eloquent/Casts/*, src/database/src/Eloquent/Concerns/HasAttributes.php
First-party casts require Model parameters, validate decoded shapes, and throw contextual JsonEncodingException values. Dirty checking handles malformed JSON while preserving decryption failures.
Database grammar and console failures
src/database/src/Query/Grammars/*, src/database/src/Console/*
JSON binding and console output encoding now use JSON_THROW_ON_ERROR. PostgreSQL also uses strict empty-clause comparison.
Eloquent and grammar tests
tests/Database/..., tests/Integration/Database/...
Tests cover cast codecs, malformed stored JSON replacement, encrypted values, JSON-path assignment, nesting limits, and unencodable database bindings.

Serialized closure result ownership

Layer / File(s) Summary
Shared serialized-result decoder
src/concurrency/src/SerializedClosureResult.php, src/concurrency/src/ProcessDriver.php, src/testbench/src/Foundation/Process/ProcessResult.php
Serialized closure decoding is centralized. The decoder validates envelopes, removes trailing gzip data, reconstructs remote exceptions, and unserializes successful results.
Concurrency wiring and tests
src/concurrency/src/Console/..., src/foundation/src/Providers/..., src/concurrency/composer.json, tests/Concurrency/..., tests/Testbench/Foundation/Process/...
The command moves to the concurrency namespace. Dependencies and provider registration are updated. Tests cover result values, malformed payloads, exception reconstruction, depth degradation, and child-process failures.

Telescope JSON handling

Layer / File(s) Summary
Storage and watcher behavior
src/telescope/src/Storage/..., src/telescope/src/Watchers/..., src/telescope/src/ExtractProperties.php
Telescope uses bounded JSON encoding and decoding. Depth-invalid top-level fields are purged. Request and response payloads are masked, size-limited, and purged when malformed or over depth. Exception updates use transactional persistence.
Telescope tests
tests/Telescope/...
Tests cover storage round trips, purge behavior, exception-family state, request redaction, malformed bodies, opaque responses, event payloads, hydration updates, and encoding failures.

Package metadata validation

Layer / File(s) Summary
Foundation and Testbench metadata parsing
src/foundation/src/PackageManifest.php, src/testbench/src/Foundation/PackageManifest.php, src/support/src/Composer.php
Composer metadata parsing fails for malformed or structurally invalid documents. Package names, versions, and Hypervel configuration are validated while missing files retain existing behavior.
Manifest subprocesses and tests
tests/Foundation/FoundationPackageManifestTest.php, tests/Testbench/Foundation/..., tests/Testing/PHPUnit/TestStateRegistrarsTest.php, src/testbench/composer.json, tests/Testbench/PackageMetadataTest.php
Tests cover invalid metadata, ignored packages, cache behavior, manifest preservation after failure, isolated subprocess paths, registrar blocking, and package dependency metadata.

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
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 69.61% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely summarizes the two primary changes: JSON correctness and package metadata discovery.
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 💡 2
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/json-correctness

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.

@binaryfire

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 11, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@greptile-apps

greptile-apps Bot commented Aug 11, 2026

Copy link
Copy Markdown

Greptile Summary

This PR standardizes framework-owned JSON nesting and failure behavior while tightening serialized concurrency results, Eloquent JSON handling, Telescope storage/redaction, and Composer package discovery.

  • Introduces a shared nested-container depth contract for framework-owned JSON round trips.
  • Centralizes serialized-closure result decoding and validation in Concurrency.
  • Makes Eloquent, validation, database, and package-discovery JSON failures explicit.
  • Adds depth-aware Telescope recovery, structured-payload masking, and atomic exception replacement.
  • Expands focused regression coverage across the affected components.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
src/support/src/Json.php Defines the shared nested-container contract, preserves caller flags, and translates decode/validation depth to native PHP units.
src/concurrency/src/SerializedClosureResult.php Centralizes serialized result envelope validation, remote exception reconstruction, binary decoding, and guarded unserialization.
src/database/src/Eloquent/Casts/Json.php Aligns default Eloquent encode/decode depths while preserving custom codec and empty-string conventions.
src/database/src/Eloquent/Concerns/HasAttributes.php Reuses contextual JSON encoding for path writes and permits valid assignments to replace malformed readable originals.
src/telescope/src/Storage/DatabaseEntriesRepository.php Adds field-level depth recovery and transactional exception visibility replacement without hiding non-depth failures.
src/telescope/src/Watchers/ClientRequestWatcher.php Masks structured payloads before sizing and purges malformed declared JSON rather than retaining raw secrets.
src/foundation/src/PackageManifest.php Distinguishes missing metadata from malformed consumed structures and provides path-specific discovery failures.
src/testbench/src/Foundation/PackageManifest.php Applies the same focused package metadata checks to Testbench root-package discovery.
src/validation/src/PlanExecutor.php Uses the shared JSON validation depth contract in the compiled validator path.
src/foundation/src/Http/Traits/HasCasts.php Routes request JSON casts through throwing shared decoding so malformed inputs fail consistently.

Reviews (2): Last reviewed commit: "fix(concurrency): validate transported e..." | Re-trigger Greptile

Read both subprocess output streams when asserting native PHP failures and reporting unexpected manifest-build exits.

PHP routes displayed fatal errors according to its runtime configuration: the CI image writes them to stdout while local logging also supplies stderr. Keep the test focused on the nonzero exit, useful diagnostic, and absent manifest rather than a php.ini-dependent stream. Update the implementation plan to match.

@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 (4)
src/concurrency/src/SerializedClosureResult.php (2)

76-83: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low value

Confirm the trust boundary for unserialize.

Static analysis flags unserialize on 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_classes cannot 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 into decode().

🤖 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 win

Validate 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 instanceof check.

🛡️ 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.php lines 207-234 assert that a missing class produces a non-null previous exception, and that stdClass produces a RuntimeException with the transported message. With this guard, both cases take the is_a branch and carry no previous exception. Update testItContainsUnavailableExceptionClassesDuringReconstruction accordingly 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 win

Add 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 testItDegradesExceptionParametersBeyondTheTransportDepth in tests/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 value

Move $this->fail() outside the matching try blocks. PHPUnit\Framework\AssertionFailedError extends RuntimeException, so handlers that catch RuntimeException or Exception swallow the failure and report misleading diagnostics. Apply this to all listed blocks except lines 146-164, whose catch (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

📥 Commits

Reviewing files that changed from the base of the PR and between 185017d and f8fb1d7.

📒 Files selected for processing (96)
  • docs/plans/2026-08-11-0932-json-correctness-and-package-metadata.md
  • src/collections/src/Arr.php
  • src/collections/src/Traits/EnumeratesValues.php
  • src/concurrency/composer.json
  • src/concurrency/src/Console/InvokeSerializedClosureCommand.php
  • src/concurrency/src/ProcessDriver.php
  • src/concurrency/src/SerializedClosureResult.php
  • src/database/src/Console/ShowCommand.php
  • src/database/src/Console/TableCommand.php
  • src/database/src/Eloquent/Casts/AsArrayObject.php
  • src/database/src/Eloquent/Casts/AsCollection.php
  • src/database/src/Eloquent/Casts/AsDataObject.php
  • src/database/src/Eloquent/Casts/AsEncryptedArrayObject.php
  • src/database/src/Eloquent/Casts/AsEncryptedCollection.php
  • src/database/src/Eloquent/Casts/AsEnumArrayObject.php
  • src/database/src/Eloquent/Casts/AsEnumCollection.php
  • src/database/src/Eloquent/Casts/AsFluent.php
  • src/database/src/Eloquent/Casts/Json.php
  • src/database/src/Eloquent/Concerns/HasAttributes.php
  • src/database/src/Query/Grammars/Grammar.php
  • src/database/src/Query/Grammars/MySqlGrammar.php
  • src/database/src/Query/Grammars/PostgresGrammar.php
  • src/database/src/Query/Grammars/SQLiteGrammar.php
  • src/docs/validation.md
  • src/filesystem/src/Filesystem.php
  • src/filesystem/src/FilesystemAdapter.php
  • src/foundation/src/FileBasedMaintenanceMode.php
  • src/foundation/src/Http/Traits/HasCasts.php
  • src/foundation/src/PackageManifest.php
  • src/foundation/src/Providers/FoundationServiceProvider.php
  • src/http/src/Client/Request.php
  • src/http/src/JsonResponse.php
  • src/inertia/src/Testing/AssertableInertia.php
  • src/session/src/Store.php
  • src/support/src/Composer.php
  • src/support/src/Json.php
  • src/support/src/Str.php
  • src/support/src/Xml.php
  • src/telescope/src/ExtractProperties.php
  • src/telescope/src/Storage/DatabaseEntriesRepository.php
  • src/telescope/src/Watchers/ClientRequestWatcher.php
  • src/telescope/src/Watchers/EventWatcher.php
  • src/telescope/src/Watchers/ModelWatcher.php
  • src/telescope/src/Watchers/RequestWatcher.php
  • src/testbench/composer.json
  • src/testbench/src/Foundation/PackageManifest.php
  • src/testbench/src/Foundation/Process/ProcessResult.php
  • src/testing/src/AssertableJsonString.php
  • src/testing/src/TestResponse.php
  • src/validation/src/Concerns/ValidatesAttributes.php
  • src/validation/src/PlanExecutor.php
  • tests/Concurrency/ConcurrencyTest.php
  • tests/Concurrency/Console/InvokeSerializedClosureCommandTest.php
  • tests/Concurrency/Fixtures/ConcurrentProcessExceptionFixtures.php
  • tests/Concurrency/PackageMetadataTest.php
  • tests/Concurrency/SerializedClosureResultTest.php
  • tests/Database/DatabaseConsoleJsonTest.php
  • tests/Database/DatabaseEloquentJsonCastTest.php
  • tests/Database/DatabaseMariaDbQueryGrammarTest.php
  • tests/Database/DatabaseMySqlQueryGrammarTest.php
  • tests/Database/DatabasePostgresQueryGrammarTest.php
  • tests/Database/DatabaseQueryGrammarTest.php
  • tests/Database/DatabaseSQLiteQueryGrammarTest.php
  • tests/Filesystem/FilesystemAdapterTest.php
  • tests/Filesystem/FilesystemTest.php
  • tests/Foundation/FoundationFileBasedMaintenanceModeTest.php
  • tests/Foundation/FoundationPackageManifestTest.php
  • tests/Foundation/Http/CustomCastingTest.php
  • tests/Http/HttpClientTest.php
  • tests/Http/HttpJsonResponseTest.php
  • tests/Inertia/Testing/AssertableInertiaTest.php
  • tests/Integration/Database/EloquentModelEncryptedCastingTest.php
  • tests/Integration/Database/EloquentModelJsonCastingTest.php
  • tests/Session/SessionStoreTest.php
  • tests/Support/ComposerFileTest.php
  • tests/Support/JsonTest.php
  • tests/Support/SupportArrTest.php
  • tests/Support/SupportCollectionTest.php
  • tests/Support/SupportStrTest.php
  • tests/Support/SupportStringableTest.php
  • tests/Support/XmlTest.php
  • tests/Telescope/ExtractPropertiesTest.php
  • tests/Telescope/Storage/DatabaseEntriesRepositoryTest.php
  • tests/Telescope/Watchers/ClientRequestWatcherTest.php
  • tests/Telescope/Watchers/EventWatcherTest.php
  • tests/Telescope/Watchers/ModelWatcherTest.php
  • tests/Telescope/Watchers/RequestWatchersTest.php
  • tests/Testbench/Foundation/Fixtures/PackageManifest/build-manifest.php
  • tests/Testbench/Foundation/PackageManifestPackageTesterTest.php
  • tests/Testbench/Foundation/PackageManifestTest.php
  • tests/Testbench/Foundation/Process/ProcessResultTest.php
  • tests/Testbench/PackageMetadataTest.php
  • tests/Testing/PHPUnit/TestStateRegistrarsTest.php
  • tests/Testing/TestResponseTest.php
  • tests/Validation/ValidationPlanExecutorTest.php
  • tests/Validation/ValidationValidatorTest.php

Comment thread src/database/src/Eloquent/Casts/AsArrayObject.php
Comment thread src/foundation/src/Http/Traits/HasCasts.php
Comment thread src/telescope/src/Storage/DatabaseEntriesRepository.php
Comment thread tests/Testing/TestResponseTest.php
Document the framework-only trust boundary for serialized closure responses and validate that a transported exception class is an available Throwable before invoking its constructor. Preserve the remote message while retaining class-resolution and constructor failures as the previous exception for useful diagnostics.

Make the transport regression tests immune to PHPUnit assertion interception, prove that non-Throwable constructors are never called, and keep combined Testbench subprocess diagnostics separated by a newline.

Record the accepted Telescope occurrence-count race and why moving a non-locking count into the existing transaction would not serialize writers. The full formatter, static analysis, parallel suite, Testbench contract suite, dogfood checks, and targeted review tests are green.
@binaryfire

Copy link
Copy Markdown
Collaborator Author

@coderabbitai I reviewed all four inline findings and all four nitpicks against the current source, producers, consumers, and test lifecycle.

Changes pushed:

  • Documented the SerializedClosureResult trust boundary and the shared subprocess-output stream.
  • Validate transported exception classes as Throwable before constructor invocation. The remote message remains the outer error; local class-resolution or constructor failures remain available as the previous exception.
  • Added a constructor-side-effect probe so the pre-construction ordering is directly tested.
  • Removed broad-catch test patterns that could intercept PHPUnit assertion failures across every affected transport test.
  • Made subprocess diagnostics portable across stdout and stderr while keeping the streams separated.
  • Recorded the accepted Telescope occurrence-count race and why a plain count inside the current transaction would not serialize writers.

No change was made for the remaining suggestions:

  • Eloquent casts use Hypervel\Database\Eloquent\Casts\Json, whose Laravel-style default encoder returns string|false. Exact false is the correct shared failure signal for native and custom codecs.
  • The removed request asJson method was unused Hypervel-only surface, not a Laravel FormRequest API.
  • Exact Telescope occurrence serialization needs a lockable family owner or dedicated lock row. Moving a snapshot count into the transaction does not fix the race.
  • VarDumper is intentionally reset to null by the authoritative after-test cleanup.
  • A 511-container exception value cannot be emitted as a valid transport envelope: the command degrades it before emission, while a directly crafted over-depth document only repeats the generic decoder-depth contract.
  • Blanket docstring coverage conflicts with the repository rule to document purpose and non-obvious behavior rather than add comments for a metric.

There were no outside-of-diff findings. The full repository checks and focused regression tests are green.

@coderabbitai

coderabbitai Bot commented Aug 12, 2026

Copy link
Copy Markdown

Tip

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '=== PR review comments ==='
gh api repos/hypervel/components-backup/pulls/14/comments --paginate \
  --jq '.[] | {path, line, body, user: .user.login}' || true

echo '=== Changed files related to reported follow-up ==='
git diff --name-only HEAD^ HEAD | rg '^(src/concurrency/|src/telescope/|tests/Concurrency/|tests/Telescope/|docs/)'

echo '=== Relevant source outlines ==='
for f in \
  src/concurrency/src/SerializedClosureResult.php \
  src/concurrency/src/ProcessDriver.php \
  src/telescope/src/Storage/DatabaseEntriesRepository.php \
  tests/Concurrency/SerializedClosureResultTest.php \
  tests/Concurrency/ConcurrencyTest.php; do
  if [ -f "$f" ]; then
    echo "--- $f"
    ast-grep outline "$f" --items all || true
  fi
done

echo '=== Targeted reported-change references ==='
rg -n -C 5 \
  'Throwable|previous|constructor|stdout|stderr|occurrence|serialize|trust boundary|shared subprocess|catch \(Throwable|catch \(\\Throwable' \
  src/concurrency src/telescope tests/Concurrency tests/Telescope docs \
  || true

Length of output: 201597


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '=== Review comments: compact ==='
gh api repos/hypervel/components-backup/pulls/14/comments --paginate \
  --jq '.[] | "\(.id)\t\(.path):\(.line // 0)\t\(.user.login)\t\(.body | gsub("\n"; " "))"' \
  | head -200

echo '=== Files changed in current PR ==='
gh pr view 14 --repo hypervel/components-backup --json files \
  --jq '.files[].path' \
  | rg '^(src/(concurrency|telescope)/|tests/(Concurrency|Telescope)/|docs/)'

echo '=== SerializedClosureResult ==='
sed -n '1,240p' src/concurrency/src/SerializedClosureResult.php

echo '=== Concurrency command and process driver ==='
sed -n '1,260p' src/concurrency/src/Console/InvokeSerializedClosureCommand.php
sed -n '1,220p' src/concurrency/src/ProcessDriver.php

echo '=== Relevant concurrency test declarations and assertions ==='
rg -n -C 6 \
  'constructor|side.effect|Throwable|previous|stdout|stderr|catch \(|SerializedClosureResult' \
  tests/Concurrency -g '*.php'

echo '=== Telescope occurrence-count implementation and documentation ==='
rg -n -C 8 \
  'occurrence|count\(|transaction|race|serializ|lock' \
  src/telescope/src/Storage/DatabaseEntriesRepository.php \
  tests/Telescope/Storage/DatabaseEntriesRepositoryTest.php \
  docs -g '*.php' -g '*.md' \
  -g '!**/dist/**'

Length of output: 201776


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '=== Serialized closure decoder ==='
cat -n src/concurrency/src/SerializedClosureResult.php | sed -n '1,220p'

echo '=== Serialized closure command ==='
cat -n src/concurrency/src/Console/InvokeSerializedClosureCommand.php | sed -n '1,240p'

echo '=== Process driver ==='
cat -n src/concurrency/src/ProcessDriver.php | sed -n '1,200p'

echo '=== Decoder test methods and local fixtures ==='
rg -n -A 28 -B 5 \
  'function test.*(Throwable|Constructor|constructor|Output|Output.*Stream|Diagnostics)|class .*Exception|function __construct|catch \(' \
  tests/Concurrency/SerializedClosureResultTest.php \
  tests/Concurrency/ConcurrencyTest.php

echo '=== PR-specific Telescope decision record ==='
fd '2026-08-11-0932-json-correctness-and-package-metadata\.md$' docs/plans \
  -x sh -c 'echo "--- $1"; rg -n -C 6 "occurrence|race|serializ|lock" "$1"' sh {}

Length of output: 82808


@binaryfire Verified against the current source, tests, and PR-specific plan.

The concurrency decoder now documents its framework-only trust boundary. It validates a transported class as a Throwable before constructor invocation. It preserves the remote message and retains local resolution or construction failures as previous.

NonThrowableConstructorProbe verifies that an invalid transported class does not execute its constructor. The transport tests no longer use broad catches that can intercept PHPUnit assertion failures. ProcessDriver checks failed child processes before decoding stdout and keeps stderr diagnostics separate.

The Telescope decision record accurately describes the accepted occurrence-count race. A plain count inside the current transaction would not serialize concurrent writers. Exact serialization would require unsupported cross-engine locking or a dedicated lock owner.

The declined suggestions are consistent with the Eloquent codec contract, Laravel API scope, transport-depth boundary, authoritative VarDumper cleanup, and the repository documentation rule. No further change is required.


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

Timestamp: 2026-08-12T02:16:31.934Z
Learning: In `src/concurrency/src/SerializedClosureResult.php`, serialized-closure output is a framework-controlled subprocess protocol. The decoder can use unrestricted `unserialize()`, but callers must never pass external input to `SerializedClosureResult::decode()`. The subprocess stream can also contain application-task output, so the JSON response envelope must be validated before use.

Learnt from: binaryfire
URL: https://github.com/hypervel/components-backup/pull/14

Timestamp: 2026-08-12T02:16:31.934Z
Learning: For Hypervel Eloquent casts, `Hypervel\Database\Eloquent\Casts\Json` follows Laravel-style encoder semantics and returns `string|false`. Callers must use an exact `false` check to distinguish JSON encoding failure for both native and custom codecs.

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!

You are interacting with an AI system.

@binaryfire

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 12, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@binaryfire binaryfire closed this Aug 12, 2026
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