(file-panel): keep a dirty file tab when a session opens a file or a diff over it by jbr-sekoia · Pull Request #369 · devsuitup/switchboard

jbr-sekoia · 2026-09-30T17:12:32Z

Closes #364.

Defect

A session's panel has one tab slot, and three routes replace what is in it: MCP openFile (and a terminal path link), MCP openDiff, which the CLI sends for every proposed edit, and a path link to a git-changed file, which opens the Changes tab. All three went through destroyCurrentTab, which destroyed a file tab's editor or dropped its snapshot, token and pending save record. Unsaved edits were lost without a word.

Rule

Nothing a session triggers shows a modal, and no route drops unsaved edits without the user saying so.

A file tab with unsaved edits is held in the session's heldFileTabs: it is snapshotted (buffer, bases, token) as a session switch already does, never destroyed.

Route	Over a dirty file tab	Over a clean one
`openFile`, same file	kept in place and re-read: edits kept, "changed on disk" if the session wrote	reloads
`openFile`, another file	held; the bar above the file tab names it ("Unsaved edits kept in: …")	replaced
`openDiff`	held, restored when the session closes the diff	replaced
link to a changed file (Changes)	held, restored when Changes is closed	replaced
the panel's close button (the user's click)	asks; a no keeps the tab	closed

A held tab also comes back when an open names its file. It is restored and re-read, so an accepted diff shows as "changed on disk", and _agreedBase has not moved, so main refuses a save against the session's write and the panel asks. Another session's tab is never touched. ViewerPanel.destroy() now clears its token, so a save that resolves after its tab was held is recorded for that tab rather than lost. mcp-bridge.js resolves the openFile path; rejects an openFile with no usable filePath with a JSON-RPC error; the renderer folds separators and case on win32 only (APFS can be case-sensitive). A held tab whose save finishes clean leaves the bar; the bar has role="status" and lengthens colliding names one directory at a time until each is distinct (a root file shows as /a.md). A same-file open that arrives before the viewer's open() has put its document in the editor queues its re-read rather than reopening, so a restored snapshot is never bypassed. Until the CodeMirror bundle has loaded, the document waiting for the editor is the buffer: it is what the dirty check and the snapshot read, a watcher event arriving then is queued like an open's re-read, and a failed load is retried on the next re-read. openFile still answers ok before the renderer acts, which stays accurate because no open is declined or deferred. Written up in .ai/contexts/viewer-panel.md, "An open aimed at a file tab".

Also fixed: a Save before the editor has loaded emptied the file (present on main)

Until the CodeMirror bundle has loaded, or after it failed to load, getContent() returns ''. A Save or Ctrl+S in that window sent {content: '', expected: <the disk>}, which main accepted because the disk still matched, and the file was emptied. The same test run against main's viewer-panel.js sends {content: '', expected: 'a0\n'}, then a queued {content: '', expected: ''}. _save() now does nothing while the document is not in the editor, the Save button is disabled for that time, and Copy copies the pending document (test/viewer-panel-pending-document.test.js).

Evidence

test/dom-file-panel-file-tab.test.js covers every route: the issue's scenario, five rapid opens (0 modals), a dirty tab surviving an openDiff and restored intact when the diff closes, an accepted diff written by the session, closeAllDiffTabs, a save failing while held, the Changes route, the close button, and Windows/Linux path spelling. test/mcp-bridge-open-file.test.js drives the real WebSocket server. Run against a10c86a, 14 of these fail: 12 on assertions, and the Windows and detached-save tests because the bar element is missing there; both of those are proven by mutations.
66 mutations of the new guards: each one turns at least one test red.
Live, in an isolated instance with a throwaway HOME and the window unfocusable. A dirty README.md, then openDiff: 0 dialogs. After accept, a session write and close_tab: the edits were back, with "changed on disk". Five rapid openFile: 0 dialogs, with README.md named in the bar, and a click on it restored the edits. openFileInPanel on a changed file showed Changes; closing Changes restored the edits.
npm test: 2537 + 120 tests, 0 fail; lint 0 errors.

…file over it A session's openFile, and a terminal path link, went through openFileTab, which replaced the session's file tab unconditionally. A tab away from the viewer lost its snapshot, its token and any save outcome recorded under that token; a tab shown in the viewer had its editor torn down. Either way unsaved edits went without a word. openFileTab now leaves a tab that has reached the viewer in place when the open names the same file: the shown tab is re-read at once, an away tab is restored and re-read on its return, so a dirty buffer keeps its edits and raises "changed on disk", and a pending save record is still applied. An open naming another file asks before discarding a dirty tab, and a no drops the open. The dirty test for an away tab reads its snapshot and counts a save that finished while away. Another session's tab is never touched. Closes #364

devsuitup · 2026-09-30T17:20:20Z

Reviewing a10c86a (adversarial review in progress).

devsuitup

Adversarial review at a10c86a.

The rule table holds, row by row (code and tests), and a decline is fully clean: return precedes destroyCurrentTab, so tab, viewerState, token and viewer are untouched; an accept releases the viewer (_openGen++, unwatch, editor destroyed, fpViewerOwner = null) and the away tab's detached record goes with its WeakMap key.

Blocking — a synchronous window.confirm on an automated open, for a session the user may not be looking at. openFileTab (public/file-panel.js:476) is reached directly from onMcpOpenFile and asks whenever the current file tab is dirty — regardless of currentPanelSessionId !== sessionId and of window focus. Consequences:

window.confirm blocks the renderer: every terminal's rendering, the sidebar, the grid freeze until someone answers. An agent opening a file in a background session while the user is away leaves the whole window frozen on a dialog.
The text names the tab (README.md has unsaved edits…), not the session, so the user is asked about a tab they cannot see.
On cancel the MCP tool has already returned ok (mcp-bridge.js handleOpenFile sends mcp-open-file and returns at once): the agent believes the file is open.

Suggested shape: when the target session is not the displayed one (or the window is unfocused), do not ask and do not replace — keep the dirty tab and surface a non-modal notice on that session (or queue the open until the session is shown); keep the confirm for the displayed session. Optionally let the MCP reply say the open was declined. A test for "open aimed at a non-displayed session with a dirty tab" would pin it.

Non-blocking:

reopenFileTab (:499-506) while the tab is shown but editorView is still null (bundle loading / editor not created): rereadFromDisk() returns at its first line, so data.content is dropped and stale content shows until the next watcher event. If editorView is null and the tab is clean with no viewerState, update tab.content.
snapshotHasUnsavedEdits (public/viewer-panel.js:601-604) compares with agreedBase only, where _isDirty also treats a buffer equal to _lastSeenDisk as clean — the same buffer can read clean when shown and dirty when away (an extra prompt, safe side). No test for an open while a save is in flight, shown or away.
current.filePath === data.filePath (:475) is a raw compare; on Windows C:\x vs C:/x or drive-letter case would turn a same-file open into a "discard?" prompt. Not verified that the two entry points spell paths differently.
reopenFileTab does not set state.panelVisible = true as openFileTab does (harmless today, asymmetric).

Verified: node --test test/dom-file-panel-file-tab.test.js 27/27; the same file against origin/main's file-panel.js in a scratch copy: 7 fail (dirty-away, dirty-shown, confirm-accept, same-file dirty shown/away, failed-save-while-away, line reveal); CI green on this head; CHANGELOG.md entry well-formed, changelog job green; comment sweep respected.

… that replaces it The first fix asked with window.confirm before an openFile of another file replaced a dirty tab, and left two routes that still dropped one without a word: an MCP openDiff, which the CLI sends for every proposed edit, and a path link to a git-changed file, which opens the Changes tab. Both go through destroyCurrentTab, which tore the viewer down. A confirm raised by the session also stacked one modal per call and could be answered by a keystroke meant for the terminal. destroyCurrentTab now keeps a file tab with unsaved edits in the session's heldFileTabs, snapshotted with its bases and token as a session switch does. The tab comes back when the diff or the Changes tab that replaced it ends, when an open names its file, or from a bar above the file tab that names the held files. It is restored and re-read, so a write made meanwhile raises "changed on disk" and a save against it is still refused by main. Nothing the session triggers shows a modal; the panel's own close button asks before it discards a dirty tab. ViewerPanel.destroy() clears the token, so a save resolving after its tab was held is recorded for that tab rather than applied to the destroyed editor. mcp-bridge resolves the openFile path, and the renderer compares paths with separators and case folded where the file system does.

devsuitup · 2026-09-30T18:05:26Z

Reviewing 84d6e50 (adversarial review in progress).

devsuitup

Re-review at 84d6e50 (delta from a10c86a). Approve — the round-1 blocker is gone; CI green on this head.

Blocker resolved. No confirm() remains on any route that replaces a tab (openFile, path link, goto-line, diff, Changes). The only confirms left are user-initiated (Close on a dirty tab, discard Changes edits, overwrite after disk change). A dirty tab replaced by an incoming open is held in heldFileTabs with its viewer state, surfaced by the "Unsaved edits kept in:" bar, and restored by endCurrentTab when the current tab ends. The requested file always opens, so the MCP "ok" reply is now accurate. Mutations: holdFileTabIfDirty as a no-op fails 10/37 in dom-file-panel-file-tab; endCurrentTab ignoring held tabs fails 6/37; main's file-panel.js fails 18/37; dropping path.resolve fails the new MCP test.

Non-blocking — should be fixed (follow-up is fine):

mcp-bridge.js:236 — path.resolve(args.filePath) now runs outside the try. path.resolve(undefined) throws TypeError (checked), the async handler has no catch on the dispatch path, so an openFile call without a string filePath gets no JSON-RPC reply and the caller hangs. Before this change the call still answered. Guard with typeof args.filePath === 'string' → sendError(-32602), plus a test.

Non-blocking — optional:

public/file-panel.js:508-514 — round-1 leftover: reopenFileTab drops fresh data.content when the tab owns the viewer but editorView is not created yet.
public/file-panel.js:562-580 — the held bar is only drawn on file tabs; while a diff or Changes tab is shown, held dirty tabs are invisible.
public/file-panel.js:283-306 — Close on a clean tab with held tabs pops the next held tab instead of hiding the panel; hiding with N held tabs takes up to N confirms.
heldFileTabs is unbounded (one CodeMirror state per file) until the session state is deleted.
Nits: reopenFileTab does not set panelVisible; fileTabHasUnsavedEdits (snapshot) vs _isDirty can still disagree — now only an extra hold, not a prompt.

- mcp-bridge: an openFile without a non-empty string filePath threw in path.resolve before any reply; it is now answered with a -32602 error and opens nothing. - Paths are case-folded on win32 only. APFS can be case-sensitive, and folding on darwin held A.md under a.md's key. - A save that finishes while its tab is held and leaves it clean takes it out of the bar: ViewerPanel reports each save recorded for an away tab. - snapshotHasUnsavedEdits also treats a buffer equal to the disk last seen as clean, as _isDirty does, so a tab reads the same shown and away. - A same-file open before the editor exists (the bundle still loading) is opened again with the content sent, rather than dropped by a re-read that has nothing to read; reopenFileTab sets panelVisible as openFileTab does. - The bar has role="status", and two held files of the same name show their parent directory.

devsuitup · 2026-09-30T18:35:08Z

Reviewing 404a9ae (adversarial review in progress).

devsuitup

Re-review at 404a9ae (delta from 84d6e50). No blocking finding in the code — but not approvable yet: the PR conflicts with main (mergeable: CONFLICTING), so no CI ran on this head. Rebase on main; I will approve once CI is green on the new head (a conflict-only rebase needs no new review round).

Previous points: filePath guard taken (test goes red when the guard is disabled); reopenFileTab with no editor view taken; snapshotHasUnsavedEdits now uses the same rule as _isDirty (buffer !== agreedBase && buffer !== lastSeenDisk, viewer-panel.js:187-190 / :605) and lastSeenDisk is set on every open (:298) — no edit-loss path found; saved held tabs are now dropped via onDetachedSave → dropSavedHeldTabs, a failed save keeps the tab (tested). Held bar not drawn over diff/Changes tabs is documented as intended — fine.

Local evidence: 49/49 in dom-file-panel-file-tab + mcp-bridge-open-file, plus changes (102), diff-save (10), goto-line (5). Ten mutations of the new code each turn one test red.

Nits:

file-panel.js:518 — state.panelVisible = true in reopenFileTab has no test (removing it stays green).
viewer-panel.js:490-494 — the _detachedSaves entry of a held tab dropped by dropSavedHeldTabs is never deleted; small leak until the viewer is destroyed.
Still open, optional: dirty held tabs are unbounded; Close on a clean tab pops the next held tab rather than hiding the panel.

…g it A same-file open that arrived before the viewer's open() had created its editor reopened the tab from tab.content with a new token. When that open was the restore of a held tab, the snapshot had already been consumed, so the restored edits were replaced by the content first read. rereadFromDisk now queues the re-read while an open() is pending and runs it once the document is in the editor; a new open() clears the queue. A fresh open and a restore are handled the same way, and the snapshot is never bypassed. The unreachable panelVisible assignment in reopenFileTab goes. Held file names that collide are lengthened one directory at a time until each is distinct, and a file at the root shows as /a.md instead of untitled/a.md.

devsuitup · 2026-09-30T19:05:06Z

Reviewing 76ac77b (adversarial review in progress).

devsuitup

Re-review at 76ac77b (delta from 404a9ae). No blocking finding — still not approvable: the PR conflicts with main, so no CI ran. Rebase on main; I approve once CI is green on the new head (a conflict-only rebase needs no new round).

The queued re-read holds up: open() sets _openPending and clears _rereadQueued synchronously (viewer-panel.js:344-345), so a re-read queued for A is dropped when B opens; the queue runs once after the document is in the editor (:379-380), after a restore has set lastSeenDisk from the snapshot, so a restored dirty buffer ends in a "changed" notice with its edits kept (new test). The panelVisible line was unreachable: every path that hides the panel clears currentTab first. The held-name disambiguation terminates (bounded by the longest path) and the three-way collision is tested.

Mutations of each new branch (queue set, queue run, queue cleared on open, _openPending reset, disambiguation loop, rereadFromDisk in reopenFileTab) each turn tests red; 50/50 unmutated in dom-file-panel-file-tab, related panel/viewer files green.

Non-blocking:

viewer-panel.js — nothing pins "re-read queued, then destroy() / open(B)": removing the pre-existing _openGen !== myGen guard in the open() closure keeps every test green. One test would cover it.
If loadCodeMirrorBundle fails, _openPending stays true and later re-reads queue forever — no editor exists in that state anyway, so no regression.
file-panel.js:512 — a second same-file open before the editor exists now re-reads from disk instead of using data.content; equivalent as long as the payload was read from that path.

Until the CodeMirror bundle has loaded, open() has not put its document in the editor, and three things went wrong in that window: - A restored held tab replaced again before its editor existed read as clean (no editor, no viewerState), so it was not held and its edits were lost. - A failed bundle load left the open pending for good: every later open of the file only queued a re-read, and the tab stayed blank. - A file-changed event arriving during the load was dropped, so the editor came up with the old content and base, and the next save was refused. open() now keeps the document as _pendingContent until it is in the editor, and _isDirty and snapshot() read it. The watcher goes through rereadFromDisk, so its re-read is queued like a same-file open's. A failed load ends the pending state, and the next re-read opens the panel again from its own snapshot, retrying the load. A test pins that a name which does not clash stays short.

devsuitup · 2026-09-30T19:35:07Z

Reviewing 1297741 (adversarial review in progress).

devsuitup

Re-review at 1297741 (delta from 76ac77b). No blocking finding — still not approvable: the PR conflicts with main, so no CI ran. Rebase on main and I approve once CI is green (a conflict-only rebase needs no new round).

Checked: _pendingContent and _agreedBase are set in the same synchronous open(), so dirtiness never compares across files; a superseded open (A then B) ends with B's path and content; the retry in rereadFromDisk does not loop (a second call while the reopen is pending is queued) and keeps restore.token, so detached-save attribution is unchanged; with an editor and no pending open, the watcher path is the old _reloadFromDisk. Each new branch (pending set/clear, catch resetting _openPending, retry branch, watcher routing, _isDirty / snapshot / _hasDocument reading the pending document) turns a test red when mutated. Related panel/viewer suites green locally (viewer-file-watch has one libuv _wcsnicmp failure on this host, identical on main).

Non-blocking:

viewer-panel.js:517-531 — destroy() leaves _pendingContent and _openPending set. After destroy during a bundle load, _hasDocument() is true and snapshot() returns the destroyed file. Not reachable from file-panel.js today (every dirty/snapshot call is behind fpViewerOwner === tab, and both destroy() sites null the owner), but it is a trap for the next caller. Reset both in destroy().
viewer-panel.js:631-634 — the retry goes through open(), which clears the notice: a save-failed notice from a detached-save restore is lost if the bundle also failed. Narrow.
viewer-panel.js:394-397 — the catch has no _openGen guard; harmless today (one shared promise).

Nit: removing this._title = title keeps every test green — assert the title in the bundle-failure test.

…nding Until the CodeMirror bundle has loaded, or after it failed to load, the editor does not exist and getContent() returns ''. A Save or Ctrl+S in that window sent {content: '', expected: <the disk>}; main accepted it, since the disk still matched, and the file was emptied. main has the same defect. _save() now does nothing while the document is not in the editor, and the Save button is disabled for that time: nothing has been typed yet. Copy copies the pending document instead of the empty editor. The failed-load test now writes to disk before the second open, so the reopen is shown to come from the panel's own snapshot and to re-read.

Brings in #370. CHANGELOG.md conflicted under Unreleased: both sides added a group. Kept both, New (#334) before Fixed (#364, #369), the order docs/changelog.md sets.

devsuitup · 2026-09-30T20:05:00Z

Reviewing 0e9fe82 (adversarial review in progress).

devsuitup

Re-review at 0e9fe82 (delta from 1297741). No blocking finding — still not approvable: the PR conflicts with main, so no CI ran. Rebase on main and I approve once CI is green (a conflict-only rebase needs no new round).

