From 576b58eff397e1b9642cded42b923ee72a2fe126 Mon Sep 17 00:00:00 2001 From: Snider Date: Sat, 8 Aug 2026 13:07:10 +0100 Subject: [PATCH 1/3] fix(cdn): stop the lower layer implementing the upper layer's interface MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit BunnyStorageService imported two types from dappcore/service — the HealthCheckable interface it implemented, and HealthCheckResult, which its healthCheck() body constructed. Neither exists in this package. php requires laravel/framework, pennant and livewire, and nothing else. So this package had production source that could not be loaded in this package. It worked only because every real consumer installs dappcore/service alongside it, which is why host.uk.com is fine and nobody has felt this. Adding the dependency was not available: dappcore/service requires dappcore/php: *, so it would close a loop. The direction composer already declares is service-depends-on-php, and php reaching up into Core\Service\* was against that grain. So the health report moves to the layer that is entitled to build it. WHAT MOVES, and it is a relocation rather than a removal: healthCheck() goes to a decorator in dappcore/service, which composes this service and implements HealthCheckable there. That repo has no session yet and the work is held; until it lands, health reporting of this service is absent with a named destination. WHAT STAYS: the zone probing, which never depended on Core\Service at all. checkZoneHealth() keeps returning its {success, latency_ms, error} array and isReachable() keeps answering the boolean question. checkZoneHealth() is now public, because the decorator composes rather than extends — a protected probe would have made the relocation cost exactly the latency and error detail that makes a health report worth reading. Safe to carry with no consumer noticing, checked rather than assumed: nothing in host.uk.com's app or vendor tree calls healthCheck() on this service, and there is no `instanceof HealthCheckable` anywhere in it. The only references were this file and documentation inside dappcore/service. ALSO HERE, and it is why the numbers move: CdnIntegrationTest built StorageUrlResolver with its arguments wrong — __construct(protected BunnyStorageService $bunnyStorage, ?CdnUrlBuilder $urlBuilder = null) new StorageUrlResolver($this->urlBuilder) passing the optional second argument into the required first slot and omitting the required one, so all 30 cases died in setUp on one TypeError. Thirty failures, one line. Fixed by resolving BunnyStorageService from the container, which is how Core\Cdn\Boot wires it in production. That fix was pointless on its own — corrected, the tests failed one line later on the missing interface — which is why both halves are in one change. ./vendor/bin/pest --testsuite=Feature,Unit 268 passed, 0 failed — unchanged ./vendor/bin/pest --testsuite=Module 448 -> 426 failed, 308 -> 330 passed CdnIntegrationTest alone 30 failed / 0 assertions -> 8 failed / 22 passed / 52 assertions vendor/bin/pint --test pass vendor/bin/phpstan analyse no errors The 8 that remain are real assertions against a constructible object rather than a constructor error: a missing config_resolved table in the test database, a singular/plural category path, a ?id= versus ?v= query parameter, and four straightforward expectation mismatches. Each is now a visible disagreement about behaviour instead of 30 identical crashes hiding one line. Co-Authored-By: Virgil --- src/Core/Cdn/Services/BunnyStorageService.php | 90 +++---------------- src/Core/Tests/Feature/CdnIntegrationTest.php | 20 ++++- 2 files changed, 31 insertions(+), 79 deletions(-) diff --git a/src/Core/Cdn/Services/BunnyStorageService.php b/src/Core/Cdn/Services/BunnyStorageService.php index 9447228..03741ef 100644 --- a/src/Core/Cdn/Services/BunnyStorageService.php +++ b/src/Core/Cdn/Services/BunnyStorageService.php @@ -14,8 +14,6 @@ use Bunny\Storage\Client; use Core\Config\ConfigService; use Core\Crypt\LthnHash; -use Core\Service\Contracts\HealthCheckable; -use Core\Service\HealthCheckResult; use Illuminate\Support\Facades\Log; use Illuminate\Support\Facades\Storage; @@ -27,9 +25,11 @@ * - Private zone: DRM/gated content * * Supports vBucket scoping for workspace-isolated CDN paths. - * Implements HealthCheckable for monitoring CDN connectivity. + * Zone reachability is exposed through checkZoneHealth() and isReachable(); + * turning that into a service health report belongs to dappcore/service, + * which is the package that depends on this one. */ -class BunnyStorageService implements HealthCheckable +class BunnyStorageService { protected ?Client $publicClient = null; @@ -555,84 +555,19 @@ public function vBucketList(string $domain, string $path = '', string $zone = 'p return $this->list($scopedPath, $zone); } - // ───────────────────────────────────────────────────────────────────────────── - // Health Check (implements HealthCheckable) - // ───────────────────────────────────────────────────────────────────────────── - - /** - * Perform a health check on the CDN storage zones. - * - * Tests connectivity by listing the root directory of configured storage zones. - * Returns a HealthCheckResult with status, latency, and zone information. - */ - public function healthCheck(): HealthCheckResult - { - $publicConfigured = $this->isConfigured('public'); - $privateConfigured = $this->isConfigured('private'); - - if (! $publicConfigured && ! $privateConfigured) { - return HealthCheckResult::unknown('No CDN storage zones configured'); - } - - $results = []; - $startTime = microtime(true); - $hasError = false; - $isDegraded = false; - - // Check public zone - if ($publicConfigured) { - $publicResult = $this->checkZoneHealth('public'); - $results['public'] = $publicResult; - if (! $publicResult['success']) { - $hasError = true; - } elseif ($publicResult['latency_ms'] > 1000) { - $isDegraded = true; - } - } - - // Check private zone - if ($privateConfigured) { - $privateResult = $this->checkZoneHealth('private'); - $results['private'] = $privateResult; - if (! $privateResult['success']) { - $hasError = true; - } elseif ($privateResult['latency_ms'] > 1000) { - $isDegraded = true; - } - } - - $totalLatency = (microtime(true) - $startTime) * 1000; - - if ($hasError) { - return HealthCheckResult::unhealthy( - 'One or more CDN storage zones are unreachable', - ['zones' => $results], - $totalLatency - ); - } - - if ($isDegraded) { - return HealthCheckResult::degraded( - 'CDN storage zones responding slowly', - ['zones' => $results], - $totalLatency - ); - } - - return HealthCheckResult::healthy( - 'All configured CDN storage zones operational', - ['zones' => $results], - $totalLatency - ); - } - /** * Check health of a specific storage zone. * + * Public because it is the seam the health-reporting decorator in + * dappcore/service consumes. That decorator composes this service rather + * than extending it, so a protected probe would be unreachable and the + * relocation would cost the detail — latency and the error string — that + * makes a health report worth reading. + * * @param string $zone 'public' or 'private' * @return array{success: bool, latency_ms: float, error?: string} */ - protected function checkZoneHealth(string $zone): array + public function checkZoneHealth(string $zone): array { $startTime = microtime(true); @@ -677,7 +612,8 @@ protected function checkZoneHealth(string $zone): array /** * Perform a quick connectivity check. * - * Simpler than healthCheck() - just returns true/false. + * A boolean answer, for callers that only need reachability. The detailed + * per-zone probe is checkZoneHealth(). * * @param string $zone 'public', 'private', or 'any' (default) */ diff --git a/src/Core/Tests/Feature/CdnIntegrationTest.php b/src/Core/Tests/Feature/CdnIntegrationTest.php index c07e3d6..b4abd07 100644 --- a/src/Core/Tests/Feature/CdnIntegrationTest.php +++ b/src/Core/Tests/Feature/CdnIntegrationTest.php @@ -12,6 +12,7 @@ namespace Core\Tests\Feature; use Core\Cdn\Services\AssetPipeline; +use Core\Cdn\Services\BunnyStorageService; use Core\Cdn\Services\CdnUrlBuilder; use Core\Cdn\Services\StorageUrlResolver; use Core\Tests\TestCase; @@ -67,9 +68,24 @@ protected function setUp(): void 'visibility' => 'private', ]); - // Initialize services + // Initialize services. + // + // StorageUrlResolver takes the storage service first and the URL builder + // second, and the builder is the optional one. This passed the builder + // into the first parameter and omitted the required argument entirely, + // so every test in this file died in setUp on a TypeError — 30 of them, + // reported as 30 failures rather than as the one line they are. + // + // BunnyStorageService comes from the container, which is how production + // gets it: Core\Cdn\Boot registers StorageUrlResolver as a singleton and + // lets Laravel autowire the chain. Resolving it here rather than + // hand-building BunnyStorageService(ConfigService) keeps the test on the + // same path the application uses. $this->urlBuilder = new CdnUrlBuilder(); - $this->urlResolver = new StorageUrlResolver($this->urlBuilder); + $this->urlResolver = new StorageUrlResolver( + app(BunnyStorageService::class), + $this->urlBuilder, + ); $this->assetPipeline = new AssetPipeline($this->urlResolver); } From 879da95bab067ee1dac84ac0de465d30b7e16935 Mon Sep 17 00:00:00 2001 From: Snider Date: Sat, 8 Aug 2026 13:09:35 +0100 Subject: [PATCH 2/3] chore(psalm): drop the suppression that existed for the dependency now removed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI's Psalm went red on this branch with UnusedIssueHandlerSuppression: Suppressed issue type "MissingDependency" for /src/Core/Cdn/ was not thrown which is the fix reporting itself. The suppression's own comment said what it was for — "Pending Core\Service module — referenced by Cdn BunnyStorageService" — so it was config-level acknowledgement of the upward dependency this branch removes, pending indefinitely. With the dependency gone there is nothing left to suppress, and Psalm treats a suppression that never fires as an error in its own right. Read from the CI job log rather than a local run, deliberately: local and CI Psalm disagree in this repository — the Laravel plugin resolves differently — and I got that wrong earlier today by trusting the copy in front of me. CI is the authority for what CI thinks is unused, and here CI is the one calling it unused. Co-Authored-By: Virgil --- psalm.xml | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/psalm.xml b/psalm.xml index 01ea359..bf113ac 100644 --- a/psalm.xml +++ b/psalm.xml @@ -64,22 +64,6 @@ - - - - - - - - - - - - - - - - From 1f29b44fe222bd877c11a8fc0158297d62e9665b Mon Sep 17 00:00:00 2001 From: Snider Date: Sat, 8 Aug 2026 13:10:23 +0100 Subject: [PATCH 3/3] fix(psalm): repair the config I broke, and remove only what should go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous commit's regex matched backwards from the wrong comment. Both blocks I meant to touch begin "Pending Core\Service module", and the pattern anchored on the first one — inside the UndefinedClass handler — then ran to the closing tag of the second, taking two unrelated referencedClass entries and a pair of closing tags with it. psalm.xml became invalid XML, and I pushed it. Restored and redone against the tags themselves rather than a shared comment prefix. What goes is exactly: - referencedClass Core\Service\Contracts\HealthCheckable - referencedClass Core\Service\HealthCheckResult - the MissingDependency handler suppressing src/Core/Cdn all three of which existed for the upward dependency this branch removes. What stays is what I should never have touched: the Core\Front\Client\Boot and Illuminate\Foundation\Auth\User entries, which have nothing to do with it. Fixed forward rather than rewritten — the broken commit is on the remote and stays there, with this on top. python3 -c "xml.etree.ElementTree.parse('psalm.xml')" valid ./vendor/bin/pest --testsuite=Feature,Unit 268 passed, 0 failed vendor/bin/pint --test pass Co-Authored-By: Virgil --- psalm.xml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/psalm.xml b/psalm.xml index bf113ac..1ec8788 100644 --- a/psalm.xml +++ b/psalm.xml @@ -64,6 +64,13 @@ + + + + + + +