Take the name of a file that is not there yet

[rodgco]

Fixes #6. The analysis and the shape of this fix are @Vyrnexis's, from the issue.

Backend::open only reads. A path with nothing behind it fails the ReadOnly open, reports "Could not open", and returns before setFileUrl, so the document ends up with no name at all — and Backend::save falls back to saveAsDialog(). Starting a file the way every editor is asked to start one, omawrite new_document.md, therefore met the portal's Save As dialog asking for a name that had already been given on the command line.

open now claims the URL for a blank document when the path is not there yet, so the first save lands where the writer said it should. A path that exists but cannot be read — a directory, a file without read permission — is still an error, and still leaves the document it could not replace alone.

The status line says New file new_document.md, and nothing is written until the writer saves.

Only where the file could actually be created: a name under a directory that is not there, or one the writer cannot write into, stays on master's Could not open. Otherwise the failure moves from the open to the first save, where there is no longer a Save As dialog to land in.

The guards this removes, put back

Taking the name early gives up two things master got for free, and both are restored here rather than left as the cost of the feature.

Nothing can watch a file that is not there. watchCurrentFile() only watches a path that exists, so between the open and the first save the path is unguarded: a git pull or a sync client can put a file there, and QSaveFile::commit() would replace it without a word. On master that case went through the Save As dialog, which asks before overwriting. The first save onto a path this document has never read now asks instead of writing.

The question it asks is the one the app already asks about outside changes, so it reuses that dialog — with its own wording, since nothing changed, a file appeared, and with Keep Mine leading rather than Reload. Reloading here discards everything the writer has typed against a file they have never seen, and the recovery snapshot with it, so it must not be the button that Enter presses.

A reload that loses its file must not start a new one. reloadFromDisk() went through open(), so if the file vanished between the "File changed" dialog opening and the Reload click, Reload took the absent path as a new file: document blanked, recovery cleared. open() now takes a flag; reload passes false and gets master's Could not open back.

Both guards key on a dedicated m_pathNeverRead rather than on m_hasKnownFileContents, which answers a different question — whether there is a copy to compare a watcher event against. Overloading that one turned out to be how a first attempt at this locked the writer out of their own filename: the dialog's Keep Mine could fail to clear it, and every later save asked the same question again. m_pathNeverRead is cleared by every answer the dialog can give, including the ones that fail.

Test

startsANewFileFromAPathThatIsNotThereYet drives the real Main.qml document:

• opening a path that is not there names the document, leaves it blank and unmodified, and writes nothing to disk;
• typing and saving lands on that path with no Save As dialog requested (QSignalSpy on saveDialogRequested stays at 0);
• a file that is there still opens and reads;
• a path that cannot be read still reports Could not open, keeping the previous document and its URL.

asksBeforeAFirstSaveReplacesAFileThatAppeared, asksOnlyOnceWhenWhatAppearedCannotBeRead, keepsTheDocumentWhenReloadRacesADeletion and putsKeepMineForwardWhenAFileAppeared cover the guards: that the first save onto a file that turned up asks rather than writes, that answering it once is enough even when what turned up cannot be read at all, that a refused reload keeps the document and leaves the path guarded for when the file comes back, and that Keep Mine is the button carrying focus.

Each fails with its own fix backed out, on the assertion it is named for. The unreadable case uses a directory on the path rather than a permission bit, since permissions invert under root.

Full suite in build-tests: 17 passed, 0 failed.

🤖 Generated with Claude Code

https://claude.ai/code/session_01TyQRJCyR76uk7XaNAB8jMC

[omarchybot]

Reviewed against master and ran the suite on a disposable VM: 13 passed, 0 failed, before and after the commit below.

Pushed 770f714 to this branch. The early return took any absent path, including one under a directory that is not there, or one the writer cannot write into — and that moves the failure from the open to the first save, where there is no longer a Save As fallback to land in. omawrite ~/notes/2026/draft.md with no 2026/ directory opens a document titled draft.md, and Ctrl+S answers with Could not save draft.md. in the footer and no dialog, because the URL is now valid enough to skip saveAsDialog(). A trailing-slash path is the same case and shows as Untitled.md, since fileName() is empty for it. The commit claims the name only where the file could actually be created, leaves everything else on master's Could not open, and adds one assertion to your test — which fails without the guard.

Two things I did not change, for the maintainer to decide:

• Nothing watches the path between open and the first save: watchCurrentFile() can only watch a file that already exists, and no directory is watched. If another process creates that file in the meantime — a git pull, a sync client — saveTo() replaces it via QSaveFile::commit() with no external-change dialog, because m_hasKnownFileContents is false and no watcher event was ever possible. On master that case went through the Save As dialog, which asks before overwriting. Narrow, but it is a guard this removes rather than one that was never there.
• reloadFromDisk() calls open(m_fileUrl), so if the file vanishes between the "File changed" dialog opening and the Reload click, Reload now blanks the document and clears recovery instead of reporting Could not open and leaving it alone. The dialog disables Reload when it already knows the file was deleted, so only that race reaches it.

Worth knowing before merge: #9 rewrites the same function. It routes every write of m_lastKnownFileContents/m_hasKnownFileContents through a new setKnownFileContents() that also maintains m_lastKnownFileText, the baseline its editorTextChanged compares against to clear the modified flag. This block sets those two members by hand and never touches m_lastKnownFileText, so whichever lands second, the new-file path has to call setKnownFileContents(QByteArray(), false). Left as is: open an existing file, then open a name that is not there, and the baseline stays the old file's text — typing exactly that text into the blank document clears modified, deletes the recovery snapshot, and closing the window never asks.

[rodgco]

The two guards you left for a decision, and what checking them turned up

Thank you for 770f714 — you were right that the early return took any absent path, and that moving the failure to the first save is worse than failing at the open, because by then there is no Save As to land in.

Both of the things you left for the maintainer are fixed here rather than accepted. Neither is a cost of the feature; both are guards master had, and taking the name early is what dropped them.

The unwatched path

Fixed in bf00e91. The first save onto a path this document has never read now asks instead of writing.

Getting there took two attempts, and the first one is the interesting part. It keyed the guard on !m_hasKnownFileContents, which reads naturally as "we have never read this path" — but that flag already means something else: whether there is a copy to compare a watcher event against. keepExternalVersion() sets it back to false when the disk read fails, so if what turned up was a directory, or a file writable but not readable, "Keep Mine" reported Kept your version, changed nothing, and every later Ctrl+S raised the same dialog again. Meanwhile saveAs() to the identical path succeeded on the first ask. The save refused what the filesystem granted, with no way out through the dialog it had raised.

So there is now a separate m_pathNeverRead, cleared by every answer the dialog can give — including the branches that fail. One boolean could not carry both meanings.

The second thing the first attempt got wrong: it reused the external-change dialog as-is, and onOpened focuses Reload whenever deleted is false. In this case the writer has no copy on disk at all, and reload clears the recovery snapshot too, so Enter on that dialog destroyed everything they had written. The dialog now takes an appeared flag; keepIsSafer drives both the focus and the primary button, and the wording says a file appeared rather than that one changed, and that reloading discards everything typed.

The reload race

Fixed in f4be8dc. open() takes a mayStartNewFile flag; reloadFromDisk() passes false and gets master's Could not open back. I chose the flag over an exists() check inside reloadFromDisk() because the check only moves the race one function along.

That closed half of it. After the refused reload the path is still unwatched and the document still holds a name, so if the file comes back — a git checkout, the sync client — the save guard did not fire and the returned file was replaced silently. 98f0c3f marks the path unread in exactly that case: a read failure from reload, on a file that is genuinely absent. The open() side is deliberately excluded, since its failure leaves a different document loaded under a different URL, and calling that one unread would be a lie about a file we did read.

The close latch, filed separately as #23

Worth knowing about, since this PR makes it easier to reach. When the unsaved-changes dialog's Save leads to a save that does not happen, pendingAction stays "close", and the next successful save — any later save at all — completes the close. The window shuts on a request the writer made minutes earlier and watched fail.

It is not this PR's bug: I reproduced it on master at 8f98892 through an ordinary failed save, and the backend already clears its own latch there (backend.cpp:476) while the QML side has no equivalent. But a guard that declines to write is a second way in, so it is worth having the two land in some order rather than by accident. #23 has the reproduction.

On #9

Your note was right that it rewrites this function, and wrong about which line fixes it — through no fault of yours, since e08b2ff landed on that branch about forty seconds after you wrote.

That commit added baselineKnown to editorTextChanged(), which is false for a new-file document, so the stale m_lastKnownFileText is never consulted and the scenario you described does not fire. I checked by merging the two branches on a throwaway and running your exact steps: modified stays true.

But the merge does introduce the opposite problem — omawrite fresh.md, type, delete everything, and modified stays true, which is #4's own complaint reappearing for new files. setKnownFileContents(QByteArray(), false) does not fix that, because baselineKnown is still false. What works, verified on the trial merge at 21 passing across both suites, is two lines:

// the new-file branch: the baseline is known, and it is empty
setKnownFileContents(QByteArray(), true);
m_pathNeverRead = true;

// and in #9's status branch, so a file never written is not called "Saved"
if (m_hasKnownFileContents && !m_pathNeverRead)
    setStatus(QStringLiteral("Saved %1").arg(fileName()));

That is only safe because the save guard no longer reads m_hasKnownFileContents. Whoever merges second should apply it.

Still the maintainer's call

Four things I decided one way and would change on request: the appeared-file wording; parameterising the existing dialog rather than adding a second one; leaving Reload enabled in the appeared case, on the grounds that taking what the sync client brought is a legitimate choice even here; and the underlying decision to raise the external-change dialog rather than fall back to Save As on that first save.

17 passing, bin/build clean.

[greptile-apps]

Greptile Summary

This PR lets an initially absent command-line path become the target of a new blank document while preserving unreadable-path errors and reload safety.

• Tracks whether the document has ever read its target and persists that state in recovery snapshots.
• Prompts before the first save when a file has appeared at the target.
• Adds appeared-file dialog messaging, safer focus behavior, pending-action cleanup, and lifecycle tests.

Confidence Score: 4/5

The PR should not merge until first-save conflict detection prevents a file created during the check-to-commit window from being silently replaced.

The appeared-file guard still checks target existence separately from the later save commit, so another process can create the target between those operations and have its contents overwritten without confirmation.

Files Needing Attention: src/backend.cpp

Important Files Changed

Filename	Overview
src/backend.cpp	Adds absent-path document initialization, first-save conflict handling, reload safeguards, and recovery persistence for never-read paths.
src/Main.qml	Connects appeared-file events to the external-change dialog and clears interrupted pending actions.
src/ExternalChangeDialog.qml	Adds appeared-file messaging and makes Keep Mine the primary focused action for that state.
src/backend.h	Declares the appeared-file signal, guarded open helper, and never-read path state.
tests/tst_omawrite.cpp	Adds coverage for absent targets, appeared files, reload deletion races, pending-action cleanup, dialog focus, and recovery.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Open local path] --> B{Path exists?}
    B -- Yes --> C[Read file and watch path]
    B -- No --> D{Parent writable?}
    D -- No --> E[Report open failure]
    D -- Yes --> F[Start blank named document]
    F --> G[First save]
    G --> H{Target now exists?}
    H -- Yes --> I[Ask Keep Mine or Reload]
    H -- No --> J[Save document]
    I -- Keep Mine --> J
    I -- Reload --> C

Loading

_{Reviews (2): Last reviewed commit: "Carry the never-read path through a reco..." | Re-trigger Greptile}

[greptile-apps]

Conflict strands pending close

When a close-triggered save encounters a file that appeared at the target, this branch returns without saveSucceeded; choosing Keep Mine only records the decision, so the pending close remains unfinished until the user manually saves again.

Knowledge Base Used:

• Document backend and filesystem lifecycle
• Editor dialogs and reusable controls

[omarchybot]

Re-reviewed at 98f0c3f, the three new commits included. Both Greptile P1s on src/backend.cpp:273-278 hold up — one as a residual, one as a real defect — and there is a third the second reviewer turned up. Reviewed by Claude Opus 5 and by Codex at xhigh reasoning, both against the tree at 98f0c3f.

The guard does what it claims, and it does not fire on the ordinary case

omawrite fresh.md on a path that is not there still writes on the first Ctrl+S with no dialog, because the guard needs QFileInfo::exists() to be true as well as the flag. startsANewFileFromAPathThatIsNotThereYet covers exactly that and passes.

The TOCTOU is real, and not a reason to hold the PR

I checked what QSaveFile does rather than what the docs say, on a Qt 6.11.2 build: commit() on a target that appeared after open() renames over it, returns true, and the inode changes. The arrived file is gone with no error. The mechanism is as described.

What makes it a residual is the size of what it replaced. Without the guard the window is the whole span between taking the name and the first save, which can be an afternoon; with it, the window is the existence check to the rename.

It is also not closable inside QSaveFile. QSaveFile::open(WriteOnly | NewOnly) returns false on an absent target too — Qt rejects the mode outright rather than giving O_EXCL semantics. Plain QFile does honour NewOnly (false on an existing path, true on an absent one), so the race could be closed by creating that first file exclusively and writing through it, at the cost of the atomic replace for the one write where there is nothing yet to replace. renameat2(RENAME_NOREPLACE) would keep both and has no Qt API. Either is a change to the save path and a decision rather than a fix, so I left it.

The close strand is real, and worse than the bot described — fixed in b0bf1cc

Built and ran it. With pendingAction at "close", the guarded save emits externalFileAppeared and no saveSucceeded, and pendingAction stays "close" through Keep Mine, through Reload, and through Escape. The window does not close. Then the next successful save — an ordinary Ctrl+S later — fires saveSucceeded, completePendingAction() runs, and the window closes on the earlier request.

You are right that this is #23 and predates the branch: I reproduced it on origin/master at 8f98892 through a failed save, and the next successful save closed the window there too. The latch is not yours. But this branch opens a second route into it that is much easier to reach than a failed save, so b0bf1cc drops the pending action where the question is raised, in onExternalFileAppeared. It does not fix #23's general case, which still needs its own repair.

Recovery loses the never-read state — fixed in df3274c

This one is codex's. writeRecovery() stores the URL and the text; restoreRecovery() reads whatever is on the path now and clears m_pathNeverRead on the strength of that read. Start fresh.md, type, crash, let something create fresh.md while Omawrite is down, restart: the arrived file is read only as the comparison baseline, the guard never fires, and Ctrl+S replaces it. I ran it — the file ended up holding the recovered draft. df3274c puts the flag in the snapshot; a snapshot without the key reads as false, which is what it meant before this branch existed. The unreadable branch still sets it true regardless, since a path that cannot be read is unverified whatever the snapshot said.

One left for you, because the obvious fix is wrong

Choosing Reload when what appeared cannot be read — a directory, or a file you may write but not read — loops. openPath(..., false) fails; the path still exists so backend.cpp:241 does not touch the flag; nothing else clears it; every later Ctrl+S raises the same dialog. I measured three in a row.

It is the mirror of the case bf00e91 fixed for Keep Mine, but clearing the flag on a failed reload would be wrong: the writer has not said to keep their version, and an unreadable file can still be writable, so the next save would replace it in silence. Keep Mine is the way out and it works. Worth deciding rather than reflexively symmetrising.

Tests

On a disposable worker, never on the machine holding credentials. bin/test 19 passing at the pushed head (your 17 plus 2), bin/build clean, Qt 6.11.2. Both new tests were checked by mutation — reverting each fix turns its own test red and nothing else. b0bf1cc is green on its own.

Codex also noted that putsKeepMineForwardWhenAFileAppeared proves less than it reads like: it checks the buttons' primary styling but never opens the dialog, so a regression in the onOpened focus line would keep it green while Enter went back to Reload. Not worth blocking on, but it is the one new assertion that could pass for the wrong reason. Where codex agreed with conclusions already reached here its independence is not currently guaranteed — it can read this session's own working files — but the recovery finding and that observation are its own.

On #9, and on #22

Still reconciled, but the merge-order note has moved since the last pass. At this head the two no longer merge cleanly: three conflicts in backend.cpp, all mechanical — #9's setKnownFileContents(...) beside this branch's m_pathNeverRead line. Resolved that way, 20 pass across both suites and the data-loss shape does not fire: baselineKnown is false for a document with a file URL and no baseline, so emptying the editor leaves modified true.

