Stop a pending draft autosave from undoing a note save

Merged
FR
2h ago
+41-3

What & why

Saving a note from the composer right after typing could bring the saved text back as a draft on the next load.

Saving a note clears the server draft in the same request. But the composer only cancelled its debounced (1s) draft autosave once the save's response came back. An edit made within that 1s before Save fired the autosave mid-save. If that write reached the server after the clear, the draft came back. A tab hidden mid-save could do the same via the sendBeacon flush, since the composer was still marked dirty.

This is what flaked note-compose.spec.ts › "auto-saves the note as a server-side draft, repopulated on load" in CI. In that test, the fresh tab's editor re-emits a loaded draft once on mount (it adds two trailing newlines), which schedules an autosave. The failing CI value was exactly that re-emitted string, not the first tab's text. The first tab's context.close() sends no beacon at all: checked directly, Playwright's context close fires neither pagehide nor visibilitychange.

Fix, in note-composer.tsx, before creating the note:

  • Save cancels the pending autosave.
  • It clears the dirty flag, so a mid-save tab hide sends no beacon.
  • It waits for every draft write already sent to finish, for at most 3s so a stalled one can't block the save.
  • The composer is read-only while saving, since the save sends the click-time text and then empties the composer.

If the save fails, the text is still in the composer, so autosave resumes.

Checklist

  • User-facing change → added/updated a PostHog event (and a funnel/insight for a new flow), or N/A: N/A, a bug fix with no new action to track
  • User-facing or service change → docs updated (README features/roadmap/services, .env.example, in-app help, CONTRIBUTING.md, onboarding checklist), or N/A: N/A, the documented behavior (saving clears the draft) is unchanged; this makes it hold
  • New error paths report via captureServerException / the error boundary, or N/A: N/A, no new error paths
  • New behavior is covered by tests, or N/A: the existing E2E test now edits right before Save and holds the save's response 2s, past the debounce. On main it fails every time with the CI symptom; with the fix it passes.
  • User-facing change → demo video attached (record-pr-demo skill); refactor through a user flow → parity run, or N/A: before and after below
  • If this fixes a recurring defect, considered a guardrail via the learn skill, or N/A: the regression test above is the guardrail; this isn't a pattern repeated elsewhere

Demo

Both takes simulate a slow connection inside the page, holding the save's response for 2s, so the race shows reliably. In production it needs Save to respond more than ~1s after the last keystroke.

❌ Before

https://github.com/user-attachments/assets/8e8e4ebd-a5ca-4ef0-be2f-fa0f600055bb

✅ After

https://github.com/user-attachments/assets/13dea3a3-fbbf-4510-a694-1ade97f4af3b

Notes

Verified locally:

  • bun run check:fix is clean.
  • Dashboard and note unit tests pass.
  • bun run test:e2e e2e/note-compose.spec.ts passes 3/3 full runs with the fix, and again after the review follow-ups. The updated test fails on main on every run.

Not verified: the save-failure path (autosave resuming) isn't exercised by a test.

Left for follow-ups, out of scope here:

  • Loading a draft typed in another tab rewrites it once with two extra trailing newlines. It's a redundant write; it doesn't keep growing.
  • Closing a tab sends the beacon twice (pagehide and visibilitychange).