[BUG]: Checklists and notes can be silently replaced by an empty uuid-only stub (non-atomic write + read errors returned as "")

[pedwards3x]

Describe the bug

A checklist (or note) can be silently replaced by an empty stub containing only a freshly generated uuid — no title, no checklistType, no items. The list still exists in the UI, but it is empty, and the original content is gone from disk with no error logged.

This happened twice to one list on my instance. Both times it followed ticking an item off.

Root cause

Three behaviours combine:

1. serverWriteFile is not atomic — app/_server/actions/file/index.ts

export const serverWriteFile = async (filePath: string, content: string) => {
  await ensureDir(path.dirname(filePath));
  await fs.writeFile(filePath, content, "utf-8");
};

fs.writeFile opens with O_TRUNC, so the file is zero bytes from the truncate until the write completes.

2. serverReadFile converts every failure into an empty string — same file

export const serverReadFile = async (filePath, customReturn?): Promise<string> => {
  try {
    return (await fs.readFile(filePath, "utf-8")) || "";
  } catch (error) {
    return customReturn || "";
  }
};

A read that lands in the truncation window is indistinguishable from a missing file.

3. The readers self-heal a missing uuid by writing it back — app/_server/actions/checklist/readers.ts (and note/readers.ts)

const content = await serverReadFile(filePath);
if (isRaw) {
  const { metadata } = extractYamlMetadata(content);
  let uuid = metadata.uuid;
  if (!uuid) {
    uuid = generateUuid();
    const updatedContent = updateYamlMetadata(content, { uuid });
    await serverWriteFile(filePath, updatedContent);   // ← writes the empty read back
  }

Given content === "", extractYamlMetadata returns {}, so there is no uuid, so a new one is minted and updateYamlMetadata("", { uuid }) produces ---\nuuid: <new>\n---\n — which is then written over the real file.

What closes the loop: updateItem calls broadcast({ type: "checklist", action: "updated", ... }) immediately after serverWriteFile. Every connected client re-reads the list at once, and queries.ts does that with isRaw: true — the exact branch that performs the self-heal write. Ticking a checkbox fires the write that races the reads it triggers. More clients or devices open on the list widens the window.

Evidence

The damaged file was exactly 51 bytes:

---
uuid: 90bebaec-330a-43ab-a96e-7247063a9757
---

Reconstructing what this path emits from an empty read gives 51 bytes, byte-for-byte identical.

listToMarkdown — the normal serializer — cannot produce that file:

metadata.title = list.title || "Untitled Checklist";   // always emits a title
...
if (list.items.length === 0) return frontmatter.trim();  // no trailing newline

It always writes a title: line, and it trims for an empty list, which would give 50 bytes with no trailing newline. The damaged file had the trailing newline and no title at all. Only the updateYamlMetadata("", { uuid }) path matches.

The uuid also changed, which an update path would never do — confirming the file was rewritten as a brand-new object rather than modified.

To Reproduce

Timing-dependent, so it is intermittent by nature:

1. Open the same checklist in two or more clients (two browsers, or phone + desktop).
2. Tick items off in fairly quick succession.
3. Occasionally the write and the broadcast-triggered re-read overlap, and the list is replaced by a uuid-only stub.

Expected behavior

A read failure should never cause a write. A concurrent reader should never observe a partially written file.

Suggested fix

Two small changes, either of which alone prevents the loss; together they are belt and braces.

Make the write atomic — write to a temp file and rename(2) it into place. rename is atomic within a filesystem, so a reader sees either the old file or the new one:

const tmp = `${filePath}.${process.pid}.${Date.now()}.tmp`;
let mode: number | undefined;
try { mode = (await fs.stat(filePath)).mode & 0o777; } catch { /* new file */ }
try {
  await fs.writeFile(tmp, content, "utf-8");
  if (mode !== undefined) await fs.chmod(tmp, mode);
  await fs.rename(tmp, filePath);
} catch (error) {
  try { await fs.unlink(tmp); } catch {}
  throw error;
}

Guard the self-heal in both checklist/readers.ts and note/readers.ts:

if (!uuid && content.trim().length > 0) {

It may also be worth reconsidering serverReadFile swallowing real errors — "file is missing" and "the read failed" are very different facts, and collapsing them is what makes this reachable.

Version

Reproduced on 1.20.0. Verified still present on main at 1.27.0 — serverReadFile and serverWriteFile are unchanged, and the self-heal write is at checklist/readers.ts:245.

Additional context

I am running the two fixes above as a local patch and have had no recurrence. Happy to open a PR if useful.

[fccview]

Hey thank you for the very detailed report!

If you feel like opening a pr to develop please feel free, otherwise I'll take your patches, test it and add it to next release as soon as I find a moment ❤️

[pedwards3x]

Full transparency, this is from Claude code as I am not a coder by nature or trade, just an IT guy who loves this project 😀. I would go the route of the team taking the patches and vetting them for the next release if that's alright with you. Thanks again!

[pedwards3x]

Follow-up with findings verified against 1.27.0 specifically — my original report was written against 1.20.0 with only a spot-check of main, and 1.27.0 turns out to differ in ways that matter.

Same disclosure as above: this analysis is from Claude Code. It is static analysis plus a locally running patched build, not a reproduction of the race.

Already handled upstream

Credit where it is due — two of the stamp sites are already guarded in 1.27.0:

• app/_server/actions/checklist/readers.ts:35 _stampUuid — if (!content) return undefined at :39, wrapped in singleFlight
• app/_server/actions/note/readers.ts:32 _stampUuid — same guard at :36

Those two are fine as they stand.

(For anyone chasing this: note/readers.ts:221 and :253 are not stamp sites. They are defensive checks after _stampUuid returns, and they write nothing.)

Still unguarded in 1.27.0

Four sites still stamp a uuid onto whatever content holds, with no empty-content check:

#	Location	Writes via	Guard needed
1	checklist/readers.ts:245 — inline self-heal in readListsRecursively	serverWriteFile	if (!uuid && content.trim().length > 0)
2	note/readers.ts:277 — inline self-heal in readNotesRecursively	direct fs.writeFile	if (!uuid && content.trim().length > 0)
3	lib/legacy-lookup.ts:94 in _uuidFor (:71)	direct fs.writeFile	if (!content.trim()) return null;
4	migration/share-migration.ts:111 in _stampItems (:101)	direct fs.writeFile	if (!content.trim()) continue;

Two notes on those:

#3 legacy-lookup.ts reads with fs.readFile rather than serverReadFile, so a read error throws and is caught by the outer handler, which returns null — that part is fine. The gap is that an empty read succeeds. A zero-length file has no uuid, so it gets stamped and written back as a stub. Signature is Promise<string | null>, hence return null rather than undefined.

#4 share-migration.ts is the one I would prioritise. It has the same shape, but _stampItems runs during the 1.26.0 folder-sharing migration, across every .md file a user owns. That is the highest-stakes moment for this bug: a bulk pass over the whole dataset, on a release that explicitly tells people to back up first.

Why atomicity alone is not sufficient

My original report framed the fix as "make serverWriteFile atomic and the rest follows". That is not quite right for 1.27.0.

serverWriteFile:143 is still a truncating fs.writeFile, and making it atomic is worth doing — the main item-save path (checklist-item/crud.ts:137, :326, :423) goes through it, so that closes the window that produced my original data loss.

But three of the four sites above bypass serverWriteFile entirely and call fs.writeFile directly. So do other writers, including reminders/scanner.ts:145, which writes a whole checklist via listToMarkdown non-atomically and can therefore create the truncation window that another reader then observes.

So the two halves do different jobs:

• Atomic serverWriteFile removes the main source of empty reads.
• The per-site empty-content guards are what actually make the destructive write unreachable, including on paths that never touch serverWriteFile.

If only one goes in, the guards are the higher-value half — they are four one-line changes and they fail safe.

What I am running

All five changes (atomic serverWriteFile plus the four guards) on a local 1.27.0 build, no recurrence. Happy to hand over the diff in whatever form is easiest, or to answer questions on any specific site — but as said above, I would rather the team vet and implement than take a PR from me.