feat: page version history (snapshots) - #505
Conversation
Add page_snapshots table, REST API for list/detail/restore, auto-snapshots from API and Hocuspocus, and editor UI for history preview/compare/restore. Made-with: Cursor
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughページのバージョン履歴（スナップショット）機能を追加します。DBスキーマ、APIエンドポイント（一覧・詳細・復元）、自動スナップショット作成・剪定ロジック、Hocuspocus 側の無効化エンドポイント、フロントエンドの履歴モーダル／プレビュー／比較、関連フック・型・テストが導入されます。 Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API as Server API
participant DB as Database
participant Hocuspocus as Hocuspocus Server
Client->>API: PUT /api/pages/:id/content
API->>DB: UPDATE page_contents, UPDATE pages
Note over API: Fire-and-forget maybeCreateSnapshot (separate connection)
API-->>Client: 200 OK (version)
Hocuspocus->>DB: saveDocumentToDb (commit)
Note over Hocuspocus: Post-commit, open separate DB client
Hocuspocus->>DB: SELECT last page_snapshots for pageId
alt snapshot due or none
Hocuspocus->>DB: INSERT page_snapshots (trigger='auto')
Hocuspocus->>DB: DELETE old snapshots (prune)
else skip
Note over Hocuspocus: No snapshot inserted
end
sequenceDiagram
participant User as Client (User)
participant UI as Page History Modal
participant API as Server API
participant DB as Database
User->>UI: Open history
UI->>API: GET /api/pages/:id/snapshots
API->>DB: SELECT page_snapshots WHERE page_id=...
API->>DB: SELECT users WHERE id IN (...)
API-->>UI: snapshots (with created_by_email)
User->>UI: Select snapshot
UI->>API: GET /api/pages/:id/snapshots/:snapshotId
API->>DB: SELECT snapshot by id,pageId
API-->>UI: snapshot detail (ydoc_state base64)
User->>UI: Confirm restore
UI->>API: POST /api/pages/:id/snapshots/:snapshotId/restore
API->>DB: BEGIN TRANSACTION
API->>DB: INSERT pre-restore snapshot (optional)
API->>DB: UPDATE page_contents (restored ydoc, version++)
API->>DB: INSERT restore snapshot
API->>DB: UPDATE pages (contentPreview, updatedAt)
API->>DB: DELETE old snapshots (prune)
API->>DB: COMMIT
API->>Hocuspocus: POST /internal/documents/:pageId/invalidate (best-effort)
API-->>UI: 200 OK (version, snapshot_id)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a version history system for pages, featuring automatic snapshotting every 10 minutes, a new page_snapshots database table, and a frontend modal for previewing and restoring versions. Feedback identifies a critical bug where restoring a snapshot via the API may be overwritten by Hocuspocus's in-memory state if not properly synchronized. Further improvements are suggested to reduce API latency by making snapshot creation asynchronous, refactoring duplicated pruning logic, and optimizing the performance of Y.Doc encoding on the frontend.
| app.post("/:id/snapshots/:snapshotId/restore", authRequired, async (c) => { | ||
| const pageId = c.req.param("id"); | ||
| const snapshotId = c.req.param("snapshotId"); | ||
| const userId = c.get("userId"); | ||
| const db = c.get("db"); | ||
|
|
||
| // 復元は編集権限が必要（所有者のみ） / Restore requires owner permission | ||
| const page = await db | ||
| .select({ id: pages.id, ownerId: pages.ownerId }) | ||
| .from(pages) | ||
| .where(and(eq(pages.id, pageId), eq(pages.isDeleted, false))) | ||
| .limit(1); | ||
|
|
||
| const pageRow = page[0]; | ||
| if (!pageRow) throw new HTTPException(404, { message: "Page not found" }); | ||
| if (pageRow.ownerId !== userId) throw new HTTPException(403, { message: "Forbidden" }); | ||
|
|
||
| // 復元対象のスナップショットを取得 | ||
| const snapRows = await db | ||
| .select() | ||
| .from(pageSnapshots) | ||
| .where(and(eq(pageSnapshots.id, snapshotId), eq(pageSnapshots.pageId, pageId))) | ||
| .limit(1); | ||
|
|
||
| const snap = snapRows[0]; | ||
| if (!snap) throw new HTTPException(404, { message: "Snapshot not found" }); | ||
|
|
||
| // トランザクションで復元処理 | ||
| const result = await db.transaction(async (tx) => { | ||
| // 1. 現在の状態をスナップショットとして保存 | ||
| const currentContent = await tx | ||
| .select() | ||
| .from(pageContents) | ||
| .where(eq(pageContents.pageId, pageId)) | ||
| .limit(1); | ||
|
|
||
| const current = currentContent[0]; | ||
| if (current) { | ||
| await tx.insert(pageSnapshots).values({ | ||
| pageId, | ||
| version: current.version, | ||
| ydocState: current.ydocState, | ||
| contentText: current.contentText, | ||
| createdBy: userId, | ||
| trigger: "pre-restore", | ||
| }); | ||
| } | ||
|
|
||
| // 2. page_contents を復元対象で上書き（version +1） | ||
| const updated = await tx | ||
| .update(pageContents) | ||
| .set({ | ||
| ydocState: snap.ydocState, | ||
| version: sql`${pageContents.version} + 1`, | ||
| contentText: snap.contentText, | ||
| updatedAt: new Date(), | ||
| }) | ||
| .where(eq(pageContents.pageId, pageId)) | ||
| .returning(); | ||
|
|
||
| const updatedRow = updated[0]; | ||
| if (!updatedRow) throw new HTTPException(500, { message: "Restore failed" }); | ||
|
|
||
| // 3. 復元後の状態もスナップショットとして保存 (trigger: 'restore') | ||
| const restoreSnap = await tx | ||
| .insert(pageSnapshots) | ||
| .values({ | ||
| pageId, | ||
| version: updatedRow.version, | ||
| ydocState: snap.ydocState, | ||
| contentText: snap.contentText, | ||
| createdBy: userId, | ||
| trigger: "restore", | ||
| }) | ||
| .returning(); | ||
|
|
||
| // 4. pages メタデータ更新 | ||
| const contentPreview = snap.contentText | ||
| ? snap.contentText.trim().replace(/\s+/g, " ").slice(0, 120) | ||
| : null; | ||
| await tx | ||
| .update(pages) | ||
| .set({ contentPreview, updatedAt: new Date() }) | ||
| .where(eq(pages.id, pageId)); | ||
|
|
||
| // 5. 100件超過分を削除 | ||
| await tx.execute( | ||
| sql`DELETE FROM page_snapshots WHERE id IN ( | ||
| SELECT id FROM page_snapshots WHERE page_id = ${pageId} | ||
| ORDER BY created_at DESC OFFSET ${MAX_SNAPSHOTS_PER_PAGE} | ||
| )`, | ||
| ); | ||
|
|
||
| return { | ||
| version: updatedRow.version, | ||
| snapshotId: restoreSnap[0]?.id, | ||
| }; | ||
| }); | ||
|
|
||
| return c.json({ | ||
| version: result.version, | ||
| snapshot_id: result.snapshotId, | ||
| }); | ||
| }); |
There was a problem hiding this comment.
There is a critical issue with the restore logic in a collaborative environment. When a snapshot is restored via this API, the Hocuspocus server is not notified. If there are active users connected to the page, Hocuspocus will keep the old state in memory. When those users eventually disconnect or Hocuspocus performs a periodic save, the restored state in the database will be overwritten by the stale in-memory state. You should implement a mechanism to invalidate or refresh the Hocuspocus document cache (e.g., via a internal webhook or Redis Pub/Sub) when a restore occurs.
There was a problem hiding this comment.
対応済みです（9ae82ad）。復元トランザクション内で page_contents を FOR UPDATE でロックし、コミット後に Hocuspocus へ POST /internal/documents/:pageId/invalidate（x-internal-secret: BETTER_AUTH_SECRET）でライブ Y.Doc を無効化するようにしました。.env.example に HOCUSPOCUS_INTERNAL_URL を追記しています。
Addressed in 9ae82ad: row lock on page_contents, post-commit Hocuspocus invalidation via internal endpoint + secret header; see .env.example for HOCUSPOCUS_INTERNAL_URL.
|
|
||
| await applyPagesMetadataUpdate(db, pageId, body); | ||
|
|
||
| await tryAutoSnapshot( |
There was a problem hiding this comment.
Awaiting tryAutoSnapshot here adds unnecessary latency to the PUT /content request. Since this is a best-effort operation that doesn't affect the main update logic, it should be executed without blocking the response.
| await tryAutoSnapshot( | |
| tryAutoSnapshot( |
There was a problem hiding this comment.
対応済みです（9ae82ad）。tryAutoSnapshot は void で非ブロッキングです。
Already non-blocking via void tryAutoSnapshot(...) (9ae82ad).
| const resultRow = result[0]; | ||
| if (!resultRow) throw new HTTPException(500, { message: "Upsert failed" }); | ||
|
|
||
| await tryAutoSnapshot( |
There was a problem hiding this comment.
Similar to the previous comment, awaiting tryAutoSnapshot here increases the response time for the client. It's better to fire-and-forget this best-effort task.
| await tryAutoSnapshot( | |
| tryAutoSnapshot( |
There was a problem hiding this comment.
同上（9ae82ad）。void tryAutoSnapshot です。
Same as above — fire-and-forget (9ae82ad).
| sql`DELETE FROM page_snapshots WHERE id IN ( | ||
| SELECT id FROM page_snapshots WHERE page_id = ${pageId} | ||
| ORDER BY created_at DESC OFFSET ${MAX_SNAPSHOTS_PER_PAGE} | ||
| )`, | ||
| ); |
There was a problem hiding this comment.
The pruning logic is duplicated in multiple places (API service, Hocuspocus utils, and here). It should be refactored into a shared utility function within the snapshotService to ensure consistency and easier maintenance.
There was a problem hiding this comment.
対応済みです（9ae82ad）。pruning SQL は snapshotService.pruneSnapshotsExceedingLimitSql に集約済みです。
Centralized in pruneSnapshotsExceedingLimitSql (9ae82ad).
| const currentYdocState = useMemo((): string => { | ||
| if (!historyOpen || !ydoc) return ""; | ||
| try { | ||
| const state = Y.encodeStateAsUpdate(ydoc); | ||
| const chunks: string[] = []; | ||
| for (let i = 0; i < state.length; i += 8192) { | ||
| chunks.push(String.fromCharCode.apply(null, [...state.subarray(i, i + 8192)])); | ||
| } | ||
| return btoa(chunks.join("")); | ||
| } catch { | ||
| return ""; | ||
| } | ||
| }, [historyOpen, ydoc]); |
There was a problem hiding this comment.
Encoding the entire Y.Doc to base64 on every change while the history modal is open can be very expensive for large documents. This might cause UI lag during collaborative editing. Consider computing this state only when the 'Compare' tab is actually selected, or debouncing the update.
There was a problem hiding this comment.
対応済みです（9ae82ad）。現在 Y.Doc の base64 化は PageHistoryModal で Compare タブ選択時のみ実行するようになっています。
Encoding runs only on the Compare tab in PageHistoryModal (9ae82ad).
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Use expect.objectContaining so TS2532 does not apply to snapshots[0]. Made-with: Cursor
- Join notes with is_deleted=false in assertPageViewAccess (align with Hocuspocus) - Fix Y.XmlText→TipTap: inline nodes under block elements; fragment wraps top-level text - Export pruneSnapshotsExceedingLimitSql; use in restore tx and maybeCreateSnapshot - Fire-and-forget tryAutoSnapshot on PUT /content; document restore+Hocuspocus caveat - Encode current Y.Doc for compare only when Compare tab is open; fix PageHistoryModal export Made-with: Cursor
|
レビュー指摘への対応を push しました（ 対応した内容
未実装（別タスク候補）
各スレッドに返信を付けます。 |
再レビュー依頼最新の変更に対するレビューをお願いします。 @coderabbitai review Claude Code Review は Actions から |
|
最新のコミット（ 🧠 Learnings used✅ Actions performedReview triggered.
|
…rify worktree entry
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Made-with: Cursor
|
レビューコメントへの対応を push しました（ @coderabbitai review |
|
🧠 Learnings used✅ Actions performedReview triggered.
|
| // 対象ドキュメントをキャッシュから外してから接続を閉じ、stale state の再保存を防ぐ。 | ||
| // Remove the live document from cache before disconnecting clients to avoid stale re-persist. | ||
| hocuspocus.documents.delete(documentName); | ||
| hocuspocus.closeConnections(); |
There was a problem hiding this comment.
🔴 closeConnections() disconnects ALL WebSocket clients instead of only the target document's clients
invalidateLiveDocument at server/hocuspocus/src/index.ts:394 calls hocuspocus.closeConnections() without a document name argument. This is the exact same call used in the SIGTERM/SIGINT shutdown handlers (server/hocuspocus/src/index.ts:500, server/hocuspocus/src/index.ts:512) to terminate every WebSocket connection server-wide. Because the intent here is to invalidate only a single document after a snapshot restore, this means restoring one page disconnects every active collaborator on every document across the entire Hocuspocus server. The documents.delete(documentName) on the line before only removes the target document from cache, so when the onDisconnect callback fires for other documents their live docs are still in the map, triggering unnecessary saveDocumentToDb calls for every unrelated document as well.
Prompt for agents
In server/hocuspocus/src/index.ts, the invalidateLiveDocument function calls hocuspocus.closeConnections() which disconnects ALL WebSocket clients server-wide. The intent is to only disconnect clients for the specific documentName being invalidated. Hocuspocus v3 does not provide a built-in per-document closeConnections overload, so the fix requires iterating over the connections for the target document and closing them individually. One approach: iterate hocuspocus.documents or the underlying WebSocket connections, filter by documentName, and close only those sockets. Alternatively, use the Hocuspocus document-level API if available (check the @hocuspocus/server v3.4.4 source for document.getConnections() or similar). The key constraint is that only the target document's connections should be closed, and the document should be removed from the documents map before closing to prevent stale re-persist in onDisconnect.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
ご指摘の通りでした。closeConnections() 無引数は全セッション切断でした。invalidateLiveDocument では hocuspocus.closeConnections(documentName) を 先に呼び、その後 documents.delete(documentName) します（closeConnections は documents を走査するため、delete より前が必須）。対応コミット: 322ddb2。
Devin is correct: no-arg closeConnections() closed every client. We now call hocuspocus.closeConnections(documentName) before documents.delete(documentName) (order matters because closeConnections iterates the map). Fix: 322ddb2.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
- invalidateHocuspocusDocument: AbortController + 2.5s timeout, log-only failures - Hocuspocus invalidate: closeConnections(documentName) before documents.delete - pageSnapshots tests: stub fetch to avoid env-dependent HTTP - pages.test: relax DB chain assertion for snapshot path - editorConfig: bilingual TSDoc for createEditorExtensions Made-with: Cursor
|
レビュー対応を push しました（
@coderabbitai review |
|
最新のコミット（ 🧠 Learnings used✅ Actions performedReview triggered.
|
| async function invalidateLiveDocument(documentName: string): Promise<boolean> { | ||
| if (!hocuspocus.documents.has(documentName)) { | ||
| return false; | ||
| } | ||
|
|
||
| // closeConnections(documentName) は documents マップを走査するため、delete より先に呼ぶ。 | ||
| // Pass documentName so only that document's WebSocket connections close (not server-wide). | ||
| hocuspocus.closeConnections(documentName); | ||
| hocuspocus.documents.delete(documentName); | ||
| return true; |
There was a problem hiding this comment.
🔴 Race condition: Hocuspocus debounced onStoreDocument can overwrite restored content
After a snapshot restore, the API commits the restored content to page_contents and then calls invalidateLiveDocument on Hocuspocus. invalidateLiveDocument calls closeConnections(documentName) and then documents.delete(documentName). However, Hocuspocus is configured with debounce: 2000 and maxDebounce: 10000 (server/hocuspocus/src/index.ts:292-293). If a document change occurred shortly before the restore (within the debounce window), the debounced onStoreDocument timer may still be pending. Manually deleting the document from the documents Map bypasses Hocuspocus's normal document lifecycle and may not cancel these internal debounce timers. When the timer fires, onStoreDocument (server/hocuspocus/src/index.ts:373-380) calls saveDocumentToDb with the stale Y.Doc state, which unconditionally upserts into page_contents (lines 214-224), overwriting the just-restored content.
Additionally, the onDisconnect handler (server/hocuspocus/src/index.ts:340-361) triggered by closeConnections checks hocuspocus.documents.get(documentName) and attempts saveDocumentToDb when remaining === 0. If onDisconnect runs synchronously during closeConnections (before documents.delete), the stale document is still in the map and gets saved back to the DB.
Prompt for agents
The invalidateLiveDocument function in server/hocuspocus/src/index.ts manually deletes a document from hocuspocus.documents, but this bypasses Hocuspocus internal cleanup (debounce timer cancellation). The onDisconnect handler at line 340 also attempts to save the document on last disconnect.
To fix:
1. Clear documentConnectionCounts for the documentName before calling closeConnections, so onDisconnect handlers see remaining=0 already handled and skip the save. Or set a flag/set tracking invalidated documents so onDisconnect and onStoreDocument skip saves for them.
2. Consider using Hocuspocus's built-in document unloading mechanism if available, instead of manually deleting from the documents Map, so debounce timers are properly canceled.
3. As a safety net, add version-checking to saveDocumentToDb so it only saves if the version matches what the in-memory doc expects (similar to the optimistic locking in the API's PUT endpoint).
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
概要
ページの変更履歴（スナップショット）を保存・閲覧・復元できるようにする。10分間隔の自動スナップショット、API と Hocuspocus の両方からの保存、オーナーのみ復元可能、エディタから履歴モーダルでプレビュー・比較・復元が可能。
Adds page version history: periodic snapshots, list/detail/restore API, auto-snapshots from API and Hocuspocus, and an editor modal for preview, compare, and restore (owner-only restore).
変更点
db/migrations/page_snapshotsテーブル（002）server/api//api/pages/:id/snapshots一覧・詳細・復元、pageAccessService、snapshotService、PUT content 時のベストエフォート自動スナップショットserver/hocuspocus/snapshotUtils）src/PageHistoryModal、React Query フック、Y.Doc→TipTap JSON、apiClient拡張、i18n変更の種類
テスト方法
002_add_page_snapshots.sqlを適用する。bun run test:run（または CI と同様の単体テスト）で API / Hocuspocus / フロントの関連テストが通ることを確認する。チェックリスト
スクリーンショット（UI 変更がある場合）
履歴モーダル・一覧・プレビュー画面のスクリーンショットを PR に添付するとレビューしやすいです。
関連 Issue
Made with Cursor
Summary by CodeRabbit