diff --git a/marimo/_session/file_change_handler.py b/marimo/_session/file_change_handler.py index 91c04ff4371..096b2a958b0 100644 --- a/marimo/_session/file_change_handler.py +++ b/marimo/_session/file_change_handler.py @@ -213,13 +213,35 @@ def _handle_file_change_locked( error=f"Session path mismatch: {session.app_file_manager.path} != {file_path}", ) - # Check if the file content matches the last save - # to avoid reloading our own writes - if session.app_file_manager.file_content_matches_last_save(): - LOGGER.debug( - f"File {file_path} content matches last save, skipping reload" - ) - return FileChangeResult(handled=False) + # Read the file once and run all pre-reload skip checks against + # the same snapshot. If the read itself fails, fall through to + # `reload()` which has its own error handling below. + try: + current_content = session.app_file_manager.read_file() + except Exception as e: + LOGGER.debug(f"Error reading {file_path}: {e}") + current_content = None + + if current_content is not None: + # Skip our own writes. + if session.app_file_manager.content_matches_last_save( + current_content + ): + LOGGER.debug( + f"File {file_path} content matches last save, " + "skipping reload" + ) + return FileChangeResult(handled=False) + + # Skip when the file is mid-merge so the notebook isn't replaced + # with unparsable cells while the user resolves conflicts (e.g. + # via git-mediate). See issue #9613. + if _has_conflict_markers(current_content): + LOGGER.warning( + f"File {file_path} contains git conflict markers, " + "skipping reload until conflicts are resolved" + ) + return FileChangeResult(handled=False) # Reload the file manager to get the latest code. ``reload`` # mutates the existing document in place via ``apply()`` and @@ -242,6 +264,16 @@ def _handle_file_change_locked( ) +def _has_conflict_markers(content: str) -> bool: + """Return True if `content` contains a git conflict start marker. + + Git writes `<<<<<<<` at the start of a line to mark the beginning of + a conflict hunk; that's a strong signal the file is mid-merge and + shouldn't be reparsed as Python. + """ + return any(line.startswith("<<<<<<<") for line in content.splitlines()) + + def create_reload_strategy( mode: SessionMode, config_manager: MarimoConfigManager ) -> ReloadStrategy: diff --git a/marimo/_session/notebook/file_manager.py b/marimo/_session/notebook/file_manager.py index ac59ebc3e4f..212c453d230 100644 --- a/marimo/_session/notebook/file_manager.py +++ b/marimo/_session/notebook/file_manager.py @@ -616,20 +616,28 @@ def read_file(self) -> str: ) return self.storage.read(self._filename) + def content_matches_last_save(self, content: str) -> bool: + """Check if the given content matches the last save. + + Used to avoid reloading the file when we detect our own writes. + Prefer this over :meth:`file_content_matches_last_save` when the + caller already has the file contents in hand. + """ + if self._last_saved_content is None: + return False + return content.strip() == self._last_saved_content + def file_content_matches_last_save(self) -> bool: """Check if current file content matches the last save. Used to avoid reloading the file when we detect our own writes. - - Returns: - True if content matches last save, False otherwise """ if self._filename is None or self._last_saved_content is None: return False - try: - current_content = self.storage.read(self._filename) - return current_content.strip() == self._last_saved_content + return self.content_matches_last_save( + self.storage.read(self._filename) + ) except Exception as e: LOGGER.debug( f"Error reading file to check if content matches: {e}" diff --git a/tests/_session/test_file_change_handler.py b/tests/_session/test_file_change_handler.py index 1caec189462..211d2aac44e 100644 --- a/tests/_session/test_file_change_handler.py +++ b/tests/_session/test_file_change_handler.py @@ -9,6 +9,7 @@ import pytest +from marimo import _loggers from marimo._ast.cell import CellConfig from marimo._ast.cell_manager import CellManager from marimo._config.manager import get_default_config_manager @@ -744,6 +745,56 @@ def broken( assert result.error is None +async def test_file_change_coordinator_skips_conflict_markers( + tmp_path: Path, + mock_session: MagicMock, + caplog: pytest.LogCaptureFixture, +) -> None: + """When git writes conflict markers into the notebook, the watcher + must not reload — otherwise cells become unparsable and conflict + resolution tools like git-mediate break. See issue #9613.""" + test_file = tmp_path / "test.py" + test_file.write_text(SINGLE_CELL_NOTEBOOK) + mock_session.app_file_manager = AppFileManager(filename=str(test_file)) + + strategy = MagicMock() + coordinator = FileChangeCoordinator(strategy) + + test_file.write_text( + dedent( + """\ + import marimo + app = marimo.App() + + @app.cell + def cell1(): + <<<<<<< HEAD + x = 1 + ======= + x = 2 + >>>>>>> other-branch + return x + """ + ) + ) + + logger = _loggers.marimo_logger() + previous_propagate = logger.propagate + try: + logger.propagate = True + with caplog.at_level("WARNING"): + result = await coordinator.handle_change(test_file, mock_session) + finally: + logger.propagate = previous_propagate + + assert not result.handled + assert result.error is None + strategy.handle_reload.assert_not_called() + assert any( + "conflict markers" in record.message for record in caplog.records + ) + + async def test_file_change_coordinator_path_mismatch( tmp_path: Path, mock_session: MagicMock ) -> None: