Skip to content

fix(tasks): preserve dirty diff edits#2542

Open
janburzinski wants to merge 3 commits into
mainfrom
jan/eng-1596-edits-inside-the-diff-viewer-are-not-applied
Open

fix(tasks): preserve dirty diff edits#2542
janburzinski wants to merge 3 commits into
mainfrom
jan/eng-1596-edits-inside-the-diff-viewer-are-not-applied

Conversation

@janburzinski

Copy link
Copy Markdown
Collaborator

Description

  • fix saving edits made in diff tabs
  • show dirty state on diff tabs
  • keep dirty diff previews open

Screenshot/Recording (if applicable)

https://streamable.com/g0c66y

Checklist
  • I kept this PR small and focused
  • I ran a self-review before opening this PR
  • I ran the relevant local checks or explained why not
  • I updated docs when behavior or setup changed
  • I added or updated tests when behavior changed, or explained why not
  • I only added comments where the logic is not obvious
  • I used Conventional Commits for commit
    messages and, when possible, the PR title

@greptile-apps

greptile-apps Bot commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR extends the Monaco model lifecycle to cover disk working-tree diff tabs in addition to file tabs, so dirty edits made in a diff editor survive tab-switching and renderer unmounts. It also adds a dirty indicator dot to diff tab items, prevents a dirty preview diff tab from being silently replaced, and fixes Ctrl+S to operate on the active editable path (file or editable diff).

  • getEditableBufferPath + allOpenEditablePaths: new helper + computed deduplicates editable paths across panes, feeding the existing register/unregister reaction and replacing allOpenFilePaths in the lifecycle store.
  • openDiffPreview dirty guard: a dirty disk-diff preview is now promoted to a permanent tab rather than replaced when a new preview is requested.
  • saveAllFiles + _hasOpenEditableTabForPath: save-all now includes editable diff paths, and the clearBuffer call is guarded against clearing content that another editable tab still owns.

Confidence Score: 4/5

Safe to merge for the core happy-path flows; one edge case involving tab group transitions on dirty diff tabs can silently discard unsaved edits, already flagged in a prior review pass.

The implementation correctly extends the Monaco model lifecycle to editable diff tabs and the new tests cover the main scenarios well. The one outstanding concern — a dirty disk-diff tab that transitions to the staged group via transitionDiffTab loses its buffer content without a save dialog, because the group change causes the path to leave allOpenEditablePaths and _unregisterModels is called before the user is prompted — was introduced by hooking disk-diff tabs into the lifecycle reaction and was flagged in a prior review comment. Everything else (preview promotion, dirty indicator, save-all extension, clearBuffer guard) is well-handled.

file-model-lifecycle-store.ts and tab-manager-store.ts — specifically the interaction between the allOpenEditablePaths reaction and transitionDiffTab mutations.

Important Files Changed

Filename Overview
apps/emdash-desktop/src/renderer/features/tasks/editor/editor-provider.tsx Replaces activeFilePath with activeEditablePath for Ctrl+S and paste handlers — minimal, correct change that extends keyboard save to editable diff tabs.
apps/emdash-desktop/src/renderer/features/tasks/editor/stores/file-model-lifecycle-store.ts Extends register/unregister lifecycle reaction from allOpenFilePaths to allOpenEditablePaths; fixes clearBuffer guard to use editable-path check; the transitionDiffTab scenario (disk→staged on a dirty tab) still triggers _unregisterModels and clears the buffer without a save dialog.
apps/emdash-desktop/src/renderer/features/tasks/tabs/tab-group-manager-store.ts Adds allOpenEditablePaths computed (deduplicated across panes) alongside the existing allOpenFilePaths; clean, mirrors the existing pattern exactly.
apps/emdash-desktop/src/renderer/features/tasks/tabs/tab-manager-store.ts Adds getEditableBufferPath helper, activeEditablePath / openEditablePaths computeds, isDirty + bufferUri on ResolvedDiffTab, and dirty-aware preview replacement in openDiffPreview; logic is correct for the happy path, though the transitionDiffTab edge case can bypass the save dialog.
apps/emdash-desktop/src/renderer/features/tasks/tabs/tab-manager-store.test.ts New diff-tab test suite covers: editable path exposure for disk diffs, dirty-preview promotion instead of replacement, and confirmed absence of bufferUri for staged diffs; mock setup is correct and properly reset in beforeEach.
apps/emdash-desktop/src/renderer/features/tasks/view/tab-bar/diff-tab-item.tsx Adds unsaved-changes dot indicator to diff tabs, mirroring the existing file-tab dirty indicator pattern; status icon is correctly hidden when the dirty dot is shown.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant UI as User / Tab Bar
    participant TM as TabManagerStore
    participant FML as FileModelLifecycleStore
    participant MR as ModelRegistry
    participant RPC as IPC / clearBuffer

    UI->>TM: openDiffPreview(diskDiff)
    TM->>MR: isDirty(previewBufferUri)?
    alt preview is dirty
        TM->>TM: "promote preview.isPreview = false"
        TM->>TM: create new preview tab
    else preview is clean / non-editable
        TM->>TM: replace preview in-place
    end

    Note over TM,FML: allOpenEditablePaths reaction
    TM-->>FML: allOpenEditablePaths changed (path added)
    FML->>MR: registerModel(path, disk/git/buffer)

    UI->>TM: closeTabWithGuard(tabId)
    TM->>FML: closeHandler(tabId)
    FML->>MR: isDirty(bufferUri)?
    alt dirty
        FML->>UI: confirmClose dialog
        UI-->>FML: save / discard / cancel
    end
    FML->>TM: closeTab(tabId)

    Note over TM,FML: allOpenEditablePaths reaction
    TM-->>FML: allOpenEditablePaths changed (path removed)
    FML->>FML: _hasOpenEditableTabForPath?
    alt no other editable tab for path
        FML->>RPC: clearBuffer(path)
    end
    FML->>MR: unregisterModel(uri)
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant UI as User / Tab Bar
    participant TM as TabManagerStore
    participant FML as FileModelLifecycleStore
    participant MR as ModelRegistry
    participant RPC as IPC / clearBuffer

    UI->>TM: openDiffPreview(diskDiff)
    TM->>MR: isDirty(previewBufferUri)?
    alt preview is dirty
        TM->>TM: "promote preview.isPreview = false"
        TM->>TM: create new preview tab
    else preview is clean / non-editable
        TM->>TM: replace preview in-place
    end

    Note over TM,FML: allOpenEditablePaths reaction
    TM-->>FML: allOpenEditablePaths changed (path added)
    FML->>MR: registerModel(path, disk/git/buffer)

    UI->>TM: closeTabWithGuard(tabId)
    TM->>FML: closeHandler(tabId)
    FML->>MR: isDirty(bufferUri)?
    alt dirty
        FML->>UI: confirmClose dialog
        UI-->>FML: save / discard / cancel
    end
    FML->>TM: closeTab(tabId)

    Note over TM,FML: allOpenEditablePaths reaction
    TM-->>FML: allOpenEditablePaths changed (path removed)
    FML->>FML: _hasOpenEditableTabForPath?
    alt no other editable tab for path
        FML->>RPC: clearBuffer(path)
    end
    FML->>MR: unregisterModel(uri)
Loading

Reviews (3): Last reviewed commit: "fix(diff): clear buffers after discard" | Re-trigger Greptile

@janburzinski

Copy link
Copy Markdown
Collaborator Author

@greptileai

@janburzinski

Copy link
Copy Markdown
Collaborator Author

@greptileai

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant