fix(editor): stop external file syncs from stranding the dirty marker (#3165)

src/web-ui/src/tools/editor/components/CodeEditor.tsx

@@ -371,7 +371,14 @@ const CodeEditor: React.FC<CodeEditorProps> = ({
 
     const previousLoadingState = isLoadingContentRef.current;
     isLoadingContentRef.current = true;
-    model.setValue(nextContent);
+    // Programmatic disk sync: bracket the write so the model manager does not
+    // flag the model dirty for content nobody typed (issue #3165).
+    monacoModelManager.beginExternalSync();
+    try {
+      model.setValue(nextContent);
+    } finally {
+      monacoModelManager.endExternalSync();
+    }
     setIndentation(applyModelIndentation(model, latestEditorConfigRef.current ?? {}, true));
 
     queueMicrotask(() => {
@@ -404,14 +411,17 @@ const CodeEditor: React.FC<CodeEditorProps> = ({
       if (pos && editorRef.current) {
         editorRef.current.setPosition(pos);
       }
+      // Settle the saved state synchronously: deferring it to a microtask let
+      // an unmount race skip markAsSaved and strand stale saved metadata after
+      // the disk sync (issue #3165).
+      if (modelRef.current && filePath) {
+        savedVersionIdRef.current = modelRef.current.getAlternativeVersionId();
+        monacoModelManager.markAsSaved(modelKey);
+      }
       onContentChange?.(fileContent, false);
       reportFileMissingFromDisk(false);
       queueMicrotask(() => {
         isLoadingContentRef.current = false;
-        if (modelRef.current && !isUnmountedRef.current && filePath) {
-          savedVersionIdRef.current = modelRef.current.getAlternativeVersionId();
-          monacoModelManager.markAsSaved(modelKey);
-        }
       });
     },
     [applyExternalContentToModel, filePath, modelKey, onContentChange, reportFileMissingFromDisk, updateLargeFileMode]
@@ -1424,12 +1434,12 @@ const CodeEditor: React.FC<CodeEditorProps> = ({
         }
         log.warn('Failed to sync disk version after encoding change', err);
       }
-      queueMicrotask(() => {
-        if (modelRef.current && !isUnmountedRef.current) {
-          savedVersionIdRef.current = modelRef.current.getAlternativeVersionId();
-          monacoModelManager.markAsSaved(modelKey);
-        }
-      });
+      // Same unmount race as the disk-sync path: settle the saved state
+      // synchronously right after the bracketed content write (issue #3165).
+      if (modelRef.current) {
+        savedVersionIdRef.current = modelRef.current.getAlternativeVersionId();
+        monacoModelManager.markAsSaved(modelKey);
+      }
     } catch (err) {
       if (isLikelyFileNotFoundError(err)) {
         reportFileMissingFromDisk(true);

src/web-ui/src/tools/editor/services/MonacoModelManager.test.ts

@@ -0,0 +1,180 @@
+// @vitest-environment jsdom
+
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
+import type * as Monaco from 'monaco-editor';
+import { setMonacoRuntime } from './monacoRuntime';
+
+vi.mock('@/shared/utils/logger', () => ({
+  createLogger: () => ({
+    debug: vi.fn(),
+    info: vi.fn(),
+    warn: vi.fn(),
+    error: vi.fn(),
+  }),
+}));
+
+import { monacoModelManager } from './MonacoModelManager';
+
+type ChangeListener = () => void;
+
+class FakeModel {
+  private value: string;
+  private versionId = 1;
+  private listeners: ChangeListener[] = [];
+  public readonly uri: { toString: () => string };
+
+  constructor(value: string, uriString: string) {
+    this.value = value;
+    this.uri = { toString: () => uriString };
+  }
+
+  getValue(): string {
+    return this.value;
+  }
+
+  setValue(next: string): void {
+    if (next === this.value) return;
+    this.value = next;
+    this.versionId += 1;
+    this.listeners.forEach(listener => listener());
+  }
+
+  getAlternativeVersionId(): number {
+    return this.versionId;
+  }
+
+  onDidChangeContent(listener: ChangeListener): { dispose: () => void } {
+    this.listeners.push(listener);
+    return {
+      dispose: () => {
+        const index = this.listeners.indexOf(listener);
+        if (index > -1) this.listeners.splice(index, 1);
+      },
+    };
+  }
+}
+
+function createFakeMonaco(): typeof Monaco {
+  const models = new Map<string, FakeModel>();
+
+  const Uri = {
+    file: (path: string) => ({ toString: () => `file://${path}` }),
+    parse: (value: string) => ({ toString: () => value }),
+  };
+
+  const editor = {
+    getModel: (uri: { toString: () => string }) => models.get(uri.toString()) ?? null,
+    createModel: (content: string, _language: string, uri: { toString: () => string }) => {
+      const model = new FakeModel(content, uri.toString());
+      models.set(uri.toString(), model);
+      return model;
+    },
+    onWillDisposeModel: () => ({ dispose: () => {} }),
+  };
+
+  return { Uri, editor } as unknown as typeof Monaco;
+}
+
+interface DirtyEvent {
+  filePath: string;
+  isDirty: boolean;
+}
+
+function listenDirtyEvents(): { events: DirtyEvent[]; dispose: () => void } {
+  const events: DirtyEvent[] = [];
+  const handler = (event: Event) => {
+    const detail = (event as CustomEvent<DirtyEvent>).detail;
+    events.push({ filePath: detail.filePath, isDirty: detail.isDirty });
+  };
+  window.addEventListener('monaco-model-dirty-changed', handler as EventListener);
+  return {
+    events,
+    dispose: () => window.removeEventListener('monaco-model-dirty-changed', handler as EventListener),
+  };
+}
+
+let dirtyListener: { events: DirtyEvent[]; dispose: () => void };
+
+beforeEach(() => {
+  setMonacoRuntime(createFakeMonaco());
+  dirtyListener = listenDirtyEvents();
+});
+
+afterEach(() => {
+  dirtyListener.dispose();
+  setMonacoRuntime(null);
+});
+
+describe('MonacoModelManager external sync dirty state (issue #3165)', () => {
+  it('updateModelContent with markAsSaved=true leaves the model clean', () => {
+    const filePath = '/repo/external-sync-saved.ts';
+    const model = monacoModelManager.getOrCreateModel(filePath, 'typescript', 'const a = 1;');
+    expect(monacoModelManager.getModelMetadata(filePath)?.isDirty).toBe(false);
+
+    // External program rewrote the file on disk; the sync path pushes the new
+    // disk truth into the open model and marks it saved.
+    monacoModelManager.updateModelContent(filePath, 'const a = 2;', true);
+
+    expect(model.getValue()).toBe('const a = 2;');
+    const metadata = monacoModelManager.getModelMetadata(filePath);
+    expect(metadata?.isDirty).toBe(false);
+  });
+
+  it('updateModelContent with markAsSaved=true broadcasts a clean dirty-changed event', () => {
+    const filePath = '/repo/external-sync-broadcast.ts';
+    monacoModelManager.getOrCreateModel(filePath, 'typescript', 'const b = 1;');
+    dirtyListener.events.length = 0;
+
+    monacoModelManager.updateModelContent(filePath, 'const b = 2;', true);
+
+    // Consumers that track the dirty dot must see the model end up clean; a
+    // transient dirty flip without a clean follow-up strands the marker.
+    const mine = dirtyListener.events.filter(event => event.filePath === filePath);
+    expect(mine.length).toBeGreaterThan(0);
+    expect(mine[mine.length - 1].isDirty).toBe(false);
+    expect(mine.some(event => event.isDirty)).toBe(false);
+  });
+
+  it('a bracketed external sync does not flip the dirty flag or broadcast transient dirt', () => {
+    const filePath = '/repo/external-sync-bracket.ts';
+    const model = monacoModelManager.getOrCreateModel(filePath, 'typescript', 'const c = 1;');
+    dirtyListener.events.length = 0;
+
+    monacoModelManager.beginExternalSync();
+    try {
+      model.setValue('const c = 2;');
+    } finally {
+      monacoModelManager.endExternalSync();
+    }
+    monacoModelManager.markAsSaved(filePath);
+
+    expect(monacoModelManager.getModelMetadata(filePath)?.isDirty).toBe(false);
+    const mine = dirtyListener.events.filter(event => event.filePath === filePath);
+    expect(mine.some(event => event.isDirty)).toBe(false);
+    expect(mine[mine.length - 1]?.isDirty).toBe(false);
+  });
+
+  it('still marks real user edits outside a sync bracket as dirty', () => {
+    const filePath = '/repo/user-edit-control.ts';
+    const model = monacoModelManager.getOrCreateModel(filePath, 'typescript', 'const d = 1;');
+    dirtyListener.events.length = 0;
+
+    // Control: an edit that is not an external sync must keep the old behavior.
+    model.setValue('const d = 2; // user typed');
+
+    expect(monacoModelManager.getModelMetadata(filePath)?.isDirty).toBe(true);
+    const mine = dirtyListener.events.filter(event => event.filePath === filePath);
+    expect(mine.some(event => event.isDirty)).toBe(true);
+  });
+
+  it('an unbalanced endExternalSync never leaves suppression stuck on', () => {
+    const filePath = '/repo/unbalanced-bracket.ts';
+    const model = monacoModelManager.getOrCreateModel(filePath, 'typescript', 'const e = 1;');
+
+    monacoModelManager.endExternalSync(); // no matching begin: must be a no-op
+    dirtyListener.events.length = 0;
+    model.setValue('const e = 2; // user typed');
+
+    expect(monacoModelManager.getModelMetadata(filePath)?.isDirty).toBe(true);
+  });
+});

src/web-ui/src/tools/editor/services/MonacoModelManager.ts

@@ -83,6 +83,14 @@ class MonacoModelManager {
 
   private globalListenersInstalled = false;
 
+  /**
+   * Depth counter for programmatic external-sync brackets. While non-zero,
+   * content changes are disk-sync writes (issue #3165), so the dirty state is
+   * owned by the sync caller (markAsSaved / updateModelContent) instead of the
+   * change listener. A counter keeps nested brackets balanced.
+   */
+  private externalSyncDepth = 0;
+
   private constructor() {}
 
   public static getInstance(): MonacoModelManager {
@@ -213,6 +221,13 @@ class MonacoModelManager {
     model: monaco.editor.ITextModel
   ): void {
     const listener = model.onDidChangeContent(() => {
+      // Inside an external-sync bracket the write is the disk truth, not a user
+      // edit: skip the dirty recompute and the transient dirty broadcast so the
+      // tab never flashes "modified" for a programmatic sync (issue #3165).
+      // The bracket caller settles the final state (markAsSaved / saved flag).
+      if (this.externalSyncDepth > 0) {
+        return;
+      }
       const metadata = this.modelMetadata.get(uriString);
       if (metadata) {
         const currentVersionId = model.getAlternativeVersionId();
@@ -345,15 +360,37 @@ class MonacoModelManager {
     const wasEmpty = !metadata || metadata.originalContent === '';
     const isFirstContentSet = wasEmpty && content.length > 0;
 
-    model.setValue(content);
+    // markAsSaved callers push the disk truth into an open model (issue
+    // #3165): bracket the write so the change listener does not recompute the
+    // dirty flag or broadcast a transient "modified" state in between.
+    if (markAsSaved) {
+      this.beginExternalSync();
+    }
+    try {
+      model.setValue(content);
+    } finally {
+      if (markAsSaved) {
+        this.endExternalSync();
+      }
+    }
 
     if (metadata) {
       if (markAsSaved) {
         metadata.savedVersionId = model.getAlternativeVersionId();
         metadata.originalContent = content;
         metadata.isDirty = false;
+        metadata.lastAccessedAt = Date.now();
+
+        window.dispatchEvent(new CustomEvent('monaco-model-dirty-changed', {
+          detail: {
+            uri: uriString,
+            filePath: metadata.filePath,
+            isDirty: false
+          }
+        }));
+      } else {
+        metadata.lastAccessedAt = Date.now();
       }
-      metadata.lastAccessedAt = Date.now();
     }
 
     this.markLoadingComplete(uriString);
@@ -368,6 +405,25 @@ class MonacoModelManager {
     }
   }
 
+  /**
+   * Open a programmatic external-sync bracket. While it is open, model content
+   * changes skip the dirty recompute and the transient dirty broadcast
+   * (issue #3165). Pair every begin with an end in a finally block.
+   */
+  public beginExternalSync(): void {
+    this.externalSyncDepth += 1;
+  }
+
+  /**
+   * Close an external-sync bracket. Unbalanced calls (more ends than begins)
+   * are no-ops so suppression can never get stuck on and swallow real edits.
+   */
+  public endExternalSync(): void {
+    if (this.externalSyncDepth > 0) {
+      this.externalSyncDepth -= 1;
+    }
+  }
+
   public markAsSaved(filePath: string, savedContent?: string, savedVersionId?: number): void {
     const uri = this.normalizeUri(filePath);
     const uriString = uri.toString();
@@ -389,7 +445,7 @@ class MonacoModelManager {
       }));
     }
   }
-  
+
   public getModelMetadata(filePath: string): ModelMetadata | undefined {
     if (!getMonacoRuntime()) {
       return undefined;