Skip to content

fix(watch): skip reload when notebook has git conflict markers#9626

Merged
mscolnick merged 4 commits into
mainfrom
ms/fix/issue-9613
May 21, 2026
Merged

fix(watch): skip reload when notebook has git conflict markers#9626
mscolnick merged 4 commits into
mainfrom
ms/fix/issue-9613

Conversation

@mscolnick
Copy link
Copy Markdown
Contributor

Summary

When marimo edit --watch is running and a git operation (cherry-pick,
git stash pop, merge) writes conflict markers into the notebook file,
the watcher parses the file and the conflict markers become an
app._unparsable_cell. This clobbers the on-disk conflict markers
through the round-trip and breaks conflict-resolution tools like
git-mediate.

The fix: before reloading, scan the file for lines starting with
<<<<<<< (git's unambiguous conflict start marker). If present, log a
warning and skip the reload until the user resolves the conflicts.

Fixes #9613.

## Summary

When `marimo edit --watch` is running and a git operation (cherry-pick,
`git stash pop`, merge) writes conflict markers into the notebook file,
the watcher parses the file and the conflict markers become an
`app._unparsable_cell`. This clobbers the on-disk conflict markers
through the round-trip and breaks conflict-resolution tools like
`git-mediate`.

The fix: before reloading, scan the file for lines starting with
`<<<<<<<` (git's unambiguous conflict start marker). If present, log a
warning and skip the reload until the user resolves the conflicts.

Fixes #9613.
Copilot AI review requested due to automatic review settings May 20, 2026 14:38
@vercel
Copy link
Copy Markdown

vercel Bot commented May 20, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
marimo-docs Ready Ready Preview, Comment May 21, 2026 6:41pm

Request Review

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Prevents marimo edit --watch from clobbering on-disk git conflict markers by detecting merge-conflict start markers in the notebook file and skipping reloads until conflicts are resolved (fixes #9613).

Changes:

  • Add a pre-reload scan for git conflict markers (<<<<<<< at start of a line) and skip reload with a warning when present.
  • Refactor “skip our own writes” logic to reuse a single file read via a new content_matches_last_save() helper.
  • Add a regression test ensuring reload is skipped and a warning is emitted when conflict markers exist.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
tests/_session/test_file_change_handler.py Adds a regression test asserting watch reload is skipped when conflict markers are present.
marimo/_session/notebook/file_manager.py Introduces content_matches_last_save() and reuses it from file_content_matches_last_save().
marimo/_session/file_change_handler.py Reads file once for pre-reload checks; skips reload on conflict markers and on own writes.

Comment thread tests/_session/test_file_change_handler.py Outdated
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 3 files

Architecture diagram
sequenceDiagram
    participant FS as File System
    participant Watch as File Watcher
    participant Handler as FileChangeHandler
    participant FM as AppFileManager
    participant Session as Session

    Note over FS,Session: File change detection and reload flow

    FS->>Watch: File modified event
    Watch->>Handler: handle_change(file_path, session)
    
    Handler->>FM: read_file()
    FM-->>Handler: current_content (str)
    
    alt Content matches last save (our own write)
        Handler->>Handler: Return FileChangeResult(handled=False)
        Note over Handler: Skip reload to avoid echo loop
    else Content has git conflict markers (<<<<<<<)
        Handler->>Handler: Log warning
        Handler->>Handler: Return FileChangeResult(handled=False)
        Note over Handler: NEW: Skip reload to preserve conflict markers
    else Normal content change
        Handler->>FM: reload() with current_content
        FM->>FM: Parse notebook, apply changes
        alt Parsable notebook
            FM-->>Handler: Success
            Handler->>Session: Apply cell changes
            Session-->>Handler: OK
            Handler-->>Watch: FileChangeResult(handled=True)
        else Unparsable notebook
            FM->>FM: Create _unparsable_cell
            FM-->>Handler: Partial/error
            Handler-->>Watch: FileChangeResult with error
        end
    end

    Note over FS,Handler: Error handling path
    alt read_file() fails
        Handler->>Handler: Log debug, set current_content=None
        Handler->>Handler: Fall through to reload()
        Note over Handler: reload() has its own error handling
    end
Loading

Re-trigger cubic

@mscolnick mscolnick added the bug Something isn't working label May 20, 2026
Address PR review: capture and restore the previous ``propagate`` value
in a ``finally`` block so the test doesn't leak global logger state to
later tests.
Comment thread marimo/_session/file_change_handler.py Outdated
akshayka
akshayka previously approved these changes May 21, 2026
Comment thread marimo/_session/file_change_handler.py Outdated
Comment thread marimo/_session/file_change_handler.py Outdated
Co-authored-by: Akshay Agrawal <akshaykagrawal7@gmail.com>
@mscolnick mscolnick merged commit e619247 into main May 21, 2026
42 of 43 checks passed
@mscolnick mscolnick deleted the ms/fix/issue-9613 branch May 21, 2026 18:24
@github-actions
Copy link
Copy Markdown

🚀 Development release published. You may be able to view the changes at https://marimo.app?v=0.23.7-dev82

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

marimo edit --watch messes with merge conflict markers

3 participants