Your two suggested lines check out. I applied them on the trial merge — setKnownFileContents(QByteArray(), true) with m_pathNeverRead = true, and && !m_pathNeverRead on #9's status branch. 20 still pass, emptying a new file goes clean with an empty status rather than Saved fresh.md, and the unknown-baseline case still stays modified. Whoever merges second wants those.

Worth knowing about #22: its autosave writes through saveToItsOwnFile() → saveTo(m_fileUrl), which never enters save() and so never sees this guard. If both land, autosave would replace an appeared file on a never-read path on a timer, without anyone touching the keyboard. Whichever goes second needs the guard on that path too.

Waiting on the maintainer for the TOCTOU decision, the failed-Reload loop, and the order of #13, #9 and #22.

[omarchybot]

Re-reviewed at df3274c. No new commits from you since the last pass, so this is one thing the second opinion found in a commit of mine, plus an answer worth stating flatly. Reviewed by Claude Opus 5 and by Codex at xhigh reasoning, both against this head.

The recovery fix was untested in the direction that matters — fixed in 6bc0092

df3274c put pathNeverRead into the recovery snapshot, and the test I added with it, remembersANeverReadPathAcrossRecovery, hand-writes that JSON. So it exercises restoreRecovery() and nothing else. Proved on a worker: delete the key from writeRecovery() (backend.cpp:595) and all 19 tests stay green, while the data-loss path the commit exists for reopens — crash on a never-read path, something creates the file while Omawrite is down, and the first save after the restart replaces it, because a snapshot without the key restores as false. My claim last time that both new tests were mutation-checked was true only for reverting a whole commit; half of that one could be reverted in silence.

6bc0092 adds writesTheNeverReadPathIntoTheSnapshot, which drives the real writer — open an absent path, type, wait for the recovery timer — and reads the file back. Removing the key fails it, writing a constant false fails it, and nothing else in the suite moves either way. That is codex's finding; it is the one thing here neither reviewer had already written down.

Nothing checks the target's existence at write time

Worth saying plainly, because it is the whole shape of the residual TOCTOU rather than a detail of it. saveTo() (backend.cpp:525-574) rejects non-local URLs, opens a QSaveFile, writes, and commits. There is no existence check and no no-replace condition anywhere in it, and commit() renames over whatever is on the path. The only existence check in the entire write path is save():273-274, which runs before the call — check-then-act by construction, so the guard is a narrowing of the window and never a closing of it.

There are two entries to saveTo() and only one is guarded. save() carries the appeared-file check; saveAs() (backend.cpp:297-298) calls saveTo() directly with no m_pathNeverRead test and no existence test, so its only protection is the file picker's overwrite prompt, which is another process and happens well before the write. That is master's behaviour rather than something this branch introduced, but it means the guard covers Ctrl+S and not Ctrl+Shift+S onto the same path — worth knowing when deciding how far to take the fix.

Greptile's two P1s, at this head

Both were raised against 98f0c3f, before the two commits I pushed on the 25th. Non-atomic appeared-file guard holds; it is the residual above, and it is a decision about the save path rather than a bug in this branch. Conflict strands pending close does not hold at this head — b0bf1cc clears pendingAction and awaitingPendingSave in onExternalFileAppeared, and dropsThePendingCloseWhenTheSaveIsRefused covers it. If you are looking at two open P1s, one of them is already answered.

Codex also read asksOnlyOnceWhenWhatAppearedCannotBeRead as proving less than its name claims. I checked and rejected that: line 336 asserts the second save reports Could not save blocked.md., which only happens through the branch the test is about.

Tests

On a disposable worker, never on the machine holding credentials. bin/test 19 passing at df3274c and 20 at 6bc0092, bin/build clean, Qt 6. Where codex agreed with conclusions already reached here its independence is not currently guaranteed — it can read this session's own working files — but the coverage gap above is its own, and it is confirmed by mutation rather than by agreement.

Still waiting on the maintainer, unchanged from the last pass: the write-time race, the failed-Reload loop, and the order of #13, #9 and #22.