Skip to content

Harden Horizon cluster, process, and publication lifecycles - #459

Merged
binaryfire merged 24 commits into
0.4from
audit/horizon-cluster-process-parity
Jul 29, 2026
Merged

Harden Horizon cluster, process, and publication lifecycles#459
binaryfire merged 24 commits into
0.4from
audit/horizon-cluster-process-parity

Conversation

@binaryfire

@binaryfire binaryfire commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator

Summary

This updates Horizon for Hypervel's pooled phpredis runtime while bringing its public surface back in line with current Laravel Horizon.

The main changes are:

  • support Horizon metadata operations on Redis Cluster by using transactions for Cluster connections and hash-tag-aware queue keys;
  • preserve queue topology and per-job delays during bulk dispatch;
  • propagate terminal process exit codes and prevent queued commands or signals from restarting a terminated monitor;
  • make Horizon locks atomic and owner-safe;
  • correct metrics ranking, clearing, snapshot locking, and timer cleanup;
  • fix MySQL and MariaDB batch search escaping;
  • consume transient queue and listener context once, including failure paths;
  • add the current CSP nonce and development-command APIs;
  • publish Horizon, Telescope, and Fortify installer rewrites atomically;
  • update Horizon configuration, package metadata, documentation, Boost guidance, and frontend dependencies; and
  • remove duplicate test cleanup while making process-test synchronization deterministic.

Why

Several Horizon paths still assumed a request-per-process runtime or a standalone Redis connection.

Direct Redis pipelines are not valid on phpredis Cluster connections. Queue bulk dispatch also lost Cluster topology and ignored job delay attributes. Horizon's process monitors discarded exit status and could continue processing commands after termination. Its locks used a check-then-set sequence and callback cleanup could delete a successor's lock. Metrics cleanup borrowed Redis repeatedly, ranking queried the wrong range, and stopwatch entries could survive failed updates.

There were also smaller correctness gaps: delayed payloads omitted delay metadata, transient coroutine context could leak into later work, MySQL batch search used invalid escaping, installer rewrites could publish partial PHP, and the split package did not declare all of its direct runtime requirements.

This change fixes those issues at their existing owners. It does not add retries, registries, background coordinators, or new hot-path synchronization.

Implementation

Redis and queue behavior

Horizon repositories now choose a normal pipeline for standalone Redis and a transaction for Redis Cluster. Horizon's Cluster prefix keeps those transaction keys in one slot.

Queue bulk dispatch preserves the active connection topology and resolves each job's existing delay metadata. Horizon's ready count now reads the same physical queue key used by the queue driver.

Redis multi-exec return annotations now distinguish callback results from the native client returned when no callback is supplied. Runtime behavior is unchanged.

Process lifecycle

Master and supervisor monitors retain their terminal status, return it through their commands, and stop processing commands, signals, persistence, and monitor work after termination. The existing no-exit() Hypervel process model remains intact.

Worker command options now include the inherited empty-queue timeout, and the command accepts the nullable result already allowed by the Queue worker contract.

Locks, metrics, and context

Lock acquisition is a single atomic Redis command. Callback-style locks use the existing owner-token release behavior so an expired owner cannot delete a successor.

Metrics use the correct maximum range, clear through one raw Redis lease, honor the configured snapshot lock duration, and release stopwatch entries even when metric updates fail.

Queue payload and listener event context is consumed once and cleared before user callbacks can fail. Delayed jobs now pass the resolved queue name and delay through the same payload hooks as immediate jobs.

Publication and package surface

Horizon, Telescope, and Fortify installer rewrites now read the current file and mode through checked boundaries and publish completed content with an atomic replacement. Failures retain package-specific diagnostics and stop later publication where required.

The Horizon package now includes current CSP nonce and development-command APIs, complete split-package requirements, updated public documentation and Boost guidance, and synchronized frontend dependency metadata.

Performance

The request path gains no network calls, locks, retries, or retained worker state. Dashboard asset rendering adds one coroutine-context read per asset method.

Standalone Redis batching is unchanged. Cluster batching replaces an invalid operation with its supported transaction form. Lock acquisition drops from two Redis round trips to one, and metrics clearing drops from repeated connection checkouts to one lease. Process termination checks run only in the existing monitor loop.

Validation

Validated with:

  • the full composer fix gate;
  • focused Horizon, Queue, Redis, Telescope, Fortify, and Reverb tests;
  • live Redis Horizon integration coverage;
  • MySQL batch-search integration coverage;
  • split-package metadata and frontend lock validation; and
  • stale-reference and whitespace checks.

Summary by CodeRabbit

  • New Features
    • Added Horizon CSP nonce support, proxy path configuration, snapshot-lock behavior, and additional worker options.
    • Enabled attribute-based job delays and improved Redis Cluster-safe batching/pipeline handling.
    • Horizon commands now return meaningful exit codes and support stopping after a configurable idle period.
  • Bug Fixes
    • Fixed batch search escaping for literal wildcard characters.
    • Improved Redis/Horizon lock atomicity and metrics clearing, plus more reliable shutdown/termination behavior.
  • Documentation
    • Expanded Horizon docs (installation, CSP nonce, worker options, metrics, notifications, tagging, and Laravel differences).

Keep the framework canonical string keys for Redis and database services while narrowing each resolved value to its real runtime contract. This preserves container conventions and lets static analysis follow connector and lifecycle calls without changing bindings or resolution behavior.

Document the same rule in AGENTS.md so future PHPStan fixes use local truthful narrowing instead of changing service keys or introducing global ignores.
Make pipeline and transaction return documentation conditional on whether a callback is supplied. No-callback calls return the native Redis client used to build the batch, while callback calls return the executed result array or false.

Add focused coverage for pipeline and transaction callback and no-callback forms, including native false results, without changing runtime dispatch.
Use a Redis transaction for Cluster bulk publication and retain the existing pipeline-plus-transaction shape for standalone Redis. This avoids unsupported Cluster pipelines while preserving batched publication semantics.

Resolve Delay attributes through the existing cached Queue metadata reader for both Redis and database bulk jobs while retaining public delay properties. Add regressions for both topologies and both delay forms.
Route Horizon repository batches through one small topology-aware trait. Standalone connections keep pipelines, while Cluster connections use transactions against the hash-tagged Horizon prefix so every batch remains in one Redis slot.

Apply the boundary to command, job, master, process, supervisor, and tag repositories. Add representative topology tests rather than duplicating the same branch assertion at every call site.
Consume and forget the last-pushed job before preparing raw payloads, and clear listener-event context in a finally block. Later work in the same coroutine can no longer inherit stale telemetry after success or failure.

Use the physical hash-tag-aware queue key for ready counts and include the resolved queue and delay in delayed payload creation. Add coverage for direct raw pushes, preparation failures, delayed metadata, hook queue names, and listener tag failures.
Read the actual maximum ranking entry, keep native snapshot field types truthful, and release stopwatch entries from the listener that consumes them even when either metric write fails.

Clear all metrics through one raw Redis lease and the existing prefix-aware scan primitive. Make snapshot lock duration configuration explicit while retaining the nested-array fallback required by shallow configuration replacement.
Acquire ordinary Horizon locks with one atomic SET EX NX operation and reject non-positive lifetimes before connecting. Callback locks delegate to the existing token-owned RedisLock implementation so an expired owner cannot delete its successor.

Preserve the public force-release behavior and primary callback exceptions. Add focused command-shape, invalid-lifetime, failure-precedence, replacement-owner, and force-release coverage.
Escape batch-search patterns with an explicit portable character so literal percent and underscore searches work on MySQL, MariaDB, and SQLite. Preserve case-insensitive matching and exact zero cursor behavior.

Make failed-job tag filtering distinguish null and empty input from the valid string zero, reuse the resolved tag consistently, and expose truthful controller result types. Add unit, SQLite, and MySQL regressions.
Restore the inherited stop-when-empty-for option that Horizon omitted while replacing the Queue worker signature. Accept the parent command nullable result instead of converting valid maintenance and once-mode paths into type errors.

Exercise the real command with a captured WorkerOptions instance and compare its option surface with the base Queue command, allowing only the Horizon supervisor addition.
Replace public loop flags with protected nullable terminal status, clear pending signals on termination, and stop later commands or monitor work from reopening a terminated owner. Return the exact status through master, supervisor, and console command boundaries.

Preserve intended restart and terminal exit-code classes, including nullable process results and literal zero names. Add isolated signal and command regressions plus a persisted-master readiness barrier so loaded test workers cannot signal before the handler exists or hang after barrier failure.
Add request-scoped CSP nonce rendering and development command registration while preserving Horizon public call shapes. Keep nonce state coroutine-local and retain the supported SMS routing surface for the first-party notification channel.

Declare proxy and snapshot-lock configuration, correct nested fallback ownership and failed-tag retention, preserve exact zero values, and use strict balancing and scaling boundaries. Update the provider to use typed container resolution and record intentional deprecated or inapplicable omissions at future-sync points.
Derive mutable command values and their test resets from one default constant per owner so worker-lifetime cleanup cannot drift from initialization.

Remove Horizon integration cleanup already owned by the framework test subscriber and Redis integration trait. This eliminates duplicate pool flushing and its defer-ordering hazard while retaining exact Queue configuration restoration.
Read each published PHP file and its mode through checked native boundaries, then publish the completed rewrite with Filesystem::replace. Missing, unreadable, and replacement-failure paths retain their command-specific diagnostics and cannot expose partial PHP.

Apply the same owner-local correction to Horizon, Telescope, and Fortify. Fortify stops before later files or provider registration after the first failure, while every successful rewrite preserves its existing mode.
Declare the routing, Symfony HTTP, mbstring, and phpredis requirements Horizon uses directly, and remove the unused Carbon dependency because Horizon consumes Hypervel Support dates.

Extend split-metadata coverage so standalone installs cannot silently lose these runtime edges or reintroduce the stale dependency.
Document current worker options, CSP nonce usage, snapshot retention semantics, and the now-supported Redis Cluster behavior in the same task-focused style as the surrounding framework guide.

Add the upstream reference and concise actionable Laravel differences without exposing internal lifecycle choreography or listing ordinary Hypervel adaptations.
Add the package-owned configure-horizon skill with focused references for supervisors, metrics, tags, and notifications, adapted to Hypervel commands, configuration, and Redis Cluster support.

Keep the final SMS-facing Horizon API and guidance intact, remove only obsolete Nexmo comments, and assign the future first-party Vonage notification channel and functional SMS coverage to one explicit todo owner.
Update Axios to the current Horizon upstream range and lock resolution while leaving unrelated tools and the committed browser bundle unchanged.

This keeps package metadata reproducible without claiming a browser-runtime fix for Node-only adapter advisories.
Drain the child pipe once after a successful reap so a result written between the parent read and process exit is not discarded.

Retain the existing 250 ms observation bound and distinguish an exited child with no message from a child that never finished, without adding timing inflation or production changes.
Record the final Horizon findings, accepted and rejected decisions, cross-package Queue, Redis, Telescope, Fortify, and Reverb ownership, validation, API result, and hot-path assessment.

Close the carried Horizon revalidations, mark the package complete, and route the next audit to Reverb with its exact required ledger entries and pending findings.
@coderabbitai

coderabbitai Bot commented Jul 29, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@binaryfire, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 33 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 1a787317-6244-4903-974a-4d0e124b69ea

📥 Commits

Reviewing files that changed from the base of the PR and between 3950ecf and f049cb5.

📒 Files selected for processing (2)
  • docs/plans/2026-07-28-2043-horizon-cluster-lifecycle-and-current-parity.md
  • tests/Integration/Horizon/Feature/SupervisorCommandTest.php
📝 Walkthrough

Walkthrough

This PR updates Horizon lifecycle exit handling, Redis cluster batching, lock acquisition, queue delay attributes, installer file replacement, CSP nonce rendering, configuration documentation, and regression tests across Horizon, Queue, Redis, Fortify, Telescope, and Reverb.

Changes

Horizon lifecycle and configuration

Layer / File(s) Summary
Exit status propagation and termination
src/horizon/src/Console/*, src/horizon/src/Supervisor.php, src/horizon/src/MasterSupervisor.php
Commands now return monitor statuses, and supervisors stop processing after termination while preserving exit codes.
Cluster-aware Redis operations
src/horizon/src/Repositories/*, src/horizon/src/RedisHorizonCommandQueue.php, src/horizon/src/Repositories/UsesClusterAwarePipeline.php
Redis batching selects transactions for cluster connections and pipelines otherwise.
Dashboard and configuration updates
src/horizon/src/Horizon.php, src/horizon/src/HorizonServiceProvider.php, src/horizon/config/horizon.php
CSP nonce support, development command registration, proxy-path configuration, typed config access, and route normalization are added.
Metrics, locks, tags, and controllers
src/horizon/src/Lock.php, src/horizon/src/Repositories/RedisMetricsRepository.php, src/horizon/src/Tags.php, src/horizon/src/Http/Controllers/*
Lock acquisition, metrics cleanup, tag-state cleanup, search escaping, and controller return types are updated.

Queue, Redis, and installer integration

Layer / File(s) Summary
Queue bulk delay handling
src/queue/src/DatabaseQueue.php, src/queue/src/RedisQueue.php
Bulk jobs can obtain delays from the Delay attribute, with cluster-specific transaction handling for Redis.
Container wiring and return contracts
src/queue/src/QueueServiceProvider.php, src/redis/src/*, src/reverb/src/ReverbServiceProvider.php
Redis and database services are resolved through typed container contracts, and MultiExec return annotations reflect aborted executions.
Installer file updates
src/fortify/src/Console/InstallCommand.php, src/horizon/src/Console/InstallCommand.php, src/telescope/src/Console/InstallCommand.php
Published provider files use the filesystem abstraction, preserve permissions, validate reads, and handle replacement exceptions.

Documentation and validation

Layer / File(s) Summary
Horizon documentation and skills
src/boost/docs/horizon.md, src/horizon/resources/boost/skills/configure-horizon/*, src/horizon/README.md, docs/todo.md
Documentation covers CSP nonces, worker options, metrics retention, notifications, supervisors, tags, and Horizon differences.
Regression coverage
tests/Horizon/*, tests/Integration/Horizon/*, tests/Queue/*, tests/Redis/*, tests/Fortify/*, tests/Telescope/*
Tests cover lifecycle statuses, cluster execution, queue delays, locks, metrics, payload cleanup, wildcard escaping, and installer failures.

Estimated code review effort: 4 (Complex) | ~60 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 77.39% 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 is concise and accurately captures the main themes of the change: Horizon cluster handling, process lifecycle, and publication hardening.
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 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch audit/horizon-cluster-process-parity

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.

@greptile-apps

greptile-apps Bot commented Jul 29, 2026

Copy link
Copy Markdown

Greptile Summary

This PR hardens Horizon’s Redis Cluster, queue, process, lock, metrics, context, and installer lifecycles while restoring current public APIs and package metadata.

  • Uses cluster-aware Redis transactions and hash-tag-compatible queue keys.
  • Preserves bulk-dispatch topology, queue names, and per-job delays.
  • Propagates terminal process statuses and prevents post-termination lifecycle work.
  • Makes locks atomic and owner-safe, and improves metrics cleanup and snapshot behavior.
  • Adds CSP nonce and development-command APIs.
  • Publishes installer rewrites atomically and updates configuration, documentation, dependencies, and regression coverage.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
src/horizon/src/Repositories/UsesClusterAwarePipeline.php Adds a shared Cluster-aware batching boundary that uses transactions for hash-tagged Horizon keys and pipelines for standalone Redis.
src/horizon/src/MasterSupervisor.php Retains terminal exit status and stops signals, commands, persistence, and monitoring after termination.
src/horizon/src/Supervisor.php Propagates supervisor exit status and prevents queued lifecycle work from reactivating a terminated monitor.
src/horizon/src/Lock.php Replaces check-then-expire acquisition with atomic Redis locking and owner-token-safe callback release.
src/horizon/src/RedisQueue.php Uses physical Cluster-safe queue keys and consumes transient payload context before potentially failing preparation.
src/horizon/src/Repositories/RedisMetricsRepository.php Corrects latest-snapshot ranking and performs metrics clearing through one raw Redis lease.
src/queue/src/RedisQueue.php Preserves Cluster topology and resolved delay metadata during bulk dispatch.
src/queue/src/DatabaseQueue.php Honors property- and attribute-based delays when inserting bulk jobs.
src/horizon/src/Console/InstallCommand.php Rewrites the published provider through an atomic filesystem replacement with checked content and permissions.
src/fortify/src/Console/InstallCommand.php Makes namespace rewrites atomic and aborts installation after publication failures.
src/telescope/src/Console/InstallCommand.php Atomically replaces the published provider while preserving file mode and package-specific diagnostics.
src/redis/src/Traits/MultiExec.php Refines callback-sensitive return annotations without changing transaction or pipeline runtime behavior.

Reviews (4): Last reviewed commit: "test: complete Horizon follow-up validat..." | Re-trigger Greptile

@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: 5

🧹 Nitpick comments (5)
tests/Horizon/Unit/RedisHorizonCommandQueueTest.php (1)

15-26: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Use PHPUnit attributes for the added tests.

  • tests/Horizon/Unit/RedisHorizonCommandQueueTest.php#L15-L26: import PHPUnit\Framework\Attributes\Test and annotate both new tests.
  • tests/Integration/Horizon/Feature/RedisPayloadTest.php#L225-L239: import PHPUnit\Framework\Attributes\Test and annotate the new test.
  • tests/Redis/MultiExecTest.php#L70-L85: import PHPUnit\Framework\Attributes\Test and annotate the new test.
  • tests/Redis/MultiExecTest.php#L129-L144: annotate the new test using the same import.

As per coding guidelines, “Test methods must declare : void, use PHPUnit attributes instead of docblock annotations.”

🤖 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/Horizon/Unit/RedisHorizonCommandQueueTest.php` around lines 15 - 26,
Update the added tests to use PHPUnit’s Test attribute and declare void return
types: annotate both testPending methods in
tests/Horizon/Unit/RedisHorizonCommandQueueTest.php (lines 15-26), the new test
in tests/Integration/Horizon/Feature/RedisPayloadTest.php (lines 225-239), and
both new tests in tests/Redis/MultiExecTest.php (lines 70-85 and 129-144). Add
the Test import once per affected file and use the existing method names as
anchors.

Source: Coding guidelines

src/horizon/src/Console/InstallCommand.php (1)

89-99: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Resolve Filesystem via constructor injection instead of make().

Same concern as src/fortify/src/Console/InstallCommand.php — deferred to the consolidated comment.

🤖 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/horizon/src/Console/InstallCommand.php` around lines 89 - 99, Update
InstallCommand to receive the Filesystem dependency through constructor
injection and store it for use in the installation flow. Replace the
hypervel->make(Filesystem::class) call in the try block with the injected
Filesystem instance, preserving the existing replace arguments and exception
handling.

Source: Path instructions

src/fortify/src/Console/InstallCommand.php (2)

1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Inject Filesystem via the constructor instead of $this->hypervel->make(Filesystem::class). All three installers resolve Filesystem from the container inside a protected method rather than through injection, even though constructor injection is straightforward for a Command class — the shared fix is to inject Filesystem once and reuse it.

  • src/fortify/src/Console/InstallCommand.php#L92-93: add a constructor accepting Filesystem $files, store it, and replace $this->hypervel->make(Filesystem::class) with $this->files in updatePublishedNamespaces.
  • src/horizon/src/Console/InstallCommand.php#L89-99: same — inject Filesystem in the constructor and use it in registerHorizonServiceProvider instead of $this->hypervel->make(Filesystem::class).
  • src/telescope/src/Console/InstallCommand.php#L135-145: same — inject Filesystem in the constructor and use it in registerTelescopeServiceProvider instead of $this->hypervel->make(Filesystem::class).
♻️ Illustrative fix (Fortify; same shape applies to Horizon/Telescope)
+use Hypervel\Filesystem\Filesystem;
 #[AsCommand(name: 'fortify:install')]
 class InstallCommand extends Command
 {
+    public function __construct(private readonly Filesystem $files)
+    {
+        parent::__construct();
+    }
+
     ...
     protected function updatePublishedNamespaces(string $namespace): bool
     {
-        $filesystem = $this->hypervel->make(Filesystem::class);
+        $filesystem = $this->files;
🤖 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/fortify/src/Console/InstallCommand.php` at line 1, Update the
constructors of the Fortify, Horizon, and Telescope InstallCommand classes to
accept and store an injected Filesystem dependency, then replace each protected
method’s hypervel->make(Filesystem::class) lookup with the stored property in
updatePublishedNamespaces, registerHorizonServiceProvider, and
registerTelescopeServiceProvider respectively.

Source: Path instructions


92-93: 📐 Maintainability & Code Quality | 🔵 Trivial

Resolve Filesystem via constructor injection instead of make().

Per path instructions for src/**/*.php, framework services should be constructor/method-injected; Container::getInstance()->make() (or the equivalent $this->hypervel->make()) is reserved for cases where injection is impossible. Here it isn't — Filesystem can be injected into the command's constructor and reused by updatePublishedNamespaces.

Deferred to the consolidated comment (shared with src/horizon/src/Console/InstallCommand.php and src/telescope/src/Console/InstallCommand.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 `@src/fortify/src/Console/InstallCommand.php` around lines 92 - 93, Update the
Console InstallCommand constructor to inject and store a Filesystem dependency,
then reuse that property in the command flow and updatePublishedNamespaces
instead of resolving Filesystem through $this->hypervel->make().

Source: Path instructions

src/telescope/src/Console/InstallCommand.php (1)

135-145: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Resolve Filesystem via constructor injection instead of make().

Same concern as the Fortify and Horizon installers — deferred to the consolidated comment.

🤖 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/Console/InstallCommand.php` around lines 135 - 145, Update
the installer class constructor to receive and store a Filesystem dependency,
then use that injected instance in the replacement logic within the try block
instead of resolving Filesystem through $this->hypervel->make(). Keep the
existing replace arguments and exception handling unchanged.

Source: Path instructions

🤖 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/boost/docs/horizon.md`:
- Around line 63-90: Update the CSP nonce example around Horizon::cspNonce to
generate a fresh cryptographically random nonce inside handle for each request
instead of passing the constant 'csp-nonce'. Reuse that generated nonce when
setting the response’s CSP header, while preserving the middleware flow and
Horizon view nonce configuration.

In `@src/horizon/src/Repositories/UsesClusterAwarePipeline.php`:
- Around line 12-23: Update UsesClusterAwarePipeline::pipeline to detect when
the selected transaction or pipeline operation returns false before the array
return. Throw a meaningful exception for an aborted EXEC result, while
preserving the existing array result behavior for successful operations.

In `@src/queue/src/RedisQueue.php`:
- Around line 252-260: Preserve legacy property-based delays by falling back to
the job’s delay property when no Delay attribute value is found. In
src/queue/src/RedisQueue.php lines 252-260, update the delay selection before
later/push branching; apply the same fallback before computing available_at in
src/queue/src/DatabaseQueue.php lines 275-282. Use the existing
isset($job->delay) behavior and keep attribute values as the first priority.

In `@tests/Fortify/Console/InstallCommandTest.php`:
- Line 1: Update all three install-command tests in the relevant test class to
guard or skip the unreadable-file precondition when the process runs as root.
Detect root execution before asserting that `@file_get_contents`($path) returns
false, while preserving the existing unreadable-file assertions for non-root
users.

In `@tests/Horizon/HorizonTest.php`:
- Around line 43-58: In testCspNonceIsIsolatedBetweenConcurrentRequests, import
Hypervel\Support\Sleep and replace both raw usleep(5000) calls inside the
parallel closures with Sleep::usleep(5000), preserving the existing delay and
assertions.

---

Nitpick comments:
In `@src/fortify/src/Console/InstallCommand.php`:
- Line 1: Update the constructors of the Fortify, Horizon, and Telescope
InstallCommand classes to accept and store an injected Filesystem dependency,
then replace each protected method’s hypervel->make(Filesystem::class) lookup
with the stored property in updatePublishedNamespaces,
registerHorizonServiceProvider, and registerTelescopeServiceProvider
respectively.
- Around line 92-93: Update the Console InstallCommand constructor to inject and
store a Filesystem dependency, then reuse that property in the command flow and
updatePublishedNamespaces instead of resolving Filesystem through
$this->hypervel->make().

In `@src/horizon/src/Console/InstallCommand.php`:
- Around line 89-99: Update InstallCommand to receive the Filesystem dependency
through constructor injection and store it for use in the installation flow.
Replace the hypervel->make(Filesystem::class) call in the try block with the
injected Filesystem instance, preserving the existing replace arguments and
exception handling.

In `@src/telescope/src/Console/InstallCommand.php`:
- Around line 135-145: Update the installer class constructor to receive and
store a Filesystem dependency, then use that injected instance in the
replacement logic within the try block instead of resolving Filesystem through
$this->hypervel->make(). Keep the existing replace arguments and exception
handling unchanged.

In `@tests/Horizon/Unit/RedisHorizonCommandQueueTest.php`:
- Around line 15-26: Update the added tests to use PHPUnit’s Test attribute and
declare void return types: annotate both testPending methods in
tests/Horizon/Unit/RedisHorizonCommandQueueTest.php (lines 15-26), the new test
in tests/Integration/Horizon/Feature/RedisPayloadTest.php (lines 225-239), and
both new tests in tests/Redis/MultiExecTest.php (lines 70-85 and 129-144). Add
the Test import once per affected file and use the existing method names as
anchors.
🪄 Autofix (Beta)

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: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 8feaea29-cba7-4424-acd0-7bb9d70c5fb6

📥 Commits

Reviewing files that changed from the base of the PR and between 5503432 and 8dfe143.

⛔ Files ignored due to path filters (1)
  • src/horizon/package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (95)
  • AGENTS.md
  • docs/plans/2026-07-12-0900-framework-coroutine-state-lifecycle-audit.md
  • docs/plans/2026-07-12-0915-framework-coroutine-state-lifecycle-audit-ledger.md
  • docs/todo.md
  • src/boost/docs/horizon.md
  • src/fortify/src/Console/InstallCommand.php
  • src/horizon/README.md
  • src/horizon/composer.json
  • src/horizon/config/horizon.php
  • src/horizon/package.json
  • src/horizon/resources/boost/skills/configure-horizon/SKILL.blade.php
  • src/horizon/resources/boost/skills/configure-horizon/references/metrics.md
  • src/horizon/resources/boost/skills/configure-horizon/references/notifications.md
  • src/horizon/resources/boost/skills/configure-horizon/references/supervisors.md
  • src/horizon/resources/boost/skills/configure-horizon/references/tags.md
  • src/horizon/src/AutoScaler.php
  • src/horizon/src/Console/HorizonCommand.php
  • src/horizon/src/Console/InstallCommand.php
  • src/horizon/src/Console/SnapshotCommand.php
  • src/horizon/src/Console/SupervisorCommand.php
  • src/horizon/src/Console/TerminateCommand.php
  • src/horizon/src/Console/WorkCommand.php
  • src/horizon/src/Horizon.php
  • src/horizon/src/HorizonServiceProvider.php
  • src/horizon/src/Http/Controllers/BatchesController.php
  • src/horizon/src/Http/Controllers/FailedJobsController.php
  • src/horizon/src/Listeners/MonitorMasterSupervisorMemory.php
  • src/horizon/src/Listeners/SendNotification.php
  • src/horizon/src/Listeners/StoreTagsForFailedJob.php
  • src/horizon/src/Listeners/UpdateJobMetrics.php
  • src/horizon/src/Lock.php
  • src/horizon/src/MasterSupervisor.php
  • src/horizon/src/Notifications/LongWaitDetected.php
  • src/horizon/src/ProcessPool.php
  • src/horizon/src/RedisHorizonCommandQueue.php
  • src/horizon/src/RedisQueue.php
  • src/horizon/src/Repositories/RedisJobRepository.php
  • src/horizon/src/Repositories/RedisMasterSupervisorRepository.php
  • src/horizon/src/Repositories/RedisMetricsRepository.php
  • src/horizon/src/Repositories/RedisProcessRepository.php
  • src/horizon/src/Repositories/RedisSupervisorRepository.php
  • src/horizon/src/Repositories/RedisTagRepository.php
  • src/horizon/src/Repositories/UsesClusterAwarePipeline.php
  • src/horizon/src/Supervisor.php
  • src/horizon/src/SupervisorCommandString.php
  • src/horizon/src/SupervisorOptions.php
  • src/horizon/src/SupervisorProcess.php
  • src/horizon/src/SystemProcessCounter.php
  • src/horizon/src/Tags.php
  • src/horizon/src/WorkerCommandString.php
  • src/queue/src/DatabaseQueue.php
  • src/queue/src/QueueServiceProvider.php
  • src/queue/src/RedisQueue.php
  • src/redis/src/Listeners/RedisConnectionLifecycleListener.php
  • src/redis/src/RedisServiceProvider.php
  • src/redis/src/Traits/MultiExec.php
  • src/reverb/src/ReverbServiceProvider.php
  • src/telescope/src/Console/InstallCommand.php
  • tests/Fortify/Console/InstallCommandTest.php
  • tests/Horizon/Console/InstallCommandTest.php
  • tests/Horizon/Console/SnapshotCommandTest.php
  • tests/Horizon/Console/WorkCommandTest.php
  • tests/Horizon/HorizonConfigTest.php
  • tests/Horizon/HorizonTest.php
  • tests/Horizon/PackageMetadataTest.php
  • tests/Horizon/Unit/FailedJobsControllerTest.php
  • tests/Horizon/Unit/LockTest.php
  • tests/Horizon/Unit/RedisHorizonCommandQueueTest.php
  • tests/Horizon/Unit/RedisMetricsRepositoryTest.php
  • tests/Horizon/Unit/RedisQueueTest.php
  • tests/Horizon/Unit/StaticStateTest.php
  • tests/Horizon/Unit/UpdateJobMetricsTest.php
  • tests/Horizon/Unit/UsesClusterAwarePipelineTest.php
  • tests/Horizon/UnitTestCase.php
  • tests/Integration/Horizon/Controller/BatchesControllerTest.php
  • tests/Integration/Horizon/Database/MySql/BatchesControllerTest.php
  • tests/Integration/Horizon/Feature/Fakes/SupervisorWithFakeMonitor.php
  • tests/Integration/Horizon/Feature/Fixtures/FakeSupervisorFactory.php
  • tests/Integration/Horizon/Feature/HorizonCommandTest.php
  • tests/Integration/Horizon/Feature/Listeners/StoreTagsForFailedTest.php
  • tests/Integration/Horizon/Feature/LockTest.php
  • tests/Integration/Horizon/Feature/MasterSupervisorTest.php
  • tests/Integration/Horizon/Feature/MetricsTest.php
  • tests/Integration/Horizon/Feature/ProcessExitStatusTest.php
  • tests/Integration/Horizon/Feature/QueueProcessingTest.php
  • tests/Integration/Horizon/Feature/RedisPayloadTest.php
  • tests/Integration/Horizon/Feature/SupervisorCommandTest.php
  • tests/Integration/Horizon/Feature/SupervisorOptionsTest.php
  • tests/Integration/Horizon/Feature/SupervisorTest.php
  • tests/Integration/Horizon/IntegrationTestCase.php
  • tests/Queue/QueueDatabaseQueueUnitTest.php
  • tests/Queue/QueueRedisQueueTest.php
  • tests/Redis/MultiExecTest.php
  • tests/Reverb/Servers/Hypervel/Scaling/SwooleTableSharedStateLockTest.php
  • tests/Telescope/Console/InstallCommandTest.php
💤 Files with no reviewable changes (2)
  • src/horizon/src/Listeners/SendNotification.php
  • tests/Integration/Horizon/IntegrationTestCase.php

Comment thread src/boost/docs/horizon.md
Comment thread src/horizon/src/Repositories/UsesClusterAwarePipeline.php
Comment thread src/queue/src/RedisQueue.php
Comment thread tests/Fortify/Console/InstallCommandTest.php
Comment thread tests/Horizon/HorizonTest.php
Document the agreed Horizon cluster, process, publication, configuration, metadata, and current-parity design, including the owning Queue, Redis, Telescope, Fortify, Boost, and test boundaries.

Record the supporting upstream research, anti-overengineering constraints, implementation order, focused coverage, authoritative validation, and final performance assessment so future maintenance can recover the decisions without reconstructing the audit.
Replace root-user and denied-mode assumptions with direct read and write capability probes. This keeps the failure paths active for ordinary users while allowing privileged CI environments to skip cases they cannot reproduce.\n\nCreate filesystem adapters and Mockery expectations only after the write precondition holds, retain mode restoration in finally blocks, and remove dead returns after PHPUnit's throwing skip boundary.
Replace the supervisor niceness test's shell and procps dependency with pcntl_getpriority(), which is provided by Horizon's required ext-pcntl runtime. Complete the touched integration test signatures at the same boundary.

Retain procps in Redis CI for the production SystemProcessCounter path and name that ownership explicitly, preventing the test cleanup from accidentally removing a runtime dependency.
Generate a fresh nonce per request and show the matching script-src and style-src response directives, so the documented Horizon CSP setup is complete and safe to copy.

Record the capability-aware permission regressions, native process-priority boundary, CSP correction, and completed cross-package ownership in the Horizon plan and audit ledger.
@binaryfire

Copy link
Copy Markdown
Collaborator Author

@coderabbitai Follow-up complete.

  • Fixed the CSP example with a fresh per-request nonce shared by the Horizon asset tags and both response-header directives.
  • Fixed root-sensitive permission regressions by probing actual capabilities, including the same-family Filesystem tests.
  • Kept the strict Horizon batch return because Horizon never uses WATCH, so an aborted EXEC is not reachable on this path.
  • Kept the queue delay lookup because the existing attribute reader already preserves runtime property fallback.
  • Kept raw usleep() in the coroutine-isolation test because real scheduling is the condition under test.
  • Replaced the shell-based niceness assertion with pcntl_getpriority(), while retaining procps for the production SystemProcessCounter path.

The full verification gate is green, including the Horizon integration group.

@binaryfire

Copy link
Copy Markdown
Collaborator Author

@coderabbitai @greptileai The implementation plan is now committed at docs/plans/2026-07-28-2043-horizon-cluster-lifecycle-and-current-parity.md.

Please use it as additional review context and flag any plan obligation that is not reflected in the code. It records the final design, rejected machinery, test boundaries, API compatibility, and performance constraints.

@coderabbitai

coderabbitai Bot commented Jul 29, 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

plan='docs/plans/2026-07-28-2043-horizon-cluster-lifecycle-and-current-parity.md'

printf '%s\n' '=== Plan location and outline ==='
fd -a '^2026-07-28-2043-horizon-cluster-lifecycle-and-current-parity\.md$' docs/plans
ast-grep outline "$plan" --items all || true
printf '%s\n' '=== Plan contents (numbered) ==='
cat -n "$plan"

printf '%s\n' '=== Changed files ==='
git diff --name-status "$(git merge-base HEAD origin/main)" HEAD 2>/dev/null || git diff --name-status HEAD~1 HEAD

printf '%s\n' '=== Candidate obligation/constraint terms in plan ==='
rg -n -i -C2 'must|shall|required|acceptance|test|compatib|reject|avoid|performance|constraint|atomic|cluster|watch|nonce|permission|lock|delay|exit|cleanup' "$plan"

Length of output: 91890


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '=== Repository state and recent commits ==='
git status --short
git log --oneline -12

printf '%s\n' '=== Required source/test files that are present ==='
for path in \
  src/horizon/src/Console/WorkCommand.php \
  src/horizon/src/Repositories/UsesClusterAwarePipeline.php \
  src/horizon/src/Repositories/RedisMetricsRepository.php \
  src/horizon/src/Lock.php \
  src/horizon/src/Horizon.php \
  src/horizon/src/Console/InstallCommand.php \
  src/telescope/src/Console/InstallCommand.php \
  src/fortify/src/Console/InstallCommand.php \
  tests/Horizon/Console/WorkCommandTest.php \
  tests/Horizon/Unit/UsesClusterAwarePipelineTest.php \
  tests/Horizon/Unit/RedisMetricsRepositoryTest.php \
  tests/Horizon/Unit/LockTest.php \
  tests/Horizon/HorizonTest.php \
  tests/Fortify/Console/InstallCommandTest.php \
  tests/Horizon/Console/InstallCommandTest.php \
  tests/Telescope/Console/InstallCommandTest.php \
  src/boost/docs/horizon.md \
  src/horizon/package.json \
  src/horizon/package-lock.json
do
  test -f "$path" && printf 'present %s\n' "$path" || printf 'MISSING %s\n' "$path"
done

printf '%s\n' '=== Required implementation markers ==='
rg -n -C2 \
  'stop-when-empty-for|function handle\(\): \?int|UsesClusterAwarePipeline|function pipeline\(callable|isCluster\(\).*transaction|snapshot_lock|withConnection\(|flushByPattern|assertPositiveLifetime|new RedisLock|CSP_NONCE_CONTEXT_KEY|registerDevCommands|CoroutineContext::forget|createPayload\(\$job, \$this->getQueue\(\$queue\), \$data, \$delay\)|protected \?int \$exitStatus|pcntl_getpriority|Filesystem::replace|`@file_get_contents`|`@fileperms`' \
  src/horizon src/queue src/redis src/telescope src/fortify src/reverb tests/Horizon tests/Queue tests/Redis tests/Telescope tests/Fortify tests/Reverb 2>/dev/null

printf '%s\n' '=== Forbidden/stale markers (matches require review) ==='
rg -n -C1 \
  'shouldExitLoop|function getLastPushed|file_put_contents|horizon:publish|Horizon::night|Redis Cluster.*not compatible|trim\(shell_exec|GuidelineAssist|Nexmo|setNx\(|->expire\(' \
  src/horizon src/boost/docs/horizon.md tests/Horizon tests/Telescope tests/Fortify 2>/dev/null || true

printf '%s\n' '=== Installer capability-probe test excerpts ==='
for path in \
  tests/Horizon/Console/InstallCommandTest.php \
  tests/Telescope/Console/InstallCommandTest.php \
  tests/Fortify/Console/InstallCommandTest.php
do
  printf '\n--- %s ---\n' "$path"
  rg -n -C4 'chmod|file_get_contents|is_readable|is_writable|markTestSkipped|replace|permissions|cannot be read|cannot be writ' "$path" || true
done

printf '%s\n' '=== CSP guide and Boost metadata excerpts ==='
rg -n -C5 'cspNonce|Content-Security-Policy|script-src|style-src|Str::random|configuring-horizon|GuidelineAssist|Sail' \
  src/boost/docs/horizon.md \
  src/horizon/resources/boost/skills/configure-horizon/SKILL.blade.php 2>/dev/null || true

printf '%s\n' '=== Frontend lock resolution ==='
rg -n -C2 '"axios"|axios@|axios:' src/horizon/package.json src/horizon/package-lock.json 2>/dev/null || true

Length of output: 65266


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '=== Files touched by the capability-aware permission commit ==='
git show --format=fuller --name-status b4d53283
printf '%s\n' '=== Files touched by the native niceness commit ==='
git show --format=fuller --name-status 7af7d885

printf '%s\n' '=== Filesystem write-capability probes ==='
rg -n -C5 'is_writable|file_put_contents|chmod|markTestSkipped|write permission|writable' \
  tests src/filesystem 2>/dev/null || true

printf '%s\n' '=== Direct Horizon connection pipeline calls and trait usage ==='
rg -n -C2 --glob '*.php' '->pipeline\s*\(' src/horizon/src
printf '%s\n' '--- Trait consumers ---'
rg -n -C2 --glob '*.php' 'use UsesClusterAwarePipeline;' src/horizon/src

printf '%s\n' '=== Plan finding IDs in companion audit ledger ==='
ledger='docs/plans/2026-07-12-0915-framework-coroutine-state-lifecycle-audit-ledger.md'
rg -n -C2 'horizon-(01|02|03|04|05|06|07|08|09|10|11|12|13|14|15|16|17|18|19|20|21)|queue-22|queue-40|redis-23|telescope-03|fortify-01|filesystem-13|reverb-06|support-02' "$ledger" || true

printf '%s\n' '=== Targeted coverage names from the plan ==='
rg -n -C1 \
  'test.*(Cluster|Delay|ReadyNow|Snapshot|Timer|Literal|Exit|Termination|Nonce|RawPush|Listener|Permission|Missing|Replacement|Niceness|MultiExec)|pcntl_getpriority' \
  tests/Horizon tests/Queue tests/Redis tests/Telescope tests/Fortify tests/Reverb 2>/dev/null || true

printf '%s\n' '=== CSP API docblock and development registration ==='
sed -n '205,245p' src/horizon/src/Horizon.php
sed -n '138,156p' src/horizon/src/HorizonServiceProvider.php

printf '%s\n' '=== Touched/new test methods lacking void return type ==='
git diff --unified=0 4f5af818^..HEAD -- tests \
  | awk '
      /^\+\+\+ b\// { file=substr($0,7); next }
      /^\+.*function test/ && $0 !~ /: void[[:space:]]*(\{|$)/ { print file ": " $0 }
    ' || true

Length of output: 202576


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '=== Direct Horizon pipeline calls (excluding trait implementation) ==='
rg -n --glob '*.php' -- '->pipeline\s*\(' src/horizon/src \
  | grep -v 'UsesClusterAwarePipeline.php' || true

printf '%s\n' '=== Trait consumers ==='
rg -n --glob '*.php' 'use UsesClusterAwarePipeline;' src/horizon/src

printf '%s\n' '=== Ledger IDs: matched lines only ==='
ledger='docs/plans/2026-07-12-0915-framework-coroutine-state-lifecycle-audit-ledger.md'
rg -n -o \
  'horizon-(01|02|03|04|05|06|07|08|09|10|11|12|13|14|15|16|17|18|19|20|21)|queue-22|queue-40|redis-23|telescope-03|fortify-01|filesystem-13|reverb-06|support-02' \
  "$ledger" | sort -u

printf '%s\n' '=== Changed test methods without : void ==='
git diff --unified=0 4f5af818^..HEAD -- tests \
  | awk '
      /^\+\+\+ b\// { file=substr($0,7); next }
      /^\+.*function test/ && $0 !~ /: void[[:space:]]*(\{|$)/ { print file ": " $0 }
    ' || true

printf '%s\n' '=== Recent cross-package commit file lists ==='
for commit in b4d53283 7af7d885 bb80acaf; do
  echo "--- $commit ---"
  git show --format='' --name-only "$commit"
done

Length of output: 5534


@binaryfire I found no plan obligation that is absent from the current implementation.

Confirmed in particular:

  • All 17 Horizon batching call sites now use the cluster-aware trait; the six planned owners consume it, with no direct underlying Horizon connection()->pipeline(...) calls left outside the trait.
  • Queue bulk topology/delay handling, Horizon queue-key handling, metrics one-lease clearing, lock ownership/atomicity, terminal exit-status propagation, and consume-once coroutine context cleanup are reflected in source and focused tests.
  • Horizon, Telescope, and Fortify installer rewrites use checked atomic replacement. Their read-failure tests are capability-aware, and the two Filesystem adapter regressions correctly probe actual writability before skipping.
  • The CSP guide now creates one fresh request nonce and shares it between Horizon::cspNonce() and both script-src / style-src response directives.
  • Required docs, Boost resources, Axios metadata/lock synchronization, split-package metadata, native niceness assertion, Reverb harness adjustment, and companion-ledger records are present.
  • The changed test methods inspected all declare : void.

This was a static plan-to-code/test/documentation audit; I did not rerun the PR’s runtime verification gate in this environment.

@coderabbitai

coderabbitai Bot commented Jul 29, 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

printf '%s\n' '== Relevant files =='
fd -a -t f \
  '2026-07-28-2043-horizon-cluster-lifecycle-and-current-parity\.md' \
  'horizon\.md' \
  'InstallCommand(Test)?\.php' \
  'Horizon(Test)?\.php' \
  'SystemProcessCounter\.php' \
  '.*Niceness.*\.php' \
  '.*Priority.*\.php' \
  . | sort

printf '%s\n' '== Changed paths relevant to follow-up =='
git diff --name-only HEAD^ HEAD -- \
  'docs/plans/2026-07-28-2043-horizon-cluster-lifecycle-and-current-parity.md' \
  'src/boost/docs/horizon.md' \
  'src/horizon' \
  'src/fortify' \
  'src/telescope' \
  'tests/Horizon' \
  'tests/Fortify' \
  'tests/Telescope' | sort

printf '%s\n' '== PHP outlines =='
for f in \
  $(fd -a -t f 'Horizon\.php|InstallCommandTest\.php|SystemProcessCounter\.php|.*Niceness.*\.php|.*Priority.*\.php' src tests | sort); do
  printf '\n-- %s --\n' "$f"
  ast-grep outline "$f" --items all --type class,function,method --view compact || true
done

printf '%s\n' '== Targeted follow-up references =='
rg -n -C 3 \
  'cspNonce|nonce-|Content-Security-Policy|fileperms|is_readable|posix_geteuid|pcntl_getpriority|procps|WATCH|EXEC|Delay::class|getAttributeValue|usleep' \
  docs src tests

Length of output: 201717


🏁 Script executed:

#!/bin/bash
set -euo pipefail

plan=$(fd -a -t f '2026-07-28-2043-horizon-cluster-lifecycle-and-current-parity\.md' docs | head -n 1)
printf 'PLAN=%s\n' "$plan"
printf '%s\n' '== Plan headings and obligation keywords =='
rg -n -i -C 2 \
  '^(#|##|###) |obligation|CSP|nonce|permission|install|WATCH|EXEC|delay|niceness|priority|procps|usleep|test boundar|acceptance|verification' \
  "$plan"

printf '%s\n' '== Exact relevant paths =='
fd -a -t f \
  'Horizon\.php' \
  'HorizonTest\.php' \
  'InstallCommandTest\.php' \
  'SystemProcessCounter\.php' \
  'SystemProcessCounterTest\.php' \
  '.*Process.*Test\.php' \
  'horizon\.md' \
  src/horizon src/boost tests/Horizon tests/Fortify tests/Telescope | sort

printf '%s\n' '== CSP docs, implementation, and tests =='
for f in src/boost/docs/horizon.md src/horizon/src/Horizon.php tests/Horizon/HorizonTest.php; do
  if [ -f "$f" ]; then
    echo "-- $f --"
    rg -n -i -C 12 'nonce|Content-Security-Policy|cspNonce|CoroutineContext' "$f" || true
  fi
done

printf '%s\n' '== Installer permission and capability test cases =='
for f in tests/Horizon/Console/InstallCommandTest.php tests/Fortify/Console/InstallCommandTest.php tests/Telescope/Console/InstallCommandTest.php; do
  if [ -f "$f" ]; then
    echo "-- $f --"
    rg -n -C 10 'permission|readable|is_readable|posix_geteuid|chmod|capabil|replace' "$f" || true
  fi
done

printf '%s\n' '== Niceness production and verification =='
rg -n -i -C 10 'nice|priority|pcntl_getpriority|procps|ps ' \
  src/horizon tests/Horizon docs/plans \
  -g '!**/dist/**' -g '!**/node_modules/**' || true

printf '%s\n' '== Follow-up related diffs from merge-base =='
base=$(git merge-base HEAD origin/main 2>/dev/null || git merge-base HEAD main 2>/dev/null || true)
if [ -n "$base" ]; then
  git diff --unified=4 "$base"...HEAD -- \
    "$plan" \
    src/boost/docs/horizon.md \
    src/horizon/src/Horizon.php \
    tests/Horizon/HorizonTest.php \
    tests/Horizon/Console/InstallCommandTest.php \
    tests/Fortify/Console/InstallCommandTest.php \
    tests/Telescope/Console/InstallCommandTest.php \
    src/horizon/src/SystemProcessCounter.php \
    tests/Horizon \
  | sed -n '1,1800p'
else
  echo 'Could not determine merge base.'
fi

Length of output: 202682


🏁 Script executed:

#!/bin/bash
set -euo pipefail

plan='docs/plans/2026-07-28-2043-horizon-cluster-lifecycle-and-current-parity.md'

printf '%s\n' '== Requested plan: headings and follow-up obligations =='
sed -n '1,260p' "$plan"
printf '\n== CSP / permissions / queue / transaction / niceness references in requested plan ==\n'
rg -n -i -C 5 \
  'csp|nonce|permission|capabilit|watch|exec|delay|attribute|usleep|nice|priority|procps|test boundary|verification' \
  "$plan"

printf '%s\n' '== Current CSP documentation, implementation, and test =='
for f in \
  src/boost/docs/horizon.md \
  src/horizon/src/Horizon.php \
  tests/Horizon/HorizonTest.php; do
  echo "-- $f --"
  rg -n -i -C 8 'csp|nonce|CoroutineContext|parallel|usleep' "$f" || true
done

printf '%s\n' '== Current installer capability/permission tests =='
for f in \
  tests/Horizon/Console/InstallCommandTest.php \
  tests/Fortify/Console/InstallCommandTest.php \
  tests/Telescope/Console/InstallCommandTest.php; do
  echo "-- $f --"
  rg -n -i -C 8 'permission|readable|capabilit|chmod|fileperms|posix|replace' "$f" || true
done

printf '%s\n' '== Current niceness production/test references =='
rg -n -i -C 8 'pcntl_getpriority|procps|nice|priority' \
  src/horizon tests/Horizon \
  -g '*.php' || true

printf '%s\n' '== Commits touching exact follow-up paths =='
git log --format='%H %s' -n 20 -- \
  "$plan" \
  src/boost/docs/horizon.md \
  src/horizon/src/Horizon.php \
  tests/Horizon/HorizonTest.php \
  tests/Horizon/Console/InstallCommandTest.php \
  tests/Fortify/Console/InstallCommandTest.php \
  tests/Telescope/Console/InstallCommandTest.php \
  src/horizon/src/SystemProcessCounter.php \
  tests/Horizon

printf '%s\n' '== Focused current-commit patches (if those paths changed in HEAD) =='
git diff --unified=5 HEAD^ HEAD -- \
  "$plan" \
  src/boost/docs/horizon.md \
  src/horizon/src/Horizon.php \
  tests/Horizon/HorizonTest.php \
  tests/Horizon/Console/InstallCommandTest.php \
  tests/Fortify/Console/InstallCommandTest.php \
  tests/Telescope/Console/InstallCommandTest.php \
  src/horizon/src/SystemProcessCounter.php \
  tests/Horizon \
  || true

Length of output: 87156


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '== Capability-aware follow-up commit: affected files and patch =='
git show --format='%H%n%s' --name-only --pretty='' b4d5328344149773580b7b9b43b276b8f8c82298
git show --format='' --unified=8 b4d5328344149773580b7b9b43b276b8f8c82298 -- \
  tests/Horizon/Console/InstallCommandTest.php \
  tests/Fortify/Console/InstallCommandTest.php \
  tests/Telescope/Console/InstallCommandTest.php \
  tests/Filesystem \
  | sed -n '1,1600p'

printf '%s\n' '== Native niceness follow-up: matching current source/tests =='
rg -n -i -C 10 'pcntl_getpriority|proc_nice|SystemProcessCounter|procps|shell_exec|nice' \
  src/horizon tests \
  -g '*.php' \
  -g '!tests/**/Fixtures/**' \
  | sed -n '1,1600p'

printf '%s\n' '== Exact files and current test contexts for capability probes =='
rg -l -i 'current user can.*writ|read files without read permission|file remains writable|pcntl_getpriority' \
  tests -g '*.php' | sort | while IFS= read -r f; do
    echo "-- $f --"
    rg -n -i -C 12 \
      'current user can.*writ|read files without read permission|file remains writable|pcntl_getpriority' \
      "$f"
  done

Length of output: 68707


@binaryfire Verified. The reviewed follow-up obligations are reflected in the current branch:

  • The Horizon CSP guide now generates a fresh request nonce, applies it to Horizon assets, and uses that same nonce in both CSP directives.
  • Horizon, Telescope, Fortify, and the same-family Filesystem tests now probe actual read/write capability instead of assuming mode bits or relying on whoami.
  • The plan and implementation retain the justified Horizon no-WATCH transaction contract, the existing Queue delay property fallback, and the coroutine-scheduling usleep() test.
  • The niceness regression now uses pcntl_getpriority(), while the production system-process counter remains intact.

I found no remaining unmet plan obligation among these follow-up items. I did not independently run the verification gate in this sandbox.


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

Timestamp: 2026-07-29T11:36:19.687Z
Learning: For Horizon Cluster metadata batching, `Hypervel\Horizon\Repositories\UsesClusterAwarePipeline` may retain its strict `array` transaction result because Horizon does not issue Redis `WATCH`; therefore an aborted `EXEC` result is not reachable on this path.

Learnt from: binaryfire
URL: https://github.com/hypervel/components/pull/459

Timestamp: 2026-07-29T11:36:19.687Z
Learning: Queue bulk delay resolution in `Hypervel\Queue\RedisQueue` and `Hypervel\Queue\DatabaseQueue` must continue using the existing `getAttributeValue($job, Delay::class, 'delay')` reader because it preserves runtime public `$delay` property fallback as well as `#[Delay]` attributes.

Learnt from: binaryfire
URL: https://github.com/hypervel/components/pull/459

Timestamp: 2026-07-29T11:36:19.687Z
Learning: Coroutine-isolation tests such as `tests/Horizon/HorizonTest.php` may deliberately use raw `usleep()` when yielding and real concurrent scheduling are the behavior being tested.

Learnt from: binaryfire
URL: https://github.com/hypervel/components/pull/459

Timestamp: 2026-07-29T11:36:19.687Z
Learning: For Horizon supervisor niceness tests, use `pcntl_getpriority()` rather than shelling out to `ps`; `ext-pcntl` is a Horizon runtime requirement. Keep the production `Hypervel\Horizon\SystemProcessCounter` procps-based process-counting path unchanged.

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!

@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: 2

🧹 Nitpick comments (1)
tests/Integration/Horizon/Feature/SupervisorCommandTest.php (1)

72-77: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Remove the unused $factory variable.

The factory instance is not read in this test; register it directly with the container instead.

🤖 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/Integration/Horizon/Feature/SupervisorCommandTest.php` around lines 72
- 77, Remove the unused $factory assignment in
testSupervisorCommandCanSetProcessNiceness and register the new
FakeSupervisorFactory directly via app->instance while preserving the existing
supervisor command and priority assertion.

Source: Linters/SAST tools

🤖 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 `@docs/plans/2026-07-28-2043-horizon-cluster-lifecycle-and-current-parity.md`:
- Around line 1134-1144: Update the “Focused tests” validation commands to run
tests/Filesystem/FilesystemAdapterTest.php before the existing Horizon, Queue,
Redis, Telescope, and Fortify groups. Preserve the remaining validation order
and include the changed Filesystem regression in the focused test stage.

In `@tests/Filesystem/FilesystemAdapterTest.php`:
- Around line 875-887: Update both permission-changing tests, including
testThrowExceptionsForPut and the corresponding test near the later referenced
block, to call clearstatcache for foo.txt immediately after chmod(…, 0400) and
again after restoring permissions. Ensure each is_writable probe runs against
refreshed filesystem metadata.

---

Nitpick comments:
In `@tests/Integration/Horizon/Feature/SupervisorCommandTest.php`:
- Around line 72-77: Remove the unused $factory assignment in
testSupervisorCommandCanSetProcessNiceness and register the new
FakeSupervisorFactory directly via app->instance while preserving the existing
supervisor command and priority assertion.
🪄 Autofix (Beta)

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: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 11edd843-72c2-4f38-9d4d-34461e873372

📥 Commits

Reviewing files that changed from the base of the PR and between 8dfe143 and 3950ecf.

📒 Files selected for processing (10)
  • .github/workflows/redis.yml
  • docs/plans/2026-07-12-0915-framework-coroutine-state-lifecycle-audit-ledger.md
  • docs/plans/2026-07-28-2043-horizon-cluster-lifecycle-and-current-parity.md
  • src/boost/docs/horizon.md
  • tests/Filesystem/FilesystemAdapterTest.php
  • tests/Fortify/Console/InstallCommandTest.php
  • tests/Horizon/Console/InstallCommandTest.php
  • tests/Integration/Horizon/Feature/ProcessInspectorTest.php
  • tests/Integration/Horizon/Feature/SupervisorCommandTest.php
  • tests/Telescope/Console/InstallCommandTest.php
🚧 Files skipped from review as they are similar to previous changes (5)
  • tests/Horizon/Console/InstallCommandTest.php
  • src/boost/docs/horizon.md
  • tests/Telescope/Console/InstallCommandTest.php
  • tests/Fortify/Console/InstallCommandTest.php
  • docs/plans/2026-07-12-0915-framework-coroutine-state-lifecycle-audit-ledger.md

Comment thread tests/Filesystem/FilesystemAdapterTest.php
Record the full Filesystem and Reverb groups in the Horizon plan now that both packages own findings completed by this work unit.

Remove the unused supervisor factory local from the native process-priority regression while preserving the container-owned fake and the asserted command behavior.
@binaryfire
binaryfire merged commit 53c09ec into 0.4 Jul 29, 2026
19 of 20 checks passed
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