Isolate logging state and normalize enum identifiers#438
Merged
Conversation
Document the framework-wide rule for converting enum-backed identifiers only at their owning string boundaries. Record compatibility, performance, implementation, regression, and completeness requirements for manager, cache, queue, database, filesystem, authorization, and related package surfaces.
Use one callback-rebinding contract across framework managers so static closures, first-class callables, and ordinary closures retain Laravel-compatible extension behavior. Normalize enum-backed driver names at manager-owned string boundaries, preserve explicit zero identifiers, align contracts, facades, and fakes, and cover inherited and package-specific manager behavior.
Replace worker-shared logger context and Monolog recursion bookkeeping with one coroutine-owned logger-family state. Preserve visible context across replicated execution while resetting transient recursion depth, prevent object-ID reuse from aliasing destroyed loggers, and retain Monolog loop-detection behavior without cross-request interference.
Provide vendor-compatible fingers-crossed and UID components whose mutable request state follows coroutine ownership instead of worker lifetime. Replicated contexts begin with safe buffer state while retaining request identity, and exact-class normalization leaves user-defined Monolog subclasses under application ownership.
Replace process-global PHP error-handler windows around yielding stream operations with checked native boundaries. Preserve Monolog retry, locking, permission, rotation, retention, and diagnostic behavior while preventing one coroutine from intercepting warnings raised by another.
Port structured exception context into the Log formatter while keeping in-flight reporting state coroutine-local. Preserve nested reporting ownership across copied contexts, restore prior state in finally blocks, and avoid recursive context expansion when the same exception is already being reported.
Build dynamic channels without retaining a synthetic named logger, apply taps from the resolved configuration, and confine manager-wide context mutation to cached named channels. Normalize only exact built-in mutable Monolog components, restore named-stack behavior, preserve zero-named channels, and document worker-cached custom component ownership.
Serialize background and deferred jobs when execution is scheduled so mutable job and context state cannot drift before a later coroutine or timer runs. Reuse the synchronous payload executor, preserve after-commit timing, surface serialization failures at the owning boundary, and clear stale log context when an incoming payload carries none.
Remove the package-local action-level wrapping workaround and let the Log package own safe handler normalization. Preserve Sentry channel behavior while ensuring buffering state follows the same coroutine-local contract as every other framework-built Monolog graph.
Convert enum-backed keys and tags only where repositories, stores, events, and Redis commands require strings. Preserve numeric-string multi-read keys, align event metadata with concrete cache APIs, keep failover raw reads truthful, retain direct string interoperability, and avoid rebuilding value-domain arrays or adding work around cache I/O.
Normalize enum-backed connection and queue names before they enter typed carrier state, and preserve explicit zero identifiers through batches, chains, queued closures, broadcasts, and mailables. Keep value-oriented enum groups intact, retain established empty-string behavior, and cover both immediate configuration and later serialized execution.
Carry normalized connection and queue names through manager lookup, routing, payload creation, storage, reservation, returned jobs, commands, and Telescope observation. Replace truthiness with each boundary established null or empty sentinel so queue zero remains addressable without changing normal default selection.
Keep explicit zero-valued connection and queue names through Horizon command parsing, supervisor options, queue clearing, and wait-time lookup. Match Queue behavior at each boundary while retaining the existing fallback for genuinely absent or empty command input.
Convert enum-backed connection and table identifiers at database-owned string boundaries and preserve zero-valued names through managers, pools, models, factories, migrations, schema commands, testing helpers, and pipeline transactions. Keep query values and bindings untouched while correcting index and command defaults that previously discarded valid zero identifiers.
Normalize queue, connection, cache-store, and timezone identifiers when schedules are configured so typed state remains valid and zero-backed enums are not replaced by defaults. Preserve DateTimeZone objects and existing scheduling behavior for ordinary strings and absent values.
Normalize enum abilities before authorization inference and preserve guard name zero through discovery, middleware parsing, role scopes, mismatch checks, Passport compatibility, and permission diagnostics. Keep application-level guard semantics and existing empty-name fallback behavior unchanged.
Convert enum cookie names to their string representation for request lookup, creation, queued lookup, and removal. Preserve downstream Symfony validation exactly, including equivalent rejection of invalid direct-string and enum-backed names, rather than bypassing domain-owned constraints.
Treat presence user ID zero as a real identifier during subscription, unsubscription, client rebroadcast, and webhook payload construction. Normalize stored IDs at the owning boundary and reserve null, rather than truthiness, for complete channel data lookup and missing shared state.
Normalize once keys, token cache stores, search connections, and Telescope tags where their owning services require strings. Preserve ordinary values unchanged and ensure integer-backed enum identifiers remain stable through persistence, monitoring, and command execution.
Use exact absence checks when converting routes and resolving translation namespaces so explicit string zero remains addressable. Preserve the established null and empty-string fallback behavior while preventing valid names from being silently replaced.
Preserve explicit zero-valued model and database names through policy generation and Testbench SQLite commands. Keep existing defaults for absent input while ensuring command arguments select the requested target instead of silently falling back.
Mark the Log package audit complete and record the final logging-state, callback, queue-capture, enum-identifier, zero-sentinel, compatibility, performance, and regression decisions. Keep the reusable audit checklist and durable findings ledger aligned with the implemented framework boundaries.
|
Important Review skippedToo many files! This PR contains 231 files, which is 181 over the limit of 50. To get a review, narrow the scope: Upgrade to a paid plan to raise the limit. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (231)
You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 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 |
Greptile SummaryThis large PR addresses two framework-wide concerns for long-lived Swoole workers: it isolates mutable logging state (context, recursion depth, fingers-crossed buffers, UID processors) into coroutine-local ownership while keeping the immutable logger graphs cached at worker scope, and it normalizes enum identifiers to strings at each receiver boundary that requires a string key rather than converting them globally.
Confidence Score: 5/5Safe to merge. The changes address well-understood state-isolation problems specific to long-lived Swoole workers and do not alter any public API contract. The coroutine state isolation, stream handler rewrites, enum identifier normalization, and queue payload serialization changes are all well-scoped and internally consistent. The recursion guard design is correct, the coroutine-ID check in isReporting() correctly prevents false positives from replicated child contexts, and the stack() channel-name fix is a genuine bug fix. No correctness defects were found in the changed code paths. No files require special attention. The most complex new code is in PerformsSafeStreamOperations and FingersCrossedHandler; both follow the established CoroutineContext/ReplicableContext pattern consistently. Important Files Changed
Reviews (2): Last reviewed commit: "Align Reverb presence integration assert..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Keep the existing coroutine-local logging depth active while MessageLogged events are dispatched so a same-channel listener that logs recursively reaches the established loop-abort threshold instead of exhausting the Swoole worker. Add a bounded regression that demonstrates the listener path emits the warning and terminates without introducing cross-channel coordination. Document why active exception reports are not enriched a second time inside JsonFormatter, and record the deliberate restraint against generic cross-channel loop detection. On-demand logger construction remains uncached and unchanged.
Decode the nested member_removed payload and assert the normalized string presence identifier directly. Presence identifiers are intentionally normalized at the protocol boundary so zero remains valid; the integration test still expected the previous numeric JSON representation even though the focused Reverb regressions already covered the string contract.
Collaborator
Author
|
@greptileai Thanks for the careful review. The follow-up is complete.
I also re-audited the logging design specifically for unnecessary complexity. The larger adapters exist only at exact Monolog boundaries whose process-global error handlers and worker-shared mutable state are unsafe under Swoole. No generic handler framework, mutex, pool, per-request logger rebuild, cross-channel registry, or speculative compatibility layer was added. Focused logging and Reverb tests, formatting, both PHPStan configurations, diff checks, and the complete parallel suite are green. |
|
The follow-up is thorough and the explanations hold up on re-examination. On the The rest of the changes are clean:
No further changes needed from my side. The design rationale is sound and the regression coverage matches the stated risk surface. |
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.
Summary
This change corrects two framework-wide ownership boundaries that become observable in long-lived Swoole workers.
First, it keeps logger graphs cached at worker scope while moving their mutable request state to coroutine ownership. Logger context, recursion depth, fingers-crossed buffers, request UIDs, and in-flight exception reporting can no longer bleed between concurrent requests. Built-in stream handlers also stop using process-global PHP error handlers around hooked filesystem operations that may yield.
Second, it makes enum-capable identifier APIs truthful end to end. Enum values are converted only where a package owns a string identifier boundary, and an explicit identifier of
0is no longer mistaken for an absent value later in a manager, cache, queue, database, command, authorization, or realtime path.The result preserves Laravel-facing call shapes and existing worker-level caches while removing request-state races, delayed type failures, wrong-default selection, and partial identifier normalization.
For more details, see docs/plans/2026-07-12-framework-coroutine-state-lifecycle-audit-ledger.md.
Logging ownership
Hypervel intentionally retains configured Monolog graphs across requests. Rebuilding or cloning those graphs per request would discard one of the useful properties of the worker model. The mutable parts of those graphs, however, cannot share the same lifetime.
This change:
Custom handlers, processors, and complete custom drivers remain application-owned. Exact built-in Monolog classes are normalized; subclasses are not silently rewritten.
Manager and identifier contracts
Framework managers now share one callback-rebinding contract. Ordinary closures retain manager binding, while static closures and first-class callables remain valid instead of failing at registration time.
Enum identifiers are normalized at the receiver that requires a string rather than by changing the general
enum_value()helper. This distinction matters because enum values used as model data, query bindings, serialized values, groups, and other value-domain inputs must retain their native integer or string representation.The corrected identifier flow covers:
Each changed defaulting expression preserves the owning Laravel behavior: boundaries that treat null and the empty string as default continue to do so, while null-only boundaries remain null-only. The correction is specifically that the string identifier
0survives delegation.Compatibility
No Laravel-facing method is removed or renamed, and no configuration key changes. Existing string and null call sites retain their behavior. Enum-capable APIs either catch up to the current Laravel contract or begin honoring unions that Hypervel already advertised.
The Reverb and translation corrections follow the same exact-absence rule without introducing a generic identifier abstraction. Cookie normalization still delegates validity to Symfony, so an enum-backed name has the same acceptance or rejection as its direct string equivalent.
Performance
The design keeps worker-cached managers, logger graphs, cache repositories, and pooled resources intact.
Ordinary string manager lookups add at most one predictable enum check. Existing enum-aware paths complete conversion they already attempted. Logging state lookups occur only when a record is handled, and feature-specific lookups apply only to channels configured with the corresponding buffering or UID component. There are no request-wide locks, registries, logger rebuilds, pools, or coordination layers.
Validation
Regression coverage exercises concurrent logger state, recursive logging, copied coroutine context, buffered handlers, UID replication, native stream failures, dynamic channel construction, queued payload timing, callback forms, enum representations, explicit zero identifiers, and direct string interoperability across the affected packages.
The formatter and static-analysis gates are clean, and the implementation has been exercised through the complete framework test workflow as well as focused package suites.