The fix is right: _save() returns while _pendingContent !== null (viewer-panel.js:456), so a Save/Ctrl+S before the editor exists can no longer send {content: ''} against a matching disk and empty the file; Copy reads _buffer(). Nothing else in the viewer or toolbar manages saveBtn.disabled, so the disable in open() and re-enable on success do not fight another state. A superseded open is re-enabled by the winning closure; a failed load leaves Save disabled until the retry succeeds, which is the intent.

Local, on this head: 62/62 in viewer-panel-pending-document + dom-file-panel-file-tab. Mutations — save guard removed, Copy back to getContent(), button not disabled — each turn 2 tests red.

Nit (carried): destroy() still leaves _pendingContent set; with this commit that also leaves _save() a no-op on a destroyed viewer, which is harmless but another reason to reset it there.

CHANGELOG.md conflicted under "## Unreleased": main added a New entry (#334) and two Fixed entries (#364, #369), this branch a Changed and a Fixed entry (#359). All five are kept, grouped New, Changed, Fixed per docs/changelog.md, main's Fixed entries first.

devsuitup requested changes Sep 30, 2026

View reviewed changes

jbr-sekoia changed the title ~~(file-panel): keep a file tab's unsaved edits when a session opens a file over it~~ (file-panel): keep a dirty file tab when a session opens a file or a diff over it Sep 30, 2026

jbr-sekoia mentioned this pull request Sep 30, 2026

(file-panel): quitting, reloading or closing a session drops unsaved file edits without a warning #373

Closed

devsuitup approved these changes Sep 30, 2026

View reviewed changes

devsuitup self-requested a review September 30, 2026 18:09

devsuitup reviewed Sep 30, 2026

View reviewed changes

jbr-sekoia requested a review from devsuitup September 30, 2026 20:02

jbr-sekoia enabled auto-merge (squash) September 30, 2026 20:02

Merge origin/main into fix/mcp-open-keeps-dirty-file-tab

c4cc77a

Brings in #370. CHANGELOG.md conflicted under Unreleased: both sides added a group. Kept both, New (#334) before Fixed (#364, #369), the order docs/changelog.md sets.

devsuitup reviewed Sep 30, 2026

View reviewed changes

jbr-sekoia merged commit 8a9220b into main Sep 30, 2026
11 checks passed

jbr-sekoia deleted the fix/mcp-open-keeps-dirty-file-tab branch September 30, 2026 20:11

devsuitup mentioned this pull request Oct 1, 2026

(file-panel): ask before quit, close or reload drops unsaved file edits (#373) #401

Merged

Navigation Menu

Repository navigation

(file-panel): keep a dirty file tab when a session opens a file or a diff over it - #369

(file-panel): keep a dirty file tab when a session opens a file or a diff over it#369
jbr-sekoia merged 7 commits into
mainfrom
fix/mcp-open-keeps-dirty-file-tab

jbr-sekoia commented Sep 30, 2026 •

edited

Loading

Uh oh!

devsuitup commented Sep 30, 2026

Uh oh!

devsuitup left a comment

Uh oh!

devsuitup commented Sep 30, 2026

Uh oh!

devsuitup left a comment

Uh oh!

devsuitup commented Sep 30, 2026

Uh oh!

devsuitup left a comment

Uh oh!

devsuitup commented Sep 30, 2026

Uh oh!

devsuitup left a comment

Uh oh!

devsuitup commented Sep 30, 2026

Uh oh!

devsuitup left a comment

Uh oh!

devsuitup commented Sep 30, 2026

Uh oh!

devsuitup left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

jbr-sekoia commented Sep 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Defect

Rule

Also fixed: a Save before the editor has loaded emptied the file (present on main)

Evidence

Uh oh!

devsuitup commented Sep 30, 2026

Uh oh!

devsuitup left a comment

Choose a reason for hiding this comment

Uh oh!

devsuitup commented Sep 30, 2026

Uh oh!

devsuitup left a comment

Choose a reason for hiding this comment

Uh oh!

devsuitup commented Sep 30, 2026

Uh oh!

devsuitup left a comment

Choose a reason for hiding this comment

Uh oh!

devsuitup commented Sep 30, 2026

Uh oh!

devsuitup left a comment

Choose a reason for hiding this comment

Uh oh!

devsuitup commented Sep 30, 2026

Uh oh!

devsuitup left a comment

Choose a reason for hiding this comment

Uh oh!

devsuitup commented Sep 30, 2026

Uh oh!

devsuitup left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jbr-sekoia commented Sep 30, 2026 •

edited

Loading