Skip to content

fix(tabs): keep unsaved edits when a .sql file already open is opened again - #2228

Merged
datlechin merged 3 commits into
mainfrom
fix/reopened-sql-file-keeps-edits
Aug 19, 2026
Merged

fix(tabs): keep unsaved edits when a .sql file already open is opened again#2228
datlechin merged 3 commits into
mainfrom
fix/reopened-sql-file-keeps-edits

Conversation

@datlechin

@datlechin datlechin commented Aug 19, 2026

Copy link
Copy Markdown
Member

What this fixes

Opening a .sql file that is already open threw away whatever you had typed in its tab, with no prompt and nothing to undo it.

Found while investigating #2217.

Root cause

QueryTabManager.addTab dedupes on the source file URL and then overwrites the buffer:

if let sourceFileURL,
   let existingIndex = tabs.firstIndex(where: { $0.content.sourceFileURL == sourceFileURL }) {
    if let query = initialQuery {
        tabs[existingIndex].content.query = query
    }
    selectedTabId = tabs[existingIndex].id
    return
}

content.query is what the editor renders, and an external write to that binding reaches the text view through setText, which resets its storage. So the replacement is not undoable, and nothing asks first.

Reachable in one session, no relaunch needed:

  1. Connect, and type anything into the tab that is in front, so it is not an empty scratch tab. (With an empty selected tab, openLinkedFavorite takes its reuse branch instead, which registers the file and short-circuits later opens.)
  2. Open a linked SQL favourite from the sidebar. That branch never registers the file with WindowLifecycleMonitor: registerSourceFile has two call sites, and neither is on this path.
  3. Type into that tab. Do not save.
  4. Click the same favourite again. window(forSourceFile:) finds nothing, the reuse branch is skipped because the tab already has a sourceFileURL, so a payload opens and addTab hits the dedupe. The edits are gone.

A tab restored from disk at launch is in the same state, since restore rebuilds sourceFileURL and registers nothing.

The app already has the right shape for this elsewhere: FileModifiedOnDiskBanner and reloadFileForTab ask before replacing a buffer from disk. QueryTab+Protection.holdsQueryWork exists for exactly "a tab holding query work must not be silently reused in place", and is consulted on the navigation path but not here.

The fix

Reopening a file that is already open shows its tab. The buffer is replaced only when there is nothing of the user's in it, which TabQueryContent.isFileDirty already answers. A clean tab still picks up the current file, so nothing is lost for the common case of reopening a file you have not touched.

The baseline moves with the buffer. savedFileContent and loadMtime were left behind by the old overwrite, so a tab could read as dirty against content it had just loaded, and raise the changed-on-disk banner for a change it had already taken. Both now follow the text, and a pending banner is cleared.

isFileDirty has to be truthful for that guard to mean anything, and it was not. It reads a missing savedFileContent as clean, and a tab rebuilt from a persisted record has none: QueryTab(from: PersistedTab) carries the text but nothing to compare it to. Restore at launch read the baseline back; reopening from Recently Closed did not, so the same tab was honest through one door and not the other. Such a tab lies three ways: no unsaved marker, Save skips it because it believes there is nothing to write, and now the reopen guard would have waved the overwrite through. FileTabBaseline is the one door both paths go through, and adoptReopenedFile additionally refuses when the baseline is unknown, because unknown is not clean.

FileTextLoader now reports the modification date it saw, taken before the read. A caller that stats afterwards records a date newer than the text it holds, so a write landing in between is invisible: the tab looks current against a file it never read. Taking it first fails the other way, so the changed-on-disk notice can fire once too often but never go missing.

Verified

  • verify.sh generate, verify.sh build: PASS.
  • verify.sh test FileTabBaselineTests SQLFileDeduplicationTests QueryTabManagerTests TabPersistenceCoordinatorTests QueryTabManagerCloseTests QueryTabManagerAdoptTabTests: PASS, 38 cases.
  • swiftlint --strict over every file this changes: clean. The one error the run reports is ExecutionAuditLog.swift:40, which is on main already and is not in this diff.

Five new cases in SQLFileDeduplicationTests: unsaved edits survive a reopen; a tab with no baseline is not overwritten either; a clean tab still takes the file's current text; the saved baseline moves with it so the tab is not left falsely dirty; and a pending changed-on-disk banner is cleared. The existing case that pins the clean-tab overwrite is unchanged and still passes.

FileTabBaselineTests covers the baseline itself against real files on disk: a rebuilt tab learns what its file says, one holding unsaved work reads as dirty once it has, a tab with no file and a file that cannot be read are both left alone rather than given an invented baseline, a list is covered whole, and the loader reports the date it read.

No UI automation: driving it needs a linked SQL favourite pointing at a file on disk, which the UI suite has no fixture for.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@datlechin
datlechin merged commit dc5c8f2 into main Aug 19, 2026
3 checks passed
@datlechin
datlechin deleted the fix/reopened-sql-file-keeps-edits branch August 19, 2026 07:23
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