Add browser-scoped request history browser - #23
Conversation
|
@Alistar84 At some point please rebase from master. I put a fix in for the warning emitted in the CI that makes this CI red. Thanks |
d26cba8 to
603fa1c
Compare
|
Rebased onto the latest master. Thanks for the fix! |
|
@Alistar84 Can we look in this one when you can? You will need to merge master. I am guessing the coverage dropped for this one and also I introduced new php-cs-fixer rules so run it when you can over here. |
Assisted-by: Codex
Assisted-by: Codex
Assisted-by: Codex
Harden session storage, request handling, and asynchronous history controls while preserving legacy entries and full test coverage. Assisted-by: Codex
Assisted-by: Codex
Assisted-by: Codex
603fa1c to
11b8040
Compare
|
Hi @niden, I've updated the PR following your comments about merging the latest
Co-authorship disclosure: these changes were co-authored with Codex (OpenAI), which assisted with implementation, tests, and review. |
|
@Alistar84 Thank you for this. I will check it out later tonight. It looks good at first glance but I have noticed a couple of things I need to check locally first before posting. More to come later on. |
niden
left a comment
There was a problem hiding this comment.
Some comments listed inline.
This is really good work and the main mechanics are in place. Have a look at the comments and share your thoughts on them.
Co-authored-by: Codex <codex@openai.com>
Co-authored-by: Codex <codex@openai.com>
|
Thanks for the detailed review. I have gone through all the comments and pushed the related changes. The main updates are the package-owned History endpoint, the History and filesystem contracts, versioned stored entries, PHP-driven widget metadata, independent collector settings, and the required storage path. Local checks are green: 297 PHP tests, 18 JavaScript tests, PHPStan, PHPCS, and PHP CS Fixer across all 164 files. Both new commits also include the requested Co-authored-by: Codex codex@openai.com trailer. |
|
@niden The requested changes and inline replies are now in place, and the CI is green. When you have a chance, could you please take another look? Thanks! |
@Alistar84 Thank you so much for all this work. First glance it looks great. I will go for rournd 2 later on today. |
niden
left a comment
There was a problem hiding this comment.
Some additional findings, none on the core functionality.
We will also need a revisit on the description and documentation.
Also, consider its usage. Is it on by default? If so, we need to document that.
Co-authored-by: Codex <codex@openai.com>
|
@niden I pushed The main changes are: validation now happens after the runtime gates, I refreshed the PR description/docs and added regression coverage for each of those cases. Local checks are green: 305 PHP tests (902 assertions), 18 JavaScript tests, PHPCS, PHP CS Fixer, and targeted PHPStan on the changed PHP files. Ready for another look when you have time - thanks! |
Co-authored-by: Codex <codex@openai.com>
|
Small follow-up in |
|
@Alistar84 once more thank you for this excellent work. Since we are close to the completion of this feature, I wanted to really see how this looks like. So I pulled down your branch on my local repo. Then I fired up the Vokuro application, which has the debugbar enabled by default. I copied the code from your branch in the With that and with clicking around plus an additional AI review we have a few things to address. This is how the bar shows with history on:
As you can see we have some inconsistencies there with time/memory and it is a bit tall. I took the liberty of changing a few things (diff in a follow up comment for your convenience), and it should look like this
Note that in the second picture the history does not show (I disabled it) but it will appear next to the memory. This shortens the height of the bar and removes the duplicate time/memory, thus allowing for a less cluttered UI. The findings are in the next comment |
1. History breaks app routes (boot-time service lookup)
With History on, Resolve at request time.
NOTE: Request does the same thing but has not been observed until now, since it is more common to use URI than replace Request. That will be a follow up PR - not in scope now. 2. The endpoint does not start the sessionMost Phalcon apps start the session lazily, when code resolves the The page was stored in its session directory, but The bar uses its own cookie, not the PHP session.
3. An absolute base URI breaks the endpointWith an absolute base URI (for example The JS gets Covered by item 1 (use only the path of the base URI), plus validation.
4. Dispatcher ACL plugins can block the endpointThe endpoint runs through the app's normal dispatch loop. Plugins on A dispatcher listener saw Documentation plus a public constant.
5. Indicators depend on HistoryTime and Memory show as a tab and as an indicator. The right side (two indicators + request control up to Diff for the change in the next comment
6. CHANGELOG line for the "Logs" tab name7. Memory/Time are on by defaultThey should stay on by default and we should just document this 8. No global limit on disk use
Storage strategy:
9. A new session ID hides earlier entriesThe storage key was Covered by 2 10. Dead
|
|
Diff as promised:
Diff against the PR branch (
|
Co-authored-by: Codex <codex@openai.com>
|
Hey @niden, thanks again for the really thorough review and for sharing the UI diff — it was very helpful. I’ve pushed the changes in Here’s the rundown:
I also tested the integration in a real Phalcon app: the cookie and stored requests survive logout/login, while anonymous access remains governed by the app ACL. Final checks are green: 318 PHP tests / 955 assertions, 19 JS tests, 100% coverage, PHPStan, PHPCS and PHP CS Fixer. The commit also keeps the Codex co-author trailer. Whenever you have a chance, it should be ready for another look. Thanks! |
|
@Alistar84 Thank you again for this. I will review it more thoroughly later on today. Two things that popped up from a quick scan - we need to update the PR description with all the latest changes (esp. with the cookie changes). Also the changelog needs a couple of pieces to be moved to the Added instead of changed. I need to test this with Vokuro just to be sure on the visual side but looking really good otherwise. More a bit later. |
|
@Alistar84 two more issues to address.
The session from that first page is lost. For example, in Vokuro, the first login submit in a new browser fails the CSRF check, and flash messages disappear. This comes from reading the Phalcon and PHP source. PHP's own ext/session comment warns about this exact case. I could not reproduce it at runtime, because PHP CLI does not record headers. The unit tests only check the raw header string, so they cannot see this. Options: (Do not use the app's cookies service. It can encrypt cookies and needs a crypt service, which couples the bar to the app's setup.)
The cookie has no Secure flag on HTTPS. It also has no Max-Age, so it is a session cookie. History "disappears" when the browser restarts, but the files stay on disk until the TTL ends. Options: |
|
Thanks @niden, good catch - and I’m sorry I missed the interaction with Phalcon’s header replacement behavior. My unit tests only inspected the queued raw header, so they did not exercise what happens when the response headers are actually sent. I agree that the current setRawHeader() approach can replace previously queued Set-Cookie headers. My preference is option (i): use native setcookie() behind an injectable callable, so it appends the debug bar cookie independently of Phalcon’s response headers and remains testable without coupling the debug bar to the application cookies service. For the attributes, I would set Secure from RequestInterface::isSecure() and keep it as a browser-session cookie, documenting that the associated files may remain until ttl_seconds after that browser identity expires. This avoids introducing cookie renewal and expiry semantics in this PR. Would this approach work for you, or would you prefer the cookie expiry to match ttl_seconds? |
|
@Alistar84 this is your baby. I am fine with either option. |
Co-authored-by: Codex <codex@openai.com>
Co-authored-by: Codex <codex@openai.com>
|
@Alistar84 Have a look below
( Options:
I have not verified the above but it might worth a look
This is similar to the one we found in round 2 I think.
They are all pretty small fixes. The first one is the most worrying one. Again, this is a debugbar intended in a local setup but you never know who will decide to push this to production with no guards on it.... |
Co-authored-by: OpenAI Codex <codex@openai.com>
|
Thanks @niden, I addressed all the points in
I also updated the documentation and PR description and added regression tests for forged/rotated identities, concurrent creation, failed eviction, CommonJS host isolation, Windows glob paths, storage validation, empty-directory cleanup, and renamed request collectors. All CI checks are green, including Code quality, Coverage, and the complete Phalcon 5/6 matrix. |
|
@Alistar84 Thank you once more for this work. Exemplary. This will make a great addition to this component. There are a few issues that need attending but those I added them in a different issue for a follow up #32 For now this is merged. Thank you again. |


Hello!
In raising this pull request, I confirm the following:
Small description of change:
This PR adds an optional, browser-scoped request history system to the debug bar and integrates it with compact request metrics in the bottom bar.
It resolves #21, which describes the current limitation where the bar can only inspect the request that rendered the current page. Completed AJAX requests, redirects, and earlier requests can now be inspected without navigating away from or reloading the host page.
Closes #21.
History browser
When history is enabled, each stored entry contains the collected debug-bar payload and request metadata such as the HTTP method, URI, response status, AJAX flag, and request and persistence timestamps.
The rightmost request control combines a search icon with the HTTP method and URI. It identifies the current request when the page first loads and updates when a stored request is selected. Clicking it closes any open collector panel and opens the history browser. Selecting a request replaces the complete bar payload and reopens the collector panel that was active before History.
Time and Memory are enabled by default and render once as compact, clickable indicators. Each collector owns its indicator metadata, and the indicators remain available when History is disabled.
Browser identity, storage, and retention
History is disabled by default and requires an explicit absolute writable storage path. The provider creates the directory when needed and validates its writability during boot. History can also be disabled through
collectors.history.History uses a dedicated random
phalcon-debugbar-historycookie and never reads, starts, or changes the application's PHP session. The cookie is HttpOnly, SameSite=Lax, marked Secure on HTTPS, scoped to the application's base path, and lasts for the browser session. It is appended through PHP's native cookie mechanism so it does not replace cookies queued by the application.The first allowed response sets the cookie but is not stored. Storage starts with the next request carrying that cookie, so clients that do not retain cookies create no history directories. Closing the browser discards its identity; associated files remain eligible for cleanup until
history.ttl_secondsexpires.Stored requests are isolated by a SHA-256 hash of the browser identity, written atomically, limited per browser by
history.max_requests, and removed after the configured TTL. The total browser-directory count is bounded byhistory.max_browsers(default 10). Creation is serialized with a filesystem lock and evicts the least recently updated browser directory before admitting another identity, bounding forged or repeatedly rotated cookie identities. Versioned payloads and metadata sidecars keep listings independent of collector payload size. Rate-limited garbage collection removes expired entries, abandoned temporary files, and empty browser directories.Internal endpoint
The provider exposes its package-owned controller at:
GET /_debugbar/open- list stored request metadataGET /_debugbar/open?id=<request-id>- load a stored debug-bar payloadDELETE /_debugbar/open- clear the current browser's historyThe endpoint resolves the final application base URI at request time without adding routes or resolving shared responses during boot. It reuses the debug bar access gate, returns private non-cacheable JSON responses with
X-Content-Type-Options: nosniff, validates request IDs, accepts only the actual transport method, and is excluded from debug-bar collection, headers, injection, and storage. Applications with dispatcher ACL plugins can allow the publicHistoryEndpoint::CONTROLLER_NAMESPACE.Backward compatibility
The feature is opt-in. Existing applications retain the current behavior when History is disabled. Time and Memory remain enabled by default and can be disabled through the collector configuration.
Tests
The PR includes PHP and JavaScript tests covering browser-cookie isolation, cookie attributes and access gating, Phalcon 5 and 6 endpoint dispatch, storage failure paths, concurrent directory creation, global browser eviction, retention, Windows glob paths, version compatibility, stale asynchronous responses, CommonJS host isolation, metadata-driven request indicators, and stored-request selection. The PHP suite has 100% statement and method coverage.
Thanks