Skip to content

Stop a pending draft autosave from undoing a note save - #702

Merged
fredrivett merged 2 commits into
mainfrom
fredrivett/note-save-draft-race
Nov 14, 2026
Merged

fredrivett merged 2 commits into
mainfrom
fredrivett/note-save-draft-race

Conversation

@fredrivett

@fredrivett fredrivett commented Nov 14, 2026 •

Copy link
Copy Markdown
Owner

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

pr-draft-race-before.mp4

✅ After

pr-draft-race-after.mp4

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).

Saving a note clears its server draft, but the composer only cancelled its
debounced draft write once the save's response came back. An edit made within
the 1s debounce before Save fired that write mid-save, and if it landed after
the clear the saved text came back as a draft on the next load. This flaked
the note-compose E2E test, where reloading a draft re-emits it once on mount.

Save now drops the pending write, stops a mid-save tab hide from beaconing it,
and waits for a write already in flight before creating the note. The E2E
test edits right before Save and holds the save's response past the debounce,
so it fails every time without the fix.

Amp-Thread-ID: https://ampcode.com/threads/T-01a1dc3f-f20d-736a-80f8-8b811bc505b5
@vercel

vercel Bot commented Nov 14, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated
abode Ignored Ignored Preview Nov 14, 2026 11:28pm UTC

Request Review

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 2 files

Reply with feedback, questions, or to request a fix.

View guided diff | Turn on auto-fix | Re-trigger cubic

…s mid-save

Review follow-ups:
- Overlapping draft autosaves are all awaited, not just the latest.
- The wait is capped at 3s so a stalled draft write can't block the save.
- The composer is read-only while saving; the save sends the click-time text
  and then empties the composer, so an edit made meanwhile was lost (and could
  schedule a draft write after the clear).

Amp-Thread-ID: https://ampcode.com/threads/T-01a1dc3f-f20d-736a-80f8-8b811bc505b5
@fredrivett
fredrivett merged commit 64650d4 into main Nov 14, 2026
14 checks passed
@fredrivett
fredrivett deleted the fredrivett/note-save-draft-race branch November 14, 2026 23:41
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.

2 participants