Repository navigation
fix(diagnostics): look on disk before reporting a package not installed - #13
Conversation
Composer\\InstalledVersions answers 'is this autoloaded in this request', not 'is this installed'. The difference is invisible for DebugBar's own dependencies and decisive for filp/whoops, which lives in xwhoops's vendor and is required lazily -- so on any request where xwhoops stood down, this page reported 'Whoops library: Not installed' on a site where it was installed and working, sending the administrator to composer for a problem composer cannot fix. packageRow() now takes an optional dirname => package map and checks the module vendor before saying no. Also records in AccessPolicy that XOOPS 2.7.3 widened the developer gate to any non-zero debug_mode, closing one of the two axes on which isRuntimeAllowed() diverges; the group-membership axis stays open and is not a defect.
Reviewer's GuideDiagnostics now check module-specific vendor directories on disk before reporting packages as not installed, and access policy documentation is updated to reflect XOOPS 2.7.3’s widened debug_mode gate while clarifying the intended divergence between toolbar and diagnostics access checks. Sequence diagram for updated diagnostics packageRow fallback to module vendorsequenceDiagram
participant SystemDiagnostics
participant ComposerInstalledVersions
participant Filesystem
SystemDiagnostics->>SystemDiagnostics: collect(xoopsConfig)
SystemDiagnostics->>SystemDiagnostics: packageRow(whoops,[filp/whoops],false,{xwhoops:filp/whoops})
loop check_packages
SystemDiagnostics->>SystemDiagnostics: packageInstalled(package)
SystemDiagnostics->>ComposerInstalledVersions: packageInstalled(package)
ComposerInstalledVersions-->>SystemDiagnostics: installed/not_installed
end
alt any_package_installed
SystemDiagnostics->>SystemDiagnostics: row(id,Installed,info,detail)
else no_package_installed
loop check_moduleVendors
SystemDiagnostics->>Filesystem: is_dir(rootPath/modules/xwhoops/vendor/filp/whoops)
Filesystem-->>SystemDiagnostics: exists/does_not_exist
end
alt module_vendor_found
SystemDiagnostics->>SystemDiagnostics: row(whoops,Installed,info,detail_with_not_loaded_note)
else none_found
SystemDiagnostics->>SystemDiagnostics: row(whoops,Not installed,info)
end
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Warning Review limit reached
Next review available in: 2 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the 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 configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe change clarifies ChangesAccess policy documentation
Package diagnostics fallback
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The new
packageRowsignature is getting a bit parameter-heavy; consider introducing a small value object or dedicated options array to clarify the meaning and ordering ofbool $activevs$moduleVendors. - The logic for checking module-specific vendor directories in
packageRowduplicates path construction; extracting a small helper for buildingmodules/<dirname>/vendor/<package>paths would improve readability and reduce future maintenance overhead.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `packageRow` signature is getting a bit parameter-heavy; consider introducing a small value object or dedicated options array to clarify the meaning and ordering of `bool $active` vs `$moduleVendors`.
- The logic for checking module-specific vendor directories in `packageRow` duplicates path construction; extracting a small helper for building `modules/<dirname>/vendor/<package>` paths would improve readability and reduce future maintenance overhead.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Greptile SummaryThe PR improves diagnostics for lazily loaded Whoops installations and documents the updated XOOPS debug-mode access behavior.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| class/Analysis/SystemDiagnostics.php | Adds an on-disk fallback for detecting filp/whoops when its Composer autoloader is not registered. |
| class/Admin/AccessPolicy.php | Updates documentation describing debug-mode and administrator-gate behavior. |
Reviews (2): Last reviewed commit: "Fixes" | Re-trigger Greptile
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@class/Analysis/SystemDiagnostics.php`:
- Around line 116-120: Update the documentation around $moduleVendors to
describe the fallback as detecting packages present on disk but absent from
current Composer metadata, based on is_dir($path); remove wording that
characterizes Composer\InstalledVersions or the fallback in terms of whether
classes were autoloaded in the request.
🪄 Autofix
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: 4f1ad562-055b-4c42-a90c-b99db278d4d2
📒 Files selected for processing (2)
class/Admin/AccessPolicy.phpclass/Analysis/SystemDiagnostics.php
|
@coderabbitai review |
|
Composer\InstalledVersions answers 'is this autoloaded in this request', not 'is this installed'. The difference is invisible for DebugBar's own dependencies and decisive for filp/whoops, which lives in xwhoops's vendor and is required lazily -- so on any request where xwhoops stood down, this page reported 'Whoops library: Not installed' on a site where it was installed and working, sending the administrator to composer for a problem composer cannot fix. packageRow() now takes an optional dirname => package map and checks the module vendor before saying no. Also records in AccessPolicy that XOOPS 2.7.3 widened the developer gate to any non-zero debug_mode, closing one of the two axes on which isRuntimeAllowed() diverges; the group-membership axis stays open and is not a defect.
Summary by Sourcery
Improve system diagnostics and access policy behavior around lazily loaded debug packages and XOOPS debug modes.
New Features:
Bug Fixes:
Enhancements:
Summary by CodeRabbit
Bug Fixes
Documentation