Skip to content

RFC: should the store worker drop a queued save for a purged document? #362

Description

@HMarzban

Parent

#328. Related: #216

What to build

A purge erases a document for good. A save that was already in the store queue when the purge landed still reaches the worker. The worker finds no version row, treats the job as a first save, and writes the document again. The erased content comes back under the old id, and the new-document email can fire. This RFC asks whether the worker checks the purge tombstone on that path, or whether the gap stays an accepted hazard.

Acceptance criteria

  • The maintainer picks option A or B below in a comment on this issue.
  • If A: a build issue exists for the worker change, with the check described under "Desired behavior".
  • If B: apps/hocuspocus.server/CLAUDE.md §Documented hazards we accept has a bullet for this window, its width, and the manual repair.
  • The "Two windows stay open" bullet in apps/hocuspocus.server/CLAUDE.md §Persistence points at the ruling.

Blocked by

None — can start now.

Agent brief

Type: HITL — the maintainer chooses a worker check (A) or an accepted hazard (B). The choice goes in a comment on this issue. An agent then records it in apps/hocuspocus.server/CLAUDE.md.

Category: ruling

Current behavior:

  • purgeDocumentFootprint (apps/hocuspocus.server/src/api/services/documentPurge.service.ts:26) publishes a sealed event, then deletes the metadata row and writes a DocumentPurgeTombstone in one transaction (:86). Version rows go with the metadata row.
  • The store hook drops a flush for a sealed room (isRoomSealed, src/config/hocuspocus.config.ts:208). The comment there says a job already in wait still reaches the worker.
  • The worker locks the head row. When none exists, isFirst is true (src/lib/queue.ts:434). It then calls upsertDocumentMetadata, which re-creates the metadata row (:450), and inserts version 1.
  • After commit, isFirstCreation sends sendNewDocumentNotification (queue.ts near :506). The content-change fan-out and the doc:{id}:saved publish also run.
  • onAuthenticate reads the tombstone only when no metadata row exists (src/hocuspocus.server.ts:540-548). Once the worker re-creates the row, the tombstone is not read, and the document opens again.
  • The dead-letter drain already reads tombstones before a replay (drainStoreDeadLetterQueue, queue.ts:246).
  • apps/hocuspocus.server/CLAUDE.md §Persistence, bullet "Two windows stay open", records this window. It is as wide as queue latency. No metric measures that latency today.

Desired behavior: Either the worker never re-creates a purged document, or the hazard is written down with its repair.

Options.

  • A. Check the tombstone on the no-row branch. Inside the transaction, when existingDoc is null, read DocumentPurgeTombstone by documentId. On a hit, write nothing, skip the email, the fan-out and the saved publish, and count the drop. The read is one primary-key lookup, and only first saves pay it. A tombstoned id has no legitimate second life. For a derived id, the purge bumps the slug epoch, so a new draft derives a new id. onAuthenticate already refuses a tombstoned id.
  • B. Accept the hazard. Record it in §Documented hazards we accept. The repair is to purge the document again.

Evidence the maintainer needs.

  1. How often purges run with saves still queued. Production logs "Dropped store for a sealed (deleted) room" when the hook drops a flush; each line is a near miss.
  2. The store-documents queue depth in the wait state under load. The worker samples it every 15 s into the queue_jobs gauge (src/lib/metrics.ts). Depth is a proxy for the window width; no metric records wait time.

Where to start: createDocumentWorker and upsertDocumentMetadata in src/lib/queue.ts; purgeDocumentFootprint; isRoomSealed in src/lib/accessRealtime.ts; the tombstone read in onAuthenticate. Line numbers are hints as of 2026-09-28; the agent searches by symbol.

Rules that apply: apps/hocuspocus.server/CLAUDE.md §Persistence and §Documented hazards we accept; root CLAUDE.md §Settled, "Collab storage design" (neither option touches it); AGENTS.md §Test Policy. If A is built, a race like this is case (c), so add one check to scripts/e2e-store-pipeline.ts. Prove it by sabotage: remove the tombstone read, and the check must fail.

Verify: After the doc edit, run bun run check from the repo root and see it pass. For the follow-up build, run bun run test:e2e in apps/hocuspocus.server. It needs a real Postgres (DATABASE_URL) and Redis (REDIS_HOST, REDIS_PORT), as the header of scripts/e2e-store-pipeline.ts says.

Out of scope

  • Moving the Trash purge loop to the worker. Move the Trash purge loop to the worker #216 owns that.
  • Changing when the purge publishes its seal. The order is a written rule in §Persistence.
  • Building option A. The ruling opens its own build issue.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions