Move all documents into a platform-provided document store

[timon-schelling]

No description provided.

[cubic-dev-ai]

9 issues found across 41 files

Confidence score: 2/5

• frontend/src/utility-functions/persistence.ts can delete IndexedDB metadata while an OPFS removal fails, allowing Clear Saved Documents to leave files that silently return on the next launch; surface or reconcile removal failures instead of broadly swallowing them.
• editor/src/messages/portfolio/portfolio_message_handler.rs has stale-state races: old listings can reopen documents closed by CloseDocument/CloseAllDocuments, and delayed GDD setup can re-enable storage after switching to legacy mode; invalidate listings and re-check the current save mode before applying async results.
• The migration paths in desktop/src/persist.rs and frontend/src/utility-functions/persistence.ts can resurrect orphaned autosaves or propagate storage/transaction failures without a controlled fallback; filter migrated files against current metadata and handle migration failures explicitly.
• Storage failure handling needs attention across editor/src/messages/portfolio/portfolio_message_handler.rs and editor/src/messages/portfolio/document_storage_io.rs: the one-shot store_unavailable latch disables later GC, rebuild cleanup misses nested resources, and a default MemoryStore can make autosaves appear successful while losing them on exit; add retry/recursive cleanup and fail loudly when the real store is not configured.

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="desktop/src/persist.rs">

<violation number="1" location="desktop/src/persist.rs:62">
P2: This migration resurrects orphaned autosave files that the previous persistence code deliberately garbage-collected, so deleted or stale documents can reappear in the stored-document list. Filter migration against the persisted document IDs or retain equivalent orphan cleanup before exposing the new store.</violation>
</file>

<file name="editor/src/messages/portfolio/document_storage_io.rs">

<violation number="1" location="editor/src/messages/portfolio/document_storage_io.rs:43">
P2: `DocumentStoreHandle::default()` silently installs an in-memory `MemoryStore`, so any construction path that forgets `set_document_store` will autosave successfully and lose everything on exit with no error. Consider making the non-memory store a required constructor argument (drop the `Default` for `PortfolioMessageHandler`/`DocumentStoreHandle`) or at least log a warning when a default memory store is in use outside tests.</violation>

<violation number="2" location="editor/src/messages/portfolio/document_storage_io.rs:225">
P2: The rebuild cleanup does not remove nested resource files because `list("")` is non-recursive. Enumerate and remove entries under `resources/` as well, or recreate the container before writing the replacement storage.</violation>
</file>

<file name="frontend/src/utility-functions/persistence.ts">

<violation number="1" location="frontend/src/utility-functions/persistence.ts:50">
P2: Any rejection inside `migrateDocumentsToStore` (e.g., `navigator.storage.getDirectory()` unavailable in private browsing, quota errors, or a failing IndexedDB transaction in `markDocumentMigrated`) propagates out of `readPersistedState`, so `editor.loadPersistedState(...)` never runs and the error surfaces as an unhandled rejection. Migration failure should be logged and isolated, not block the startup persistence load.</violation>

<violation number="2" location="frontend/src/utility-functions/persistence.ts:125">
P2: This catch hides OPFS removal failures after the IndexedDB metadata is already deleted, so Clear Saved Documents can leave documents behind and silently restore them on the next launch. Ignore only a missing directory; surface or retry other failures, and avoid removing the metadata until storage cleanup succeeds.</violation>
</file>

<file name="document/container/src/backends/opfs.rs">

<violation number="1" location="document/container/src/backends/opfs.rs:277">
P2: This delete path also handles resource garbage collection, so it should not show the autosave failure alert for every delete error. Log deletion failures separately or use a storage-generic alert instead.

(Based on your team's feedback about reserving error reporting for appropriately severe and relevant failures.)</violation>
</file>

<file name="editor/src/messages/portfolio/portfolio_message_handler.rs">

<violation number="1" location="editor/src/messages/portfolio/portfolio_message_handler.rs:396">
P2: `store_unavailable` is a one-shot latch: after a single failed `list()`, resource GC is disabled for the rest of the session and no retry ever runs. Meanwhile `load_document` keeps queueing `attach_container` for new documents, so every subsequent document creation shows the storage-failure dialog again. Reset the flag (or gate `attach_container` on it) and re-attempt the listing, e.g. on the next `LoadPersistedState`, so a transient store failure (OPFS not ready, permissions) does not permanently disable GC and autosave-availability messaging.</violation>

<violation number="2" location="editor/src/messages/portfolio/portfolio_message_handler.rs:406">
P2: A stale store listing can reopen documents after `CloseDocument` or `CloseAllDocuments` removes them. Ignore IDs closed since the listing began, or invalidate and refresh listings after document-removal mutations.</violation>

<violation number="3" location="editor/src/messages/portfolio/portfolio_message_handler.rs:456">
P2: A stale asynchronous completion can re-enable GDD storage after the user switched back to legacy mode. Re-check `preferences.save_as_gdd` before attaching the returned GDD (or associate the completion with a mode generation).</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

1 issue found across 5 files (changes from recent commits).

Confidence score: 4/5

• In document/container/src/backends/opfs.rs, the early return treats every NotFoundError from queued Write/Append mutations as an expected deletion race, suppressing the debounced “Autosave is not working” alert for other failures; narrow the filter to the intended document-deletion case, matching the existing Mutation::Delete handling.

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="document/container/src/backends/opfs.rs">

<violation number="1" location="document/container/src/backends/opfs.rs:49">
P2: This early return suppresses the debounced "Autosave is not working" alert for every NotFoundError on queued Write/Append mutations, not just the intended document-deletion race. `Mutation::Delete` already filters NotFound upstream, so this branch is effectively write/append-only; for those, `write_file`/`append_file` use create=true, so a NotFoundError always comes from the cached container handle being stale — which also happens when the browser evicts or clears OPFS storage while a document is still open. In that case every autosave write now fails silently with only a warn (and the misleading "the container no longer exists" message, plus one warn per failed mutation with no debounce) and the user keeps editing unaware that autosave has stopped. Consider gating the alert suppression on the container having been explicitly removed (e.g., a flag on `Inner` set when the document's directory is deleted/closed), or at minimum correcting the log message and deduplicating the warns.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

P2: This early return suppresses the debounced "Autosave is not working" alert for every NotFoundError on queued Write/Append mutations, not just the intended document-deletion race. Mutation::Delete already filters NotFound upstream, so this branch is effectively write/append-only; for those, write_file/append_file use create=true, so a NotFoundError always comes from the cached container handle being stale — which also happens when the browser evicts or clears OPFS storage while a document is still open. In that case every autosave write now fails silently with only a warn (and the misleading "the container no longer exists" message, plus one warn per failed mutation with no debounce) and the user keeps editing unaware that autosave has stopped. Consider gating the alert suppression on the container having been explicitly removed (e.g., a flag on Inner set when the document's directory is deleted/closed), or at minimum correcting the log message and deduplicating the warns.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At document/container/src/backends/opfs.rs, line 49:

<comment>This early return suppresses the debounced "Autosave is not working" alert for every NotFoundError on queued Write/Append mutations, not just the intended document-deletion race. `Mutation::Delete` already filters NotFound upstream, so this branch is effectively write/append-only; for those, `write_file`/`append_file` use create=true, so a NotFoundError always comes from the cached container handle being stale — which also happens when the browser evicts or clears OPFS storage while a document is still open. In that case every autosave write now fails silently with only a warn (and the misleading "the container no longer exists" message, plus one warn per failed mutation with no debounce) and the user keeps editing unaware that autosave has stopped. Consider gating the alert suppression on the container having been explicitly removed (e.g., a flag on `Inner` set when the document's directory is deleted/closed), or at minimum correcting the log message and deduplicating the warns.</comment>

<file context>
@@ -46,6 +46,10 @@ thread_local! {
 }
 
 fn report_write_error(operation: &str, path: &str, error: &JsValue) {
+	if is_not_found(error) {
+		log::warn!("OPFS {operation} on {path} dropped: the container no longer exists");
+		return;
</file context>