fix(application): anchor opener route regex for tree paths#7903
fix(application): anchor opener route regex for tree paths#7903The-Peacemaker wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes incorrect route matching in the application extension by anchoring the opener route regex so /tree/.../notebooks/... paths are no longer treated as notebook/editor routes, and adds a UI regression test to cover the scenario from #7216.
Changes:
- Anchor the opener route regex to only match full
/notebooks/...and/edit/...paths. - Add a Playwright/Galata UI test to ensure reloading a tree URL containing a directory named
notebooksdoes not surface a “File Load Error”.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/application-extension/src/index.ts | Anchors the route regex used by the opener router handler to prevent substring matches inside /tree/... paths. |
| ui-tests/test/tree.spec.ts | Adds a regression test for navigating to and reloading a tree path that includes a literal notebooks directory. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| await page.goto(`tree/${nestedPath}`); | ||
| await page.waitForSelector('.jp-FileBrowser-crumbs >> text=/test/'); | ||
|
|
||
| await page.reload(); |
| await page.waitForSelector('.jp-FileBrowser-crumbs >> text=/test/'); | ||
|
|
||
| await page.reload(); | ||
| await page.waitForSelector('.jp-FileBrowser-crumbs >> text=/test/'); |
|
I reproduced the existing bug locally: Navigating to http://localhost:8889/tree/test/notebooks/test shows "File Load Error for test" -> the app tries to open test as a notebook file instead of showing the directory listing. The issue is the unanchored regex: const TREE_PATTERN = new RegExp('/(notebooks|edit)/(.*)'); This matches /(notebooks|edit)/ anywhere in the URL. So /tree/test/notebooks/test matches the substring /notebooks/test in the middle, and the router thinks you're trying to open a file called test. The fix anchors it with ^ so only URLs that actually start with /notebooks/ or /edit/ are matched — tree paths with a directory named notebooks are no longer intercepted. The unit-test also looks like it reproduces to catch the same issue. LGTM (UI test failure looks unrelated). |
Avoid false matches when a tree URL contains a directory named 'notebooks'. Adds a UI regression test for nested tree paths containing /notebooks/ and ensures tests wait for breadcrumb navigation to avoid networkidle timeouts in Chromium.
746d015 to
b092230
Compare
References
Fixes #7216
Code changes
/(notebooks|edit)/(.*)^/(notebooks|edit)/(.*)$ui-tests/test/tree.spec.tscovering tree navigation/reload when the path contains a directory literally namednotebooks.Problem / root cause
Tree URLs such as
/tree/test/notebooks/testwere being incorrectly matched by the opener route regex because it was not anchored. That caused the router to treat part of a tree URL as a notebook route and attempt to open a file, leading toFile Load Error: Invalid response: 404 Not Found.Solution
Use an anchored regex so the opener only handles paths that begin with
/notebooks/or/edit/.User-facing changes
Users can navigate and refresh tree paths containing directories named
notebookswithout spurious file load errors.Backwards-incompatible changes
None.