Skip to content

[Website] Hide incomplete autosaves from recovery - #4244

Draft
ashfame wants to merge 2 commits into
trunkfrom
agent/incomplete-autosave-recovery-test
Draft

[Website] Hide incomplete autosaves from recovery#4244
ashfame wants to merge 2 commits into
trunkfrom
agent/incomplete-autosave-recovery-test

Conversation

@ashfame

@ashfame ashfame commented Aug 3, 2026

Copy link
Copy Markdown
Member

What

  • Add a shared lifecycle check for an initial OPFS copy observed unfinished by a later document.
  • Exclude those incomplete autosaves from Recent and matching-setup recovery suggestions while keeping same-document autosaves that are still copying eligible.
  • Reuse the same check in the boot path so restore eligibility and boot eligibility cannot disagree.
  • Add unit coverage for the eligibility distinction and deterministic Chromium coverage for the recovery prompt.

Why

Playground persists autosave metadata before its initial filesystem copy completes. If another same-origin document opens before that copy finishes, it loads a record with initialOpfsSyncPending: true. The creating document may have stopped, or it may still be syncing in another tab. In either case, the new document cannot boot the record yet.

The boot path correctly refuses that incomplete record, but the recovery classifier still offered it as a “Recent autosaved Playground.” Because the prompt waits for the replacement Playground client, it appeared after the new Playground finished loading and looked like a recovery offer created during the user's first successful visit.

This keeps unfinished records loaded from storage out of recovery surfaces instead of offering a Restore action that cannot currently succeed. Same-document first copies remain unaffected because loadedFromStorage distinguishes them from records discovered by a later document.

Browser lifecycle validation

The browser-owned sequence was reproduced against the deployed code without this fix on both playground.wordpress.net and pr4220.pg.ashfame.com. Chrome's explicit urgent-discard action discarded the real hidden Playground tab after its pending metadata had been written; activating the same tab created a replacement document with document.wasDiscarded === true, navigation type back_forward, and the recovery prompt visible. No application reload, tab close, second manually opened Playground tab, or metadata injection was used.

System Firefox 153's native TabUnloader produced the same result on the unfixed official deployment. Firefox marked the background Playground tab pending, and selecting it created a new document with navigation type back_forward and the recovery prompt. Firefox does not expose document.wasDiscarded in this path.

These explicit browser actions prove that tab discard/unload is sufficient, but they do not attribute the reporter's historical occurrence. Chrome's urgent-discard action can bypass normal eligibility. Firefox automatic low-memory unloading is disabled by default on Linux, excludes the selected tab, and normally protects tabs accessed within ten minutes; the validation deliberately bypassed that time threshold. WebKit separately has an automatic reload path after memory/CPU termination, a crash, an unresponsive process, or termination requested by its network/GPU/model process, including when the page is visible. The eligibility fix is intentionally independent of the browser's interruption reason.

Pre-script instrumentation of fresh unfixed playground.wordpress.net/ boots found one top-level document, no History API mutation, no URL change, and no application reload. Source inspection also rules out a normal same-document recovery-effect rerun for /: the Dock's startup replaceState path only removes an existing ?overlay=... parameter, while initial autosave activation uses updateUrl: false.

Memory profiling makes WebKit's foreground termination path plausible. Two official-host autosave boots peaked at 670,092 KiB and 676,056 KiB renderer RSS; ?storage=temp controls peaked at 564,336 KiB and 582,580 KiB. The autosave path added about 91–103 MiB renderer RSS and 73–88 MiB browser RSS during the pending initial-copy interval. Profiling the direct pre-Dock parent commit found the same pattern: normal boots peaked at 630,312–635,432 KiB renderer RSS versus 546,772 KiB for the temporary-site control, with another 80–82 MiB autosave delta in the browser process. The memory-heavy initial copy and vulnerable lifecycle window therefore predate the Dock deployment.

Fixes #4243.

Testing

  • npm exec nx test playground-website --testFile=slice-sites.spec.ts
  • npm exec nx run playground-website:e2e:playwright --args='packages/playground/website/playwright/e2e/opfs.spec.ts --project=chromium --grep "initial OPFS sync" --retries=0 --workers=1'
  • npm exec nx run-many -t lint typecheck -p playground-website --parallel=2
  • npm run format:uncommitted
  • git diff --check
  • Manual Chrome discard reproduction on the unfixed official and preview deployments
  • Manual Firefox TabUnloader reproduction on the unfixed official deployment
  • Top-document lifecycle and memory profiling on the unfixed official deployment
  • Comparative normal-versus-temporary memory profiling on the pre-Dock build

) {
// If the initial OPFS sync was interrupted, the site files are incomplete
// and we can't boot this site.
} else if (hasUnfinishedInitialOpfsSyncFromStorage(site)) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I love this function name @ashfame. I know many people who would say it's too long and we should find a shorter one, but I really like it. It tells me everything I need to know about that operation without actually reading the function body ❤️

@adamziel

adamziel commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

Good work @ashfame! I'd brush up the description to be as short as possible (but no shorter) so we can come back to this in a year and decide with a glance if this PR is relevant to our debugging or not.

@adamziel adamziel left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM, nice tests, too! We just need a shorter description.

@adamziel

adamziel commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

Actually, the skipped CI job logs:

layground storage › should not show autosave status when embedded in an iframe (retry #1) (667ms)
✘ 113 [chromium] › packages/playground/website/playwright/e2e/website-ui.spec.ts:3413:6 › Default Playground storage › should keep a Playground saved after saving from the restore nudge state (retry #1) (1.2m)

Which we should fix before merging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Website] Incomplete first-boot autosave is offered as recoverable

2 participants