diff --git a/marimo/_islands/_island_generator.py b/marimo/_islands/_island_generator.py index d6bc6de86c5..0938803f0b4 100644 --- a/marimo/_islands/_island_generator.py +++ b/marimo/_islands/_island_generator.py @@ -3,6 +3,7 @@ import asyncio import json +import os import sys from textwrap import dedent from typing import TYPE_CHECKING, cast @@ -249,6 +250,10 @@ def __init__(self, app_id: str = "main"): self._app = InternalApp(App()) self._stubs: list[MarimoIslandStub] = [] self._config = _AppConfig() + # When constructed via ``from_file``, this records the notebook + # source path so cells see ``__file__`` / ``mo.notebook_dir()`` + # resolve to the notebook rather than to the host process. + self._source_filename: str | None = None @staticmethod def from_file( @@ -267,6 +272,10 @@ def from_file( file_manager = load_notebook(filename) generator = MarimoIslandGenerator() + # Resolve at capture time so a chdir between ``from_file`` and + # ``build`` doesn't change which absolute path cells see for + # ``__file__`` / ``mo.notebook_dir()``. + generator._source_filename = os.path.abspath(filename) stubs = [] for cell_data in file_manager.app.cell_manager.cell_data(): stubs.append( @@ -338,7 +347,9 @@ async def build(self) -> App: raise ValueError("You can only call build() once") (session, did_error) = await run_app_until_completion( - file_manager=AppFileManager.from_app(self._app), + file_manager=AppFileManager.from_app( + self._app, filename=self._source_filename + ), cli_args={}, argv=None, ) diff --git a/marimo/_session/notebook/file_manager.py b/marimo/_session/notebook/file_manager.py index edbe1455c8e..ac59ebc3e4f 100644 --- a/marimo/_session/notebook/file_manager.py +++ b/marimo/_session/notebook/file_manager.py @@ -105,16 +105,28 @@ def filename(self, value: str | Path | None) -> None: self._filename = _maybe_path(value) @staticmethod - def from_app(app: InternalApp) -> AppFileManager: + def from_app( + app: InternalApp, + filename: str | Path | None = None, + ) -> AppFileManager: """Create AppFileManager from an existing InternalApp. Args: app: The internal app to wrap + filename: Optional source path for the notebook. When set, + ``AppMetadata.filename`` (and therefore ``__file__`` / + ``mo.notebook_dir()`` inside cells) resolves to the source + file rather than the host process's ``__main__``. Returns: AppFileManager instance """ manager = AppFileManager(None) + # Snapshot to an absolute path at assignment time so a later + # ``chdir`` cannot change which file ``self.path`` resolves to. + manager.filename = ( + os.path.abspath(str(filename)) if filename is not None else None + ) manager.app = app return manager diff --git a/tests/_islands/test_island_generator.py b/tests/_islands/test_island_generator.py index 1a31193bca5..cea31016de0 100644 --- a/tests/_islands/test_island_generator.py +++ b/tests/_islands/test_island_generator.py @@ -151,6 +151,98 @@ def __(mo): assert "The answer is 42" in stub2.output.data +async def test_from_file_propagates_filename_to_cells(tmp_path: Path): + # Regression test for #9391: cells rendered through + # ``MarimoIslandGenerator.from_file()`` must see ``__file__`` and + # ``mo.notebook_dir()`` resolve to the notebook source, not to the + # host process's ``__main__`` (which under installed CLIs is a + # launcher shim). + marimo_file = tmp_path / "nb.py" + marimo_file.write_text( + """ +import marimo + +app = marimo.App() + +@app.cell +def __(): + import marimo as mo + return (mo,) + +@app.cell +def __(mo): + mo.md(f"FILE={__file__} | DIR={mo.notebook_dir()}") + return () + """ + ) + + generator = MarimoIslandGenerator.from_file(str(marimo_file)) + await generator.build() + + captured = generator._stubs[1].output + assert captured is not None + data = captured.data + assert str(marimo_file) in data, ( + f"expected __file__ to resolve to {marimo_file}, got: {data}" + ) + assert str(marimo_file.parent) in data, ( + f"expected notebook_dir() to resolve to {marimo_file.parent}, got: {data}" + ) + + +async def test_from_file_resolves_relative_path_at_capture_time( + tmp_path: Path, +) -> None: + # Companion to ``test_from_file_propagates_filename_to_cells``: + # passing a relative path to ``from_file`` should snapshot the + # absolute path immediately, so a ``chdir`` between ``from_file`` + # and ``build`` cannot change which file cells see. + import os + + nb_dir = tmp_path / "notebooks" + nb_dir.mkdir() + marimo_file = nb_dir / "nb.py" + marimo_file.write_text( + """ +import marimo + +app = marimo.App() + +@app.cell +def __(): + import marimo as mo + return (mo,) + +@app.cell +def __(mo): + mo.md(f"FILE={__file__}") + return () + """ + ) + + other_dir = tmp_path / "elsewhere" + other_dir.mkdir() + + original_cwd = os.getcwd() + try: + os.chdir(nb_dir) + generator = MarimoIslandGenerator.from_file("nb.py") + # Move to an unrelated directory before ``build`` runs. + os.chdir(other_dir) + await generator.build() + finally: + os.chdir(original_cwd) + + captured = generator._stubs[1].output + assert captured is not None + data = captured.data + assert str(marimo_file) in data, ( + f"expected __file__ to resolve to {marimo_file}, got: {data}" + ) + # And the path shouldn't accidentally resolve under ``other_dir``. + assert str(other_dir / "nb.py") not in data + + def test_render_head(): generator = MarimoIslandGenerator() head_html = generator.render_head() diff --git a/tests/_server/test_file_manager_filename.py b/tests/_server/test_file_manager_filename.py index c2ba0b57586..9e282d97e6f 100644 --- a/tests/_server/test_file_manager_filename.py +++ b/tests/_server/test_file_manager_filename.py @@ -59,3 +59,29 @@ def test_app_file_manager_rename_collision_raises_error( file_manager.rename(str(target_path)) assert "already exists" in str(exc_info.value.detail) + + def test_from_app_snapshots_relative_filename_to_absolute( + self, tmp_path: Path + ) -> None: + # ``AppFileManager.from_app(..., filename=...)`` must snapshot a + # relative ``filename`` to an absolute path at call time so a + # later ``chdir`` cannot shift which file ``self.path`` resolves + # to (which is what ``AppMetadata.filename`` / ``__file__`` are + # built from). + import os + + from marimo._ast.app import App, InternalApp + + nb_dir = tmp_path / "notebooks" + nb_dir.mkdir() + original_cwd = os.getcwd() + try: + os.chdir(nb_dir) + manager = AppFileManager.from_app( + InternalApp(App()), filename="nb.py" + ) + expected = str(nb_dir / "nb.py") + os.chdir(tmp_path) + assert manager.path == expected + finally: + os.chdir(original_cwd)