fix: handle with_dynamic_directory mounted at sub-path (#8322)#8434
fix: handle with_dynamic_directory mounted at sub-path (#8322)#8434
with_dynamic_directory mounted at sub-path (#8322)#8434Conversation
When mounting a marimo ASGI app at a sub-path (e.g.,
`app.mount("/server2", server.build())`), the `DynamicDirectoryMiddleware`
failed to match requests because Starlette's `Mount` keeps the mount prefix
in `scope["path"]` while also setting `scope["root_path"]`.
For example, with `with_dynamic_directory(path="/apps", ...)` mounted at
`/server2`, a request to `/server2/apps/notebook/` would have
`path="/server2/apps/notebook/"` and `root_path="/server2"`. The middleware
checked `path.startswith("/apps/")` which failed because of the `/server2`
prefix.
Additionally, the `app_builder` was called with the filesystem path as
`base_url` instead of a proper URL path.
## Changes
- Strip `root_path` from `scope["path"]` before matching `base_path`, so
the middleware works regardless of where it's mounted
- Keep fallback for frameworks that fully strip the path (not just prefix)
- Compute proper URL-based `base_url` for sub-apps instead of filesystem
paths
- Update `my_server.py` smoke test with two mounted servers (root +
`/server2`)
Closes #8322
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
Fixes DynamicDirectoryMiddleware routing when a marimo ASGI app is mounted under a sub-path by accounting for scope["root_path"], and corrects the base_url passed into dynamically-built notebook apps so it’s a URL path (not a filesystem path).
Changes:
- Strip
root_pathfromscope["path"]to correctly matchbase_pathunder Starlette/FastAPI mounts. - Compute and pass a URL-path
base_urltoapp_builderfor dynamic directory apps. - Add regression tests covering sub-mount behavior (HTTP, redirects, assets, websockets) and update the custom smoke-test server example.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
marimo/_server/asgi.py |
Adjusts dynamic-directory request matching for mounted apps and fixes base_url computation for sub-app creation. |
tests/_server/test_asgi.py |
Adds regression coverage for dynamic directories when the ASGI app is mounted at a sub-path. |
marimo/_smoke_tests/custom_server/my_server.py |
Expands the manual smoke-test script to include root mount + sub-path mount scenarios. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| cache_key = str(marimo_file) | ||
| if cache_key not in self._app_cache: | ||
| LOGGER.debug(f"Creating new app for {cache_key}") | ||
| # Compute the URL base path for this notebook. | ||
| # This is used for template rendering (e.g., OpenGraph URLs). | ||
| try: | ||
| relative_notebook = str( | ||
| marimo_file.relative_to(self.directory) | ||
| ) | ||
| if relative_notebook.endswith(".py"): | ||
| relative_notebook = relative_notebook[:-3] | ||
| # Compute the URL prefix for this notebook. When | ||
| # root_path already ends with base_path (because the | ||
| # parent mount includes it), avoid doubling the prefix. | ||
| if root_path.endswith(self.base_path): | ||
| url_prefix = root_path |
There was a problem hiding this comment.
relative_notebook = str(marimo_file.relative_to(self.directory)) will use OS-specific path separators (e.g., backslashes on Windows), which can leak into notebook_base_url and produce invalid URL paths. Use a POSIX/URL-safe form (e.g., Path(...).as_posix() or replace os.sep with "/") before concatenating into a URL.
tests/_server/test_asgi.py
Outdated
| the parent framework strips the prefix from scope["path"] and adds it to | ||
| scope["root_path"], causing the middleware to never match. |
There was a problem hiding this comment.
The class docstring says the parent mount "strips the prefix from scope['path']", but the issue description for #8322 (and this test setup) is about frameworks keeping the mount prefix in scope['path'] while also setting scope['root_path']. Please update the docstring to reflect the actual behavior being tested to avoid confusion for future maintainers.
| the parent framework strips the prefix from scope["path"] and adds it to | |
| scope["root_path"], causing the middleware to never match. | |
| the parent framework keeps the mount prefix in scope["path"] while also | |
| setting scope["root_path"], causing the middleware's path matching logic | |
| to not behave as originally expected. |
| # Key test: dynamic directory at "/" relative to mount = "/server2/" absolute | ||
| server2 = ( | ||
| marimo.create_asgi_app() | ||
| # Dynamic directory at sub-path: /apps becomes /server2/apps in FastAPI | ||
| .with_dynamic_directory( | ||
| path="/apps", directory=str(dirname / "altair_examples") |
There was a problem hiding this comment.
dirname / "smoke_tests" does not exist in marimo/_smoke_tests (the smoke test notebooks live directly under dirname). As written, with_dynamic_directory(..., directory=str(dirname / "smoke_tests")) will fail at runtime. Point this at an existing directory (likely str(dirname)) or add the missing folder to the repo.
| # Dynamic directory at sub-path: /apps becomes /server2/apps in FastAPI | ||
| .with_dynamic_directory( | ||
| path="/apps", directory=str(dirname / "altair_examples") |
There was a problem hiding this comment.
with_dynamic_directory(path="/", ...) is likely not reachable: DynamicDirectoryMiddleware normalizes base_path with rstrip("/"), turning "/" into an empty string, and the request-matching logic is guarded by if self.base_path .... Use path="" for a root mount (consistent with with_app(path="", ...)) or adjust the middleware to explicitly treat an empty base_path as root.
fix: handle
with_dynamic_directorymounted at sub-pathFixes #8322
When mounting a marimo ASGI app at a sub-path (e.g.,
app.mount("/server2", server.build())), theDynamicDirectoryMiddlewarefailed to match requests because Starlette'sMountkeeps the mount prefix inscope["path"]while also settingscope["root_path"].For example, with
with_dynamic_directory(path="/apps", ...)mounted at/server2, a request to/server2/apps/notebook/would havepath="/server2/apps/notebook/"androot_path="/server2". The middlewarechecked
path.startswith("/apps/")which failed because of the/server2prefix.Additionally, the
app_builderwas called with the filesystem path asbase_urlinstead of a proper URL path.Changes
root_pathfromscope["path"]before matchingbase_path, sothe middleware works regardless of where it's mounted
base_urlfor sub-apps instead of filesystempaths