Add request-owned start time and correct runtime timing lifecycles - #478
Merged
Conversation
Use Symfony ArgvInput command discovery instead of assuming the command is always argv[1]. Preserve the existing public Application API while correctly recognizing commands preceded by valueless global options or inline option values. Add focused coverage for long and short option prefixes.
Create one Symfony ArgvInput before Testbench application bootstrap and use its first argument for serve and watch mode detection. Pass the same input instance into the console kernel so bootstrap classification and command execution cannot disagree.
Capture the precise worker-side request timestamp during Request initialization and normalize the standard server time fields when they are absent. Expose the stable instant as a lazily-created CarbonImmutable through startedAt(), regenerate the Request facade metadata, and cover construction, conversion, duplication, bridge precision, mutation stability, and kernel termination boundaries.
Pass the routed Request explicitly into both framework and Testbench health views and calculate render duration from its owned start instant. Remove the process-wide constant guard and add deterministic consecutive-request coverage so long-lived workers cannot report accumulated process uptime.
Derive Telescope request duration from Request::startedAt() instead of reading transport metadata directly. Remove the nullable fallback branch and freeze time in the watcher regression so the recorded millisecond duration is exact.
Start HTTP transactions from the Request-owned Carbon instant and preserve its microsecond precision when converting to Sentry epoch seconds. Delete the server-value and process-constant fallback chain, and prove later ServerBag mutation cannot alter the captured transaction start.
Document Request::startedAt(), normalized uppercase server metadata, kernel timing boundaries, and WebSocket handshake semantics. Replace the collection timeout example built from process startup with an invocation-local now() deadline and remove its obsolete import.
Add the established boolean or selected-key context-copy contract to Waiter and the global wait helper while preserving a fresh child context by default. Move synthetic HTTP testing onto the base Waiter with explicit full-context copying, retain replication-failure coverage, delete the redundant Foundation wrapper, and document the public behavior.
Keep pause, interruption, and maintenance control in the scheduler while running user filters and each task invocation in a finite child carrying only replicated Log Context. Share foreground and bounded-background dispatch across initial and repeated runs, preserve foreground ordering, and make claimed single-server work count as handled. Guard never-checked repeat events at their public predicate and advance paused repeats at their configured cadence, with regressions for context isolation, defers, background repeats, pause behavior, and user-visible output.
Use BeforeHandle command names instead of positional process arguments, correct the default ignored command set, and keep the long-lived schedule daemon outside recording. Start recording inside approved scheduled-task coroutines at the storage-opportunity boundary so every enabled watcher shares the real task lifecycle. Cover configured ignores, daemon silence, package discovery, and task-local recording.
Boot-register scheduled-task terminal listeners without argv gates and let Telescope own deferred storage at each finite coroutine boundary. Remove explicit repository and store ownership, suppress duplicate Finished-to-Failed terminal delivery for one task, and verify success, failure, configured ignores, distinct batches, output, and persistence before the scheduler parent exits.
Read the task exit code published before ScheduledTaskFinished to assign success or internal-error status from the finalized outcome. Finish and flush each scheduled transaction once across successful, non-zero Finished-to-Failed, and throw-before-Finished paths, with integration coverage for every terminal sequence.
Delete the published ignore_commands option because no current Hypervel or Sentry code consumes it. Remove the matching provider filter entry rather than preserving dead configuration or adding a command-tracing mechanism solely to justify the stale surface.
Capture the Laravel, Swoole, Hypervel, Telescope, Sentry, console, and scheduler research behind the request-owned timing design. Document the final ownership boundaries, anti-overengineering constraints, implementation map, regression plan, validation cadence, and completion criteria so future maintenance can distinguish deliberate runtime adaptations from missing Laravel behavior.
Bring the View correctness and lifecycle parity work from PR #477 into the request start-time branch.\n\nThe branches modify no common files, and the merged behavior has been checked across request timing, health rendering, View compilation and cleanup, Testbench lifecycle cleanup, Waiter context handling, and synthetic request synchronization. No request-timing or scheduler design changes are required.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (12)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughThe change replaces process-global request timing with request-owned timestamps, adds coroutine context-copying controls, updates scheduler execution and Telescope recording, improves CLI command resolution, and updates Sentry tracing, health rendering, documentation, and regression tests. ChangesRuntime lifecycle changes
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 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 |
Greptile SummaryThe PR introduces request-owned start timestamps and corrects command, scheduler, Telescope, and Sentry lifecycle timing in long-lived workers.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| src/http/src/Request.php | Adds stable request-owned start-time capture, server-time normalization, and lazy immutable timestamp access. |
| src/console/src/Application.php | Adds Symfony-definition-backed command resolution for option-prefixed console invocations. |
| src/console/src/Commands/ScheduleRunCommand.php | Moves user filters and task execution into finite task-owned coroutines while preserving scheduler-owned control checks. |
| src/coroutine/src/Waiter.php | Extends waited coroutines with optional full or selective context propagation. |
| src/telescope/src/ListensForStorageOpportunities.php | Uses resolved command identity and starts Telescope recording within scheduled-task coroutine lifecycles. |
| src/telescope/src/Watchers/ScheduleWatcher.php | Registers schedule listeners at boot, relies on coroutine-deferred storage, and suppresses duplicate terminal records. |
| src/sentry/src/Features/ConsoleSchedulingFeature.php | Finalizes scheduled transactions once using the task's published exit code. |
| src/testbench/hypervel/artisan | Reuses one option-aware ArgvInput and now consistently imports referenced classes, resolving the previous style finding. |
Reviews (2): Last reviewed commit: "docs: update request timing implementati..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
src/telescope/src/ListensForStorageOpportunities.php (1)
94-111: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd
voidreturn types to the new listeners.Both closures complete without a value. Declare
: voidon both closures.As per coding guidelines, “use native types for parameters, return values, and properties wherever permitted.”
Proposed fix
- $events->listen(BeforeHandleCommand::class, function (BeforeHandleCommand $event) { + $events->listen(BeforeHandleCommand::class, function (BeforeHandleCommand $event): void { // The long-lived scheduler records only inside each finite task coroutine. if ($event->command->getName() === 'schedule:run') { return; } @@ - $events->listen(ScheduledTaskStarting::class, function () { + $events->listen(ScheduledTaskStarting::class, function (): void { if (static::shouldListen() && static::commandIsApproved('schedule:run')) { static::startRecording(); }🤖 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/ListensForStorageOpportunities.php` around lines 94 - 111, Add the native : void return type to both listener closures registered in the BeforeHandleCommand and ScheduledTaskStarting listeners, without changing their existing control flow or behavior.Source: Coding guidelines
src/support/src/Facades/Request.php (1)
16-16: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winImport
CarbonImmutablefor this facade annotation.Line 16 uses a fully qualified class name. Add a
use Hypervel\Support\CarbonImmutable;statement and useCarbonImmutablein the annotation. As per coding guidelines, import classes withusestatements instead of using fully qualified class names.🤖 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/support/src/Facades/Request.php` at line 16, Add a use statement for Hypervel\Support\CarbonImmutable in the facade file, then update the startedAt() annotation to reference CarbonImmutable without the fully qualified namespace.Source: Coding guidelines
tests/Foundation/ApplicationRunningInConsoleTest.php (1)
230-246: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd regression cases for separate option values.
These tests cover
--env=productionand the-vflag. They do not cover--env productionor-e production. The current unboundArgvInputpath returns the option value as the command in those cases. (raw.githubusercontent.com)Add assertions for both separate-value forms.
🤖 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/Foundation/ApplicationRunningInConsoleTest.php` around lines 230 - 246, Extend the regression tests in testRunningConsoleCommandMatchesCommandAfterLongOption and testRunningConsoleCommandMatchesCommandAfterShortOption to use separate option values: --env followed by production and -e followed by production. Assert the actual command is detected and neither option nor its separate value is reported as the command.
🤖 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.
Nitpick comments:
In `@src/support/src/Facades/Request.php`:
- Line 16: Add a use statement for Hypervel\Support\CarbonImmutable in the
facade file, then update the startedAt() annotation to reference CarbonImmutable
without the fully qualified namespace.
In `@src/telescope/src/ListensForStorageOpportunities.php`:
- Around line 94-111: Add the native : void return type to both listener
closures registered in the BeforeHandleCommand and ScheduledTaskStarting
listeners, without changing their existing control flow or behavior.
In `@tests/Foundation/ApplicationRunningInConsoleTest.php`:
- Around line 230-246: Extend the regression tests in
testRunningConsoleCommandMatchesCommandAfterLongOption and
testRunningConsoleCommandMatchesCommandAfterShortOption to use separate option
values: --env followed by production and -e followed by production. Assert the
actual command is detected and neither option nor its separate value is reported
as the command.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 9b8460bc-1ef0-4922-bfa7-3191c7ffeee9
📒 Files selected for processing (45)
docs/plans/2026-08-06-1121-request-start-time-and-runtime-timing.mdsrc/boost/docs/collections.mdsrc/boost/docs/coroutines.mdsrc/boost/docs/requests.mdsrc/console/src/Commands/ScheduleRunCommand.phpsrc/console/src/Scheduling/Event.phpsrc/coroutine/src/Waiter.phpsrc/coroutine/src/functions.phpsrc/foundation/src/Application.phpsrc/foundation/src/Configuration/ApplicationBuilder.phpsrc/foundation/src/Testing/Concerns/MakesHttpRequests.phpsrc/foundation/src/Testing/Coroutine/Waiter.phpsrc/foundation/src/resources/health-up.blade.phpsrc/http/src/Request.phpsrc/sentry/config/sentry.phpsrc/sentry/src/Features/ConsoleSchedulingFeature.phpsrc/sentry/src/SentryServiceProvider.phpsrc/sentry/src/Tracing/Middleware.phpsrc/support/src/Facades/Request.phpsrc/telescope/src/ListensForStorageOpportunities.phpsrc/telescope/src/Telescope.phpsrc/telescope/src/Watchers/CommandWatcher.phpsrc/telescope/src/Watchers/RequestWatcher.phpsrc/telescope/src/Watchers/ScheduleWatcher.phpsrc/testbench/hypervel/artisansrc/testbench/src/Workbench/Workbench.phptests/Console/Scheduling/EventTest.phptests/Console/Scheduling/ScheduleRunCommandTest.phptests/Console/Scheduling/ScheduleRunContextPropagationTest.phptests/Coroutine/WaiterTest.phptests/Foundation/ApplicationRunningInConsoleTest.phptests/Foundation/Http/KernelTest.phptests/Foundation/Testing/Coroutine/WaiterTest.phptests/Foundation/Testing/RequestContextSynchronizerTest.phptests/Http/HttpRequestTest.phptests/HttpServer/RequestBridgeTest.phptests/Integration/Console/Scheduling/SubMinuteSchedulingTest.phptests/Integration/Foundation/Support/Providers/RouteServiceProviderHealthTest.phptests/Sentry/Features/ConsoleSchedulingIntegrationTest.phptests/Sentry/Tracing/MiddlewareTest.phptests/Telescope/Telescope/TelescopeTest.phptests/Telescope/Watchers/CommandWatcherTest.phptests/Telescope/Watchers/RequestWatchersTest.phptests/Telescope/Watchers/ScheduleWatcherTest.phptests/Testbench/Workbench/DiscoversTest.php
💤 Files with no reviewable changes (4)
- src/foundation/src/Testing/Coroutine/Waiter.php
- src/sentry/config/sentry.php
- src/sentry/src/SentryServiceProvider.php
- tests/Foundation/Testing/Coroutine/WaiterTest.php
Pre-bootstrap command checks used an unbound ArgvInput, so a separated optional --env value could be mistaken for the command name. Add one shared resolver that binds Symfony's authoritative global definition plus Hypervel's environment option using Symfony's own catch-and-rebind flow. Use it for the public application classifier while preserving the existing protected environment-option extension point. Cover attached and separated global option values, command-specific options rejected during preliminary binding, and execution after the real console kernel rebinds the same input.
Both Testbench CLI paths classified server commands before Symfony had bound the global input definition. A separated --env value could therefore leave APP_RUNNING_IN_CONSOLE enabled for a serve invocation. Route the skeleton artisan and bin/testbench Commander through the shared console resolver, keep the same ArgvInput for kernel execution, and normalize the entrypoint imports and constructor style. Declare Testbench's direct hypervel/console dependency and add a focused regression for the separated environment option form.
Declare the command and scheduled-task storage opportunity closures as void. This makes their callback contracts explicit and aligns the listeners with the repository's full-typing convention without changing recording behavior.
Add concise Laravel-style method documentation to the three scheduled-task lifecycle handlers. The comments state each handler's tracing responsibility while keeping the upstream-derived feature implementation free of a broad documentation-only rewrite.
Replace the earlier bounded command-name parsing proposal with the final shared Symfony-definition-backed resolver design. Record all three shipped CLI consumers, the generic kernel boundary, signal setup behavior, direct Testbench dependency, regression coverage, stale-code checks, and final review criteria so the plan matches the implemented code without preserving rejected approaches.
Collaborator
Author
|
@greptile-apps @coderabbitai I reviewed every finding and pushed the accepted changes. Accepted changes:
The separate Two suggestions were intentionally not applied:
The follow-up audit also added Testbench direct dependency on |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== changed files ==='
git diff --name-status HEAD^ HEAD
echo
echo '=== relevant diff ==='
git diff --unified=60 HEAD^ HEAD -- \
src/foundation/src/Application.php \
src/testbench/hypervel/artisan \
src/testbench/composer.json \
src/telescope/src/ListensForStorageOpportunities.php \
src/sentry/src/Features/ConsoleSchedulingFeature.php \
src/support/src/Facades/Request.php \
tests/Foundation/ApplicationRunningInConsoleTest.php \
tests/Testbench/Workbench/DiscoversTest.php
echo
echo '=== source outlines ==='
fd -a -t f 'Application.php|artisan|ConsoleSchedulingFeature.php|ListensForStorageOpportunities.php|Request.php|composer.json' \
src/foundation/src src/testbench src/sentry/src src/telescope/src src/support/src 2>/dev/null \
| while IFS= read -r file; do
echo "--- $file"
ast-grep outline "$file" --items all 2>/dev/null || true
done
echo
echo '=== console option resolver and CLI entrypoint usages ==='
rg -n -C 5 --glob '*.php' --glob 'artisan' \
'ArgvInput|InputDefinition|runningConsoleCommand\s*\(|--env|resolve.*Command|bind.*Definition' \
src/foundation src/testbench testsLength of output: 117346 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== PR commit and file metadata ==='
gh api repos/hypervel/components/pulls/478 --jq '{base: .base.sha, head: .head.sha, commits: .commits, changed_files: .changed_files}'
gh api repos/hypervel/components/pulls/478/files --paginate \
--jq '.[] | [.filename, .status] | `@tsv`' | sort
echo
echo '=== command resolver implementation ==='
resolver_file="$(fd -a -t f 'Application.php' src/console/src | head -n 1)"
echo "--- $resolver_file"
rg -n -A100 -B15 'resolveCommandName\s*\(' "$resolver_file"
echo
echo '=== shipped CLI entrypoints and resolver calls ==='
rg -n -C 8 --glob 'artisan' --glob '*.php' \
'resolveCommandName\s*\(|new ArgvInput|APP_RUNNING_IN_CONSOLE' \
src/testbench src/foundation tests/Testbench
echo
echo '=== Testbench console dependency ==='
cat src/testbench/composer.json
printf '\n--- lock references ---\n'
rg -n -C 2 '"hypervel/console"' composer.json composer.lock src/testbench/composer.json 2>/dev/null || true
echo
echo '=== Telescope closure signatures ==='
rg -n -A18 -B8 --glob '*.php' 'function\s*\([^)]*\)\s*(:\s*void)?\s*\{' \
src/telescope/src/ListensForStorageOpportunities.php
echo
echo '=== Sentry scheduling handler docs ==='
sed -n '/function handleScheduledTaskStarting/,/function startCheckIn/p' \
src/sentry/src/Features/ConsoleSchedulingFeature.php
echo
echo '=== generated Request facade annotations ==='
sed -n '1,215p' src/support/src/Facades/Request.phpLength of output: 48192 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== committed artisan entrypoints ==='
fd -a -t f -p '/artisan$' . -E vendor -E .git | sort
echo
echo '=== resolver calls in committed entrypoints ==='
fd -a -t f -p '/artisan$' . -E vendor -E .git -x sh -c '
echo "--- $1"
rg -n -C 5 "resolveCommandName\s*\(|new ArgvInput|APP_RUNNING_IN_CONSOLE" "$1" || true
' sh {}Length of output: 1251
The shared The Testbench entrypoint imports its dependencies and uses the shared resolver. Both relevant Telescope listener closures declare Keeping the generated Request facade annotations fully qualified is consistent with the file. Skipping repository-wide docstrings is appropriate because those changes are outside this PR’s behavior. I did not run 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. |
|
Everything checks out. The implementation is clean across all touched files. A few specific observations on the landed code:
Sentry double-flush guard — Having verified Telescope One thing to confirm: |
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.
Why
HYPERVEL_STARTis captured when an Artisan process starts. That works for a short-lived PHP process, but it is not a request start time in a long-lived Swoole worker. Health responses can report worker uptime, and consumers either need to know aboutREQUEST_TIME_FLOATor maintain their own fallback chain.Tracing those consumers also exposed the same process-level assumption in console and scheduler handling. Telescope classified commands from a fixed argv position, recorded the long-running
schedule:rundaemon instead of each task, and deferred task storage until a coroutine that may never exit. Scheduled Sentry transactions could also be finalized as successful before a non-zero exit code was published.What changed
This PR makes request start time part of the request itself:
Request::initialize()captures the preciseREQUEST_TIME_FLOATvalue supplied by Swoole or Symfony. Directly constructed requests use onemicrotime(true)fallback. MissingREQUEST_TIME_FLOATandREQUEST_TIMEserver values are populated at that boundary, while supplied server values remain unchanged.The stored timestamp is stable for the request lifetime.
startedAt()returns aCarbonImmutableonly when called, so ordinary requests pay for one float and do not allocate a Carbon instance unless a consumer needs it.The health view now receives the routed request explicitly and always renders duration from
startedAt(). Telescope request duration and Sentry HTTP transaction start time use the same API instead of reading transport details or a process constant.The existing server bag remains the complete metadata API. Swoole keys continue to use the PHP/Symfony uppercase form, so developers can retrieve the raw value with
$request->server("REQUEST_TIME_FLOAT")or inspect all server metadata with$request->server().Console and scheduler lifecycle
The request timing audit found several places where long-lived command state was treated as one short process:
ArgvInput::getFirstArgument(), including the publicrunningConsoleCommand()API and the Testbench Artisan entrypoint. This handles common global-option prefixes without adding a custom argument parser.BeforeHandle. Theschedule:rundaemon remains unrecorded, while each approved scheduled task starts recording in its own finite coroutine.Waiter::wait()and thewait()helper gain the same optional all-key or selected-key context copying supported by the other coroutine helpers. The default remains a fresh context.nullintoabs().sentry.ignore_commandsconfiguration is removed rather than retained as a non-functional option.These changes do not replace the existing HTTP or console kernel lifecycle clocks. Those clocks still describe the narrower
handle()andterminate()lifecycle, whileRequest::startedAt()belongs to the request and scheduled-task events belong to each task.Compatibility and performance
The public API remains Laravel-shaped and additive. Existing request server access, kernel timing APIs, foreground ordering, and bounded background scheduling behavior are preserved.
The HTTP hot path adds one float property assignment per request. Carbon conversion is lazy. The additional coroutine boundary applies only to due scheduled task invocations, not HTTP request handling.
Documentation and tests
The request and coroutine documentation now covers the new APIs and timing boundaries. The health, Request bridge, Telescope, Sentry, scheduler, console classification, and Testbench behavior all have focused regression coverage, including consecutive requests in one worker and task-local recording/storage.
The complete formatter, static analysis, parallel component suite, Testbench suite, and package dogfood checks pass.
Summary by CodeRabbit
Request::startedAt()for accurate health, tracing, and duration reporting.