[BUG] External file updates lost during Capture prompt

[LER0ever]

Describe the bug

Quickadd currently performs a full read of the target file upon command activation, and prompts for user input, and finally formats the input to target file. During the wait for input stage, which typically takes several seconds for user to type something, any external updates (typically from Obsidian Sync) to that file is lost. Quickadd then overwrites the file with updated ones based on the old version.

This is a followup to a bug report to Obsidian Sync that I submitted earlier to the Obsidian Forum. In that thread, @lishid pointed out that it could be one of the plugins overwriting the file with outdated content, which made me realize every time that Sync bug happens I was always using Quickadd to write something.

Cause

Current function call stack when triggering the Quickadd capture is as follows:

src/engine/CaptureChoiceEngine.ts: async run(): Promise<void>

const filePath = await this.getFilePath(captureTo);
let content = await this.getCaptureContent();
let file: TFile;

if (await this.fileExists(filePath)) {
    file = await this.getFileByPath(filePath);
    if (!file) return;

    const fileContent: string = await this.app.vault.read(file);
    const newFileContent: string = await this.formatter.formatContentWithFile(content, this.choice, fileContent, file);
    // the above function waits for user input. 
    await this.app.vault.modify(file, newFileContent);
}

And the this.formatter.formatContentWithFile would call into promptForValue() down the road, which creates an InputPrompt and waits for user input. Depending on how much text the user want to type, this would take several seconds to several minutes, and we definitely don't want to discard external updates to the file during this period.

A potential improvement

To avoid race condition, and minimize the possibility of simultaneous update to the same file, we'd like to minimize the duration between file reads and file writes, i.e. we'd like to make sure the update we perform is based on the latest version.

Current call sequence

• read file content
• format file content
  • prompt and wait for user input (long running)
  • process user input and format to file
  • return updated file
• write updated file to vault

Suggested call sequence

• prompt and wait for user input (long running)
• read file content
• format file content with user input
• write updated file to vault

To Reproduce

The common use case to trigger this issue:

0. iOS and Mac has note xxx.md at version 1
1. Edit note (xxx.md) on Mac to make it version 2, Sync uploads the file
2. Open iOS Obsidian
3. Obsidian Sync tries to pull updates (version 2) from server
4. Start QuickAdd capture from command, and start to write, QuickAdd reads file at version 1
5. Obsidian Sync finishes the download, and updates the file xxx.md to version 2
6. Finishes writing, click OK on QuickAdd prompt
7. QuickAdd writes the formatted / updated file to vault, making it a new version (it's version 3 to Sync)
8. The version 3 is synced across all device, version 2 update is lost

This is 100% consistently reproducible.

Expected behavior

If the file we want to edit is no longer the latest version, it should at least trigger the obsidian's conflict resolution, and should never nuke any file updates.

Additional context

Relevant Obsidian Sync Bug Report: https://forum.obsidian.md/t/obsidian-sync-updates-from-one-device-overwritten-by-another/33007

[lishid]

That's such a great bug report!

[alexlyzhov]

Can confirm. Constantly had this happening, I thought it's a Sync problem but have now learned it's attributable to QuickAdd.

[claremacrae]

Just an observer, but I’m trying to understand how the expected order of operations would work in practice?

How could it know the right position to insert the new text, if it reads the file after the user has given their input?

is it that Obsidian retains the cursor position even if the file changes due to sync? Should the list of expected order of operations include steps for when to read the cursor position?

Perhaps the plugin could include some checks on completion like:

• is the cursor on the same word
• Is the selection text unchanged?

[chhoumann]

This is indeed a fantastic bug report. Thank you, @LER0ever!

I have implemented a fix which will be included in the next release. There are further optimizations to be made, but I wanted to prevent this kind of data loss from this point on.

Thank you, everyone, for your input here! It has been very valuable in diagnosing & fixing this issue.

And for those interested:

1. I've implemented a 'first pass' over format syntax, which will do most of the prompting & therefore take most of the time. We do not read the file at this point.
2. Then, a second pass does the inserting of the formatted capture content into the file contents. Only here do we read the file.
3. We read the file again to check for external file updates. If there are any, we attempt a 3-way merge. If this is not possible, QuickAdd will not modify the file.

Currently, the only case I see when the second pass would take long is if the user writes e.g. {{VALUE:something}} inside their file manually, and then captures to it. This would make the second pass prompt the user for the value of something.

In the future, I'm looking to iterate and improve on this. Pending a rewrite and decoupling of some classes, so this will happen when that happens.

[github-actions[bot]]

:tada: This issue has been resolved in version 0.11.5 :tada:

The release is available on GitHub release

Your semantic-release bot :package::rocket: