Skip to content

Rebuild the package manifest when retrying provider loading - #265

Open
LukeTowers wants to merge 1 commit into
developfrom
fix/package-manifest-retry
Open

LukeTowers wants to merge 1 commit into
developfrom
fix/package-manifest-retry

Conversation

@LukeTowers

@LukeTowers LukeTowers commented Oct 22, 2026 •

Copy link
Copy Markdown
Member

registerConfiguredProviders() clears the package, service and class caches and retries once when provider loading fails — added in 6eef811 to recover from a cached provider that Composer has since removed, "emulating someone running php artisan config:clear before php artisan package:discover. If a failure still occurs, then the failure is made known to the user."

Neither happens. clearPackageCache() resets PackageManifest::$manifest to an empty array, but getManifest() short-circuits on ! is_null(), so the retry runs against an empty manifest instead of rebuilding it from the installed packages. With app.loadDiscoveredPackages enabled that drops every discovered provider and swallows the original exception, so a broken provider looks like a clean boot. Observed with one discovered provider that throws and one that doesn't:

manifest = [] (today) manifest = null
registerConfiguredProviders() returns, no exception throws provider blew up
manifest after the retry [] both providers

Tests

tests/Foundation/PackageCacheTest.php builds a base path with a real vendor/composer/installed.json plus a stale storage/framework/packages.php naming a provider that is no longer installed. All three fail without the change:

  • testClearingThePackageCacheRebuildsTheManifest — direct guard: after clearPackageCache() the manifest reports the installed package again, not nothing.
  • testProviderLoadingRetryRebuildsTheClearedPackageManifest — the stale provider fails the first load, and the retry registers the discovered one.
  • testProviderLoadingReportsAProviderThatIsStillMissingAfterTheRetry — a provider still missing after the rebuild reaches the caller instead of being swallowed.

The last two can only trigger the retry where discovered providers reach the provider repository, so on wip/1.3 they need #255 merged first; the first one passes on either branch.

Summary by CodeRabbit

  • Bug Fixes
    • Clearing the package cache now allows the package manifest to be rebuilt, helping the application discover installed packages and load their providers correctly.
    • If a provider is still unavailable after a retry, the application reports an error.

`clearPackageCache()` reset `PackageManifest::$manifest` to an empty array, but `getManifest()` short-circuits on `! is_null()`, so the retry ran against an empty manifest instead of rebuilding it from the installed packages. With `app.loadDiscoveredPackages` enabled that dropped every discovered provider, and swallowed the original exception along with it, so a genuinely broken provider looked like a clean boot.
@coderabbitai

coderabbitai Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: bff2b437-58a3-42e8-bdb3-8af9704b5f0a
📥 Commits

Reviewing files that changed from the base of the PR and between d6e5607 and 8779917.

📒 Files selected for processing: 2
  • src/Foundation/Application.php
  • tests/Foundation/PackageCacheTest.php

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

Walkthrough

clearPackageCache() now sets the package manifest to null after deleting cached files. New tests check manifest rebuilding, provider registration after a loading retry, and error propagation when a provider remains unavailable.

Estimated code review effort

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 87799

The change appears to restore provider discovery after cache clearing. No actionable issue remains before normal checks.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 55.56% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed 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 states the main change: the package manifest is rebuilt during a provider-loading retry. The raw summary says clearPackageCache() now sets the manifest to null instead of an empty array, and…
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.
Full details: Title check

Explanation

The title states the main change: the package manifest is rebuilt during a provider-loading retry. The raw summary says clearPackageCache() now sets the manifest to null instead of an empty array, and the new tests cover the rebuild-on-retry behavior. The objectives describe the same fix, where an empty manifest was treated as already loaded, so the retry did not rebuild it. The title is specific and accurate, and it is concise.

✨ Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

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.

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