fix: rollback improvements and activity feed real-time simplification #149
Conversation
When undoing a rollback of a create operation, the system was incorrectly trying to trash the page again instead of restoring it. The bug was that `shouldBeTrashed = action === 'rollback'` didn't account for `rollingBackRollback`. When rolling back a rollback: - action is still 'rollback' - but rollingBackRollback is true - so we should RESTORE (shouldBeTrashed = false), not trash Fixed for both pages and drives. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The restoredValues was using `trashed` and `pageId` fields, but currentValues uses `isTrashed`. This field name mismatch caused false conflict detection when undoing a rollback. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
When undoing an AI conversation, skip conflict detection for pages since all changes are from the same conversation - there can't be external conflicts by definition. This is simpler and more robust than the previous approach of querying for external modifications. - 'create' operations: skip conflict check (just trashing) - AI undo (undoGroupActivityIds provided): skip conflict check - Regular rollback: normal conflict detection 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Apply the same fix to all 7 resource types (page, drive, member, permission, role, agent, message) - skip conflict detection entirely when undoGroupActivityIds is provided since all changes are from the same AI conversation by definition. This removes ~130 lines of complex external-modifications queries that weren't working correctly, replacing with simple skip logic. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
When undoing an AI conversation, individual operations may appear as no-ops (e.g., "Already at this version") but the resource will be affected by other operations in the same undo group (e.g., trashed by a create rollback). Skip no-op detection when undoGroupActivityIds is provided to avoid misleading "Cannot undo" warnings. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Pass undoGroupActivityIds to previewRollback and executeRollback in rollback-to-point service, matching the AI undo behavior. This skips conflict detection and no-op warnings for activities within the same rollback group. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Replace truncated warning lists that showed "...and X more" with scrollable lists using ScrollArea. Users need to see exactly what they're rolling back - truncation is unacceptable for transparency. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Apply the same fix to UndoAiChangesDialog - replace truncated lists with scrollable ScrollArea components for warnings, conflicts, and activities. Users need full transparency on what they're undoing. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Skip create no-op check for AI undo operations - old activities from previous sessions may reference already-trashed pages that should be silently ignored (same aiConversationId spans all tool calls in thread) - Replace ScrollArea with native overflow-y-auto for warning lists - ScrollArea with max-h classes causes overflow instead of scrolling - Remove unused ScrollArea import from UndoAiChangesDialog 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add guard in execute path to skip rollback of create operations when the page is already trashed. This prevents: - Wasted database updates - Unnecessary revision increments on the page and its children - Confusing activity log entries for no-op operations The check happens early in executeRollbackPage before any mutations, making the operation truly idempotent for old activities from previous sessions that share the same aiConversationId. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
AI undo was showing 35 activities when only 9 actually happened in the conversation. The aiConversationId is the page's chat thread ID which persists across sessions, so old activities from previous sessions were being included. Changes: - rollback-service.ts: Restore no-op detection for create operations, but for AI undo return canExecute:true with isNoOp:true (silent skip) instead of an error - ai-undo-service.ts: Filter out activities where preview.isNoOp is true so only real changes are shown in the preview count This ensures the preview accurately reflects only the changes from the current AI conversation, not historical activities from past sessions. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Apply the same no-op filtering to rollback-to-point-service that was added to ai-undo-service. This ensures consistent behavior across all rollback preview dialogs - only real changes are shown, not stale activities for already-trashed pages. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The earlier fix bypassed conflict detection entirely for AI undo, which also bypassed the detection of external changes (edits by other users or manual edits after the AI change). This fix restores proper conflict detection for all resource types: - page, drive, member, permission, role, agent, message For each type: 1. Always run conflict detection (getConflictFields) 2. If conflicts found AND we have undo group context: - Query for modifications after this activity - Exclude modifications from activities in the undo group - If any EXTERNAL modifications exist, flag as conflict - If only INTERNAL modifications, ignore (not a real conflict) 3. External conflicts still require force=true to proceed This ensures external changes are protected while internal changes from the same AI conversation are still handled smoothly. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Fixes issue where AI undo fails with "Message not found" immediately
after streaming completes, requiring a page refresh.
Root cause: The Vercel AI SDK generates assistant message IDs on the
client independently from the server. When the user clicks undo, the
client's ID doesn't match the server-saved ID in the database.
Solution: Generate the message ID on the server and send it to the
client via the stream protocol. Both client and server now use the
same ID.
- Page AI chat: Use writer.write({ type: 'start', messageId })
- Global Assistant: Use generateMessageId option in toUIMessageStreamResponse
References:
- https://ai-sdk.dev/docs/ai-sdk-ui/chatbot-message-persistence
- vercel/ai#3512
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add broadcastActivityEvent with 500ms debouncing to socket-utils - Add activity room handlers (join/leave) to realtime service - Create useActivitySocket hook for frontend subscription - Update SidebarActivityTab to auto-refresh on activity:logged events - Add setActivityBroadcastHook to activity-logger for dependency injection - Create instrumentation.ts to initialize broadcast hook at server startup - Fix collectConsecutiveRollbacks to respect changeGroupId Activity updates are now collaborative - all users viewing the same drive/page see each other's activities in real-time without refresh. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Update cleanup guard in useActivitySocket to verify socket is connected before emitting leave event, avoiding emit on disconnected socket. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…cket Previously, the activity sidebar had a separate broadcast system that: - Required instrumentation.ts to set up hooks (wasn't executing in Docker) - Used dedicated activity rooms (activity:drive:*, activity:page:*) - Had its own broadcastActivityEvent function with debouncing This simplifies to reuse the existing page tree socket: - SidebarActivityTab now listens for page events (page:created, etc.) - These events are already broadcast when pages change - No additional broadcast infrastructure needed Changes: - Update SidebarActivityTab to use useSocket and listen for page events - Remove broadcastActivityEvent calls from activity-logger.ts - Delete useActivitySocket.ts hook (no longer needed) - Delete instrumentation.ts (was setting up activity broadcast hook) - Remove activity room handlers from realtime service - Remove unused broadcast code from socket-utils.ts - Update test mocks 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📝 WalkthroughWalkthroughThis PR introduces server-generated message ID synchronization for AI responses, adds real-time socket-based activity updates, implements change group IDs for atomic rollback batches, and refactors rollback service logic to handle undo-group-aware conflict resolution and no-op filtering. UI components now display full scrollable lists instead of truncated previews. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client UI
participant Route as Chat Route
participant Stream as Streaming Response
participant DB as Database
Client->>Route: POST /api/ai/chat (message)
activate Route
Route->>Route: Generate serverAssistantMessageId
Note over Route: createId() creates UUID
Route->>Stream: Create streaming response
Stream->>Client: Write start message<br/>(type: "start", messageId)
Note over Client: Client receives server ID
rect rgb(200, 220, 255)
Note over Route,Stream: Streaming AI response
Route->>Stream: Stream AI tokens
Stream->>Client: Forward tokens with messageId
end
Route->>DB: Persist AI response<br/>(using serverAssistantMessageId)
Note over DB: Message saved with server ID
Route-->>Client: Complete stream
deactivate Route
Client->>Client: Match streamed ID<br/>with persisted ID
sequenceDiagram
participant Tab as SidebarActivityTab
participant Socket as Socket Connection
participant Activity as Activity Service
participant DB as Database
Tab->>Socket: Mount - Subscribe to page events
activate Socket
loop On page event (created/updated/moved/trashed/restored/content-updated)
Socket->>Tab: Event received
Note over Tab: Debounce 300ms
Tab->>Activity: loadActivities()
Activity->>DB: Query recent activities
DB-->>Activity: Activity list
Activity-->>Tab: Updated activities
Tab->>Tab: Re-render activity list
end
Tab->>Socket: Unmount - Unsubscribe & cleanup
deactivate Socket
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
apps/realtime/src/index.tsapps/web/src/app/api/ai/chat/route.tsapps/web/src/app/api/ai/global/[id]/messages/route.tsapps/web/src/components/activity/RollbackToPointDialog.tsxapps/web/src/components/activity/utils.tsapps/web/src/components/ai/shared/chat/UndoAiChangesDialog.tsxapps/web/src/components/layout/right-sidebar/ai-assistant/SidebarActivityTab.tsxapps/web/src/components/layout/right-sidebar/ai-assistant/__tests__/SidebarActivityTab.test.tsxapps/web/src/lib/websocket/socket-utils.tsapps/web/src/services/api/ai-undo-service.tsapps/web/src/services/api/rollback-service.tsapps/web/src/services/api/rollback-to-point-service.tspackages/lib/src/monitoring/activity-logger.ts
🧰 Additional context used
📓 Path-based instructions (10)
apps/web/src/app/**/route.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/src/app/**/route.{ts,tsx}: In Next.js 15 route handlers,paramsin dynamic routes are Promise objects and MUST be awaited before destructuring
UseResponse.json()orNextResponse.json()for returning JSON from route handlers
Get request body usingconst body = await request.json();
Get search parameters usingconst { searchParams } = new URL(request.url);
apps/web/src/app/**/route.{ts,tsx}: In Next.js 15 dynamic routes,paramsare Promise objects and MUST be awaited before destructuring:const { id } = await context.params;
In Route Handlers, get request body withconst body = await request.json();
In Route Handlers, get search parameters withconst { searchParams } = new URL(request.url);
In Route Handlers, return JSON usingResponse.json(data)orNextResponse.json(data)
For permission logic, use centralized functions from@pagespace/lib/permissions:getUserAccessLevel(),canUserEditPage()
Files:
apps/web/src/app/api/ai/chat/route.tsapps/web/src/app/api/ai/global/[id]/messages/route.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Never useanytypes - always use proper TypeScript types
Use camelCase for variable and function names
Use UPPER_SNAKE_CASE for constants
Use PascalCase for type and enum names
Use kebab-case for filenames, except React hooks (camelCase withuseprefix), Zustand stores (camelCase withuseprefix), and React components (PascalCase)
Lint with Next/ESLint as configured inapps/web/eslint.config.mjs
Message content should always use the message parts structure with{ parts: [{ type: 'text', text: '...' }] }
Use centralized permission functions from@pagespace/lib/permissions(e.g.,getUserAccessLevel,canUserEditPage) instead of implementing permission logic locally
Always use Drizzle client from@pagespace/dbpackage for database access
Use ESM modules throughout the codebase
**/*.{ts,tsx}: Never useanytypes - always use proper TypeScript types
Write code that is explicit over implicit and self-documenting
Files:
apps/web/src/app/api/ai/chat/route.tsapps/web/src/lib/websocket/socket-utils.tsapps/web/src/components/layout/right-sidebar/ai-assistant/__tests__/SidebarActivityTab.test.tsxapps/web/src/app/api/ai/global/[id]/messages/route.tsapps/web/src/components/ai/shared/chat/UndoAiChangesDialog.tsxapps/web/src/services/api/rollback-to-point-service.tsapps/realtime/src/index.tsapps/web/src/components/activity/utils.tsapps/web/src/services/api/ai-undo-service.tsapps/web/src/services/api/rollback-service.tsapps/web/src/components/activity/RollbackToPointDialog.tsxpackages/lib/src/monitoring/activity-logger.tsapps/web/src/components/layout/right-sidebar/ai-assistant/SidebarActivityTab.tsx
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.ts: React hook files should use camelCase matching the exported hook name (e.g.,useAuth.ts)
Zustand store files should use camelCase withuseprefix (e.g.,useAuthStore.ts)
Files:
apps/web/src/app/api/ai/chat/route.tsapps/web/src/lib/websocket/socket-utils.tsapps/web/src/app/api/ai/global/[id]/messages/route.tsapps/web/src/services/api/rollback-to-point-service.tsapps/realtime/src/index.tsapps/web/src/components/activity/utils.tsapps/web/src/services/api/ai-undo-service.tsapps/web/src/services/api/rollback-service.tspackages/lib/src/monitoring/activity-logger.ts
**/*.{ts,tsx,js,jsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Format code with Prettier
Files:
apps/web/src/app/api/ai/chat/route.tsapps/web/src/lib/websocket/socket-utils.tsapps/web/src/components/layout/right-sidebar/ai-assistant/__tests__/SidebarActivityTab.test.tsxapps/web/src/app/api/ai/global/[id]/messages/route.tsapps/web/src/components/ai/shared/chat/UndoAiChangesDialog.tsxapps/web/src/services/api/rollback-to-point-service.tsapps/realtime/src/index.tsapps/web/src/components/activity/utils.tsapps/web/src/services/api/ai-undo-service.tsapps/web/src/services/api/rollback-service.tsapps/web/src/components/activity/RollbackToPointDialog.tsxpackages/lib/src/monitoring/activity-logger.tsapps/web/src/components/layout/right-sidebar/ai-assistant/SidebarActivityTab.tsx
apps/web/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/web/src/**/*.{ts,tsx}: Use message parts structure for message content:{ parts: [{ type: 'text', text: '...' }] }
For database access, always use Drizzle client from@pagespace/db:import { db, pages } from '@pagespace/db';
Use centralized Drizzle ORM with PostgreSQL for all database operations - no direct SQL or other ORMs
Use Socket.IO for real-time collaboration features - imported from the realtime service at port 3001
Use Vercel AI SDK with async/await for all AI operations and streaming
Use Next.js 15 App Router and TypeScript for all routes and components
Files:
apps/web/src/app/api/ai/chat/route.tsapps/web/src/lib/websocket/socket-utils.tsapps/web/src/components/layout/right-sidebar/ai-assistant/__tests__/SidebarActivityTab.test.tsxapps/web/src/app/api/ai/global/[id]/messages/route.tsapps/web/src/components/ai/shared/chat/UndoAiChangesDialog.tsxapps/web/src/services/api/rollback-to-point-service.tsapps/web/src/components/activity/utils.tsapps/web/src/services/api/ai-undo-service.tsapps/web/src/services/api/rollback-service.tsapps/web/src/components/activity/RollbackToPointDialog.tsxapps/web/src/components/layout/right-sidebar/ai-assistant/SidebarActivityTab.tsx
**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
**/*.tsx: React component files should use PascalCase (e.g.,UserProfile.tsx)
Use @dnd-kit for drag-and-drop functionality
Use Zustand for client state management
Use SWR for server state management and caching
Files:
apps/web/src/components/layout/right-sidebar/ai-assistant/__tests__/SidebarActivityTab.test.tsxapps/web/src/components/ai/shared/chat/UndoAiChangesDialog.tsxapps/web/src/components/activity/RollbackToPointDialog.tsxapps/web/src/components/layout/right-sidebar/ai-assistant/SidebarActivityTab.tsx
**/*.{tsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
Use Tailwind CSS and shadcn/ui components for styling and UI
Files:
apps/web/src/components/layout/right-sidebar/ai-assistant/__tests__/SidebarActivityTab.test.tsxapps/web/src/components/ai/shared/chat/UndoAiChangesDialog.tsxapps/web/src/components/activity/RollbackToPointDialog.tsxapps/web/src/components/layout/right-sidebar/ai-assistant/SidebarActivityTab.tsx
apps/web/src/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/web/src/components/**/*.{ts,tsx}: When document editing, register editing state withuseEditingStore.getState().startEditing()to prevent UI refreshes, and clean up in return statement
When AI is streaming, register streaming state withuseEditingStore.getState().startStreaming()to prevent UI refreshes, and clean up in return statement
For SWR data fetching with editing protection, useisPaused: () => hasLoadedRef.current && isEditingActive()to allow initial fetch and only pause after, withrevalidateOnFocus: false
Use Zustand for client-side state management as the primary state solution
Use SWR for server state and caching with proper configuration includingrevalidateOnFocus: falsefor editing protection
Use TipTap rich text editor with markdown support for document editing
Use Monaco Editor for code editing features
Use @dnd-kit for drag-and-drop functionality instead of other libraries
Use Tailwind CSS with shadcn/ui components for all UI styling and components
Files:
apps/web/src/components/layout/right-sidebar/ai-assistant/__tests__/SidebarActivityTab.test.tsxapps/web/src/components/ai/shared/chat/UndoAiChangesDialog.tsxapps/web/src/components/activity/utils.tsapps/web/src/components/activity/RollbackToPointDialog.tsxapps/web/src/components/layout/right-sidebar/ai-assistant/SidebarActivityTab.tsx
apps/realtime/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Socket.IO for real-time collaboration features
Files:
apps/realtime/src/index.ts
**/*ai*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Vercel AI SDK for AI integrations
Files:
apps/web/src/services/api/ai-undo-service.ts
🧠 Learnings (10)
📚 Learning: 2025-12-23T18:49:41.966Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-23T18:49:41.966Z
Learning: Applies to apps/web/src/components/**/*.{ts,tsx} : When AI is streaming, register streaming state with `useEditingStore.getState().startStreaming()` to prevent UI refreshes, and clean up in return statement
Applied to files:
apps/web/src/app/api/ai/chat/route.tsapps/web/src/app/api/ai/global/[id]/messages/route.tsapps/web/src/components/layout/right-sidebar/ai-assistant/SidebarActivityTab.tsx
📚 Learning: 2025-12-23T18:49:41.966Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-23T18:49:41.966Z
Learning: Applies to apps/web/src/**/*.{ts,tsx} : Use Socket.IO for real-time collaboration features - imported from the realtime service at port 3001
Applied to files:
apps/web/src/lib/websocket/socket-utils.tsapps/web/src/components/layout/right-sidebar/ai-assistant/SidebarActivityTab.tsx
📚 Learning: 2025-12-18T05:22:42.263Z
Learnt from: 2witstudios
Repo: 2witstudios/PageSpace PR: 96
File: apps/web/src/components/layout/right-sidebar/ai-assistant/SidebarChatTab.tsx:641-657
Timestamp: 2025-12-18T05:22:42.263Z
Learning: In apps/web/src/components/layout/right-sidebar/ai-assistant/SidebarChatTab.tsx, the provider/model selector buttons are intentionally non-functional placeholders in the compact sidebar view. The `hideModelSelector={true}` prop is passed to ChatInput to hide the full ProviderModelSelector. Users are expected to use the full GlobalAssistantView for model selection. A settings link may be added in a future iteration.
Applied to files:
apps/web/src/components/layout/right-sidebar/ai-assistant/__tests__/SidebarActivityTab.test.tsx
📚 Learning: 2025-12-23T18:49:41.966Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-23T18:49:41.966Z
Learning: Applies to apps/web/src/**/*.{ts,tsx} : Use Next.js 15 App Router and TypeScript for all routes and components
Applied to files:
apps/web/src/components/layout/right-sidebar/ai-assistant/__tests__/SidebarActivityTab.test.tsx
📚 Learning: 2025-12-14T14:54:45.713Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T14:54:45.713Z
Learning: Applies to **/*.{ts,tsx} : Keep commits and diffs minimal and focused on specific changes
Applied to files:
apps/web/src/components/ai/shared/chat/UndoAiChangesDialog.tsxapps/web/src/components/activity/RollbackToPointDialog.tsx
📚 Learning: 2025-12-16T19:03:59.870Z
Learnt from: 2witstudios
Repo: 2witstudios/PageSpace PR: 91
File: apps/web/src/components/ai/shared/chat/tool-calls/CompactToolCallRenderer.tsx:253-277
Timestamp: 2025-12-16T19:03:59.870Z
Learning: In apps/web/src/components/ai/shared/chat/tool-calls/CompactToolCallRenderer.tsx (TypeScript/React), use the `getLanguageFromPath` utility from `formatters.ts` to infer syntax highlighting language from file paths instead of hardcoding language values in DocumentRenderer calls.
Applied to files:
apps/web/src/components/ai/shared/chat/UndoAiChangesDialog.tsx
📚 Learning: 2025-12-14T14:54:45.713Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T14:54:45.713Z
Learning: Applies to **/*.{ts,tsx} : Always use the Drizzle client and database exports from `pagespace/db` (e.g., `import { db, pages } from 'pagespace/db'`) for all database access
Applied to files:
apps/web/src/services/api/rollback-to-point-service.ts
📚 Learning: 2025-12-23T18:49:41.966Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-23T18:49:41.966Z
Learning: Applies to apps/web/src/**/*.{ts,tsx} : For database access, always use Drizzle client from `pagespace/db`: `import { db, pages } from 'pagespace/db';`
Applied to files:
apps/web/src/services/api/rollback-to-point-service.ts
📚 Learning: 2025-12-22T20:04:40.910Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-22T20:04:40.910Z
Learning: Applies to apps/realtime/**/*.{ts,tsx} : Use Socket.IO for real-time collaboration features
Applied to files:
apps/web/src/components/layout/right-sidebar/ai-assistant/SidebarActivityTab.tsx
📚 Learning: 2025-12-22T20:04:40.910Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-22T20:04:40.910Z
Learning: Use Next.js 15 App Router with TypeScript
Applied to files:
apps/web/src/components/layout/right-sidebar/ai-assistant/SidebarActivityTab.tsx
🧬 Code graph analysis (7)
apps/web/src/app/api/ai/global/[id]/messages/route.ts (1)
packages/lib/src/logging/logger-config.ts (1)
loggers(8-18)
apps/web/src/components/ai/shared/chat/UndoAiChangesDialog.tsx (1)
apps/web/src/components/ui/badge.tsx (1)
Badge(46-46)
apps/web/src/services/api/rollback-to-point-service.ts (2)
apps/web/src/services/api/rollback-service.ts (2)
previewRollback(1555-1562)executeRollback(1569-1825)packages/lib/src/monitoring/change-group.ts (1)
createChangeGroupId(5-7)
apps/web/src/components/activity/utils.ts (1)
apps/web/src/components/activity/types.ts (1)
ActivityLog(10-36)
apps/web/src/services/api/ai-undo-service.ts (2)
packages/lib/src/monitoring/change-group.ts (1)
createChangeGroupId(5-7)apps/web/src/services/api/rollback-service.ts (1)
executeRollback(1569-1825)
apps/web/src/services/api/rollback-service.ts (3)
packages/db/src/schema/monitoring.ts (1)
activityLogs(430-502)packages/lib/src/monitoring/change-group.ts (1)
createChangeGroupId(5-7)packages/db/src/schema/core.ts (1)
pages(24-68)
apps/web/src/components/layout/right-sidebar/ai-assistant/SidebarActivityTab.tsx (1)
apps/web/src/hooks/useSocket.ts (1)
useSocket(5-30)
🔇 Additional comments (40)
apps/web/src/components/layout/right-sidebar/ai-assistant/__tests__/SidebarActivityTab.test.tsx (1)
14-29: LGTM!The mock additions properly support testing the component in isolation:
useRoutermock provides the expected Next.js 15 App Router methodsuseSocketreturningnullcorrectly simulates the absence of a socket connection, which prevents real socket interactions during tests while allowing the component's conditional logic (if (!socket || !driveId)) to handle it gracefullyapps/web/src/components/layout/right-sidebar/ai-assistant/SidebarActivityTab.tsx (2)
247-281: Solid real-time integration pattern.The debounced socket event handling is well-implemented:
- Proper guard for missing socket or driveId
- 300ms debounce prevents excessive API calls during rapid events
- Clean teardown of listeners and pending timers
One consideration: since
debouncedRefetchis recreated on each effect run, callingsocket.off(event, debouncedRefetch)in cleanup correctly removes the listeners registered in that same effect cycle. This works correctly due to closure scope.
3-3: Imports look good.The
useRefaddition supports the debounce timer reference, anduseSockethook import aligns with the coding guidelines for Socket.IO integration.Also applies to: 38-38
apps/realtime/src/index.ts (1)
265-271: Formatting looks good.The blank line cleanly separates the guard clause from the handler logic, improving readability. The overall structure and error handling throughout the file are solid.
apps/web/src/lib/websocket/socket-utils.ts (1)
356-357: LGTM!Formatting-only change (trailing newline). No functional impact.
packages/lib/src/monitoring/activity-logger.ts (3)
157-157: LGTM!Formatting change (blank line after constant definition).
219-219: LGTM!Documentation update correctly reflects the architectural change: activity sidebar now refreshes via existing page tree socket events rather than a separate broadcast system.
234-234: LGTM!Documentation update for
logActivityWithTxmirrors the change inlogActivity, maintaining consistency.apps/web/src/components/activity/utils.ts (1)
103-124: LGTM!The refined grouping logic correctly ensures consecutive rollbacks are only grouped when they share the same
changeGroupId. The===comparison correctly handles thenullcase (grouping rollbacks withnullchangeGroupId together). This aligns with the PR's undo-group semantics.apps/web/src/components/activity/RollbackToPointDialog.tsx (2)
187-196: LGTM!Good improvement replacing truncated lists with full scrollable lists. The
max-h-[120px]constraint withoverflow-y-autokeeps the dialog manageable while showing all items. Thepr-2padding correctly accounts for scrollbar space.
209-218: LGTM!Consistent implementation with the cannotRollbackActivities section above. Same scrollable pattern applied to conflict warnings.
apps/web/src/components/ai/shared/chat/UndoAiChangesDialog.tsx (3)
211-217: LGTM!Warnings list now renders all items in a scrollable container, consistent with the RollbackToPointDialog pattern.
229-237: LGTM!Conflict list rendered in scrollable container, matching the pattern used elsewhere in this PR.
247-257: LGTM!Activities list now displays all items in a scrollable container. The slightly smaller
max-h-[100px]keeps this section compact, which is appropriate given this section appears within a radio group option area.apps/web/src/app/api/ai/global/[id]/messages/route.ts (1)
738-755: LGTM!Excellent fix for the undo-after-streaming issue. By generating the message ID server-side and passing it via
generateMessageId, client and server are guaranteed to use the same ID. This ensures:
- The streamed message ID matches what's persisted
- Subsequent undo operations can correctly identify the message
The documentation comment referencing the AI SDK docs is helpful for future maintainers.
apps/web/src/app/api/ai/chat/route.ts (2)
725-740: LGTM!Server-side message ID generation with the
startmessage pattern. This approach differs slightly from the global messages route (which usesgenerateMessageIdcallback) but achieves the same goal: ensuring client and server share the same message ID for consistent undo behavior.The
writer.write({ type: 'start', messageId })sends the ID at stream initialization, which the client'suseChathook will consume.
825-827: LGTM!Correctly uses the server-generated ID for message persistence, ensuring the saved message ID matches what was sent to the client.
apps/web/src/services/api/ai-undo-service.ts (4)
15-16: LGTM!Import of
createChangeGroupIdfrom@pagespace/lib/monitoringis correctly added to support the new undo grouping functionality.
295-309: LGTM!Good improvement to filter out no-op activities during preview. This ensures users see an accurate count of real changes that will occur. The debug logging provides visibility into what's being skipped without cluttering the preview results.
454-455: LGTM!Creating a shared
changeGroupIdfor all rollbacks in this undo batch ensures they're grouped together in the activity log. This aligns with the rollback grouping changes inutils.tsand enables coherent auditing of multi-activity undo operations.
491-491: LGTM!The
changeGroupIdis correctly passed toexecuteRollback, ensuring all rollback activities within this undo batch share the same group identifier. This works in conjunction withundoGroupActivityIdsto enable proper conflict detection and activity grouping.apps/web/src/services/api/rollback-to-point-service.ts (5)
10-10: LGTM! Import aligns with usage.The
createChangeGroupIdimport is correctly sourced from@pagespace/lib/monitoringand used appropriately in the execute path to group batch rollbacks.
149-151: LGTM! Undo group collection is correct.Collecting all activity IDs upfront enables proper conflict detection within the group, allowing the rollback service to distinguish between internal (within-group) and external modifications.
173-173: LGTM! undoGroupActivityIds propagation is correct.Passing the undo group to
previewRollbackenables the service to skip conflict detection for modifications made by other activities in the same batch.
192-201: LGTM! No-op filtering improves preview accuracy.Silently filtering no-op activities (e.g., pages already trashed from previous sessions) ensures the preview count reflects only meaningful changes. The debug logging provides good observability.
330-330: LGTM! Execute call correctly propagates grouping options.The
executeRollbackcall now passesundoGroupActivityIdsandchangeGroupId: sharedChangeGroupId, ensuring all rollbacks in the batch share the same change group for atomicity and proper conflict detection.apps/web/src/services/api/rollback-service.ts (14)
288-291: Type change to optional is appropriate.Making
pageMutationMetaoptional (PageMutationMeta | undefined) correctly handles cases where rollback operations don't produce mutation metadata (e.g., no-ops for already-trashed pages at line 1861).
739-760: Good AI undo idempotency handling for create operations.The logic correctly distinguishes between:
- AI undo (with
undoGroupActivityIds): ReturnscanExecute: true, isNoOp: trueto allow silent skipping- Non-AI undo: Returns error with
isNoOp: truefor user feedbackThis enables filtering stale activities from previous sessions while maintaining proper error handling for manual rollbacks.
763-808: Internal vs external conflict detection is well-implemented.The logic correctly:
- Detects conflicts via
getConflictFields- Queries for external modifications (activities not in the undo group)
- Clears conflict if all modifications are internal to the undo group
The
gt(activityLogs.timestamp, activity.timestamp)ensures only modifications after the target activity are considered.
850-898: Drive conflict detection follows the same pattern.The implementation mirrors the page conflict detection with appropriate resource type filtering (
eq(activityLogs.resourceType, 'drive')).
996-1032: Member conflict detection is consistent.Follows the established pattern for internal vs external conflict detection.
1133-1169: Permission conflict detection is consistent.Follows the established pattern for internal vs external conflict detection.
1209-1212: Role reorder conflict skip is appropriate.Skipping conflict detection for
role_reorderduring AI undo makes sense since reorder operations within the same conversation are inherently related.
1294-1330: Role update conflict detection is consistent.Follows the established pattern for internal vs external conflict detection.
1384-1419: Agent conflict detection is consistent.Follows the established pattern for internal vs external conflict detection.
1463-1498: Message conflict detection is consistent.Follows the established pattern for internal vs external conflict detection.
1521-1523: No-op skip for AI undo is appropriate.Skipping no-op detection when
undoGroupActivityIds.length > 0prevents false "Already at this version" errors when a resource will be affected by other operations in the undo group.
1573-1576: executeRollback signature extension is clean.Adding optional
changeGroupIdto options allows callers to provide a shared group ID for batch operations while maintaining backward compatibility.
1616-1616: changeGroupId fallback is correct.Using
passedChangeGroupId ?? createChangeGroupId()ensures a new group ID is created when not provided, maintaining consistency with single-rollback operations.
1849-1889: Handle already-trashed pages in rollbackPageChange.The changes correctly:
- Check if page is already trashed before attempting to trash
- Return early with
{ restoredValues: { isTrashed: true }, pageMutationMeta: undefined }for no-op cases- Use
isTrashed(correct field name) inrestoredValuesThe
pageMutationMeta: undefinedreturn aligns with the type change at line 290.
| // Collect all activity IDs for undo group logic (skip conflict/no-op detection within group) | ||
| const undoGroupActivityIds = preview.activitiesAffected.map(a => a.id); | ||
|
|
||
| // Create a shared changeGroupId so all rollbacks in this batch are grouped together | ||
| const sharedChangeGroupId = createChangeGroupId(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how undoGroupActivityIds is used in executeRollback
rg -nA 20 'undoGroupActivityIds' apps/web/src/services/api/rollback-service.ts | head -150Repository: 2witstudios/PageSpace
Length of output: 6014
🏁 Script executed:
#!/bin/bash
# Also check the previewRollbackToPoint function to see how it builds undoGroupActivityIds at line 150
rg -nB 5 -A 10 'const undoGroupActivityIds' apps/web/src/services/api/rollback-to-point-service.tsRepository: 2witstudios/PageSpace
Length of output: 1631
🏁 Script executed:
#!/bin/bash
# Check for context around line 290-294 in rollback-to-point-service.ts
sed -n '280,310p' apps/web/src/services/api/rollback-to-point-service.tsRepository: 2witstudios/PageSpace
Length of output: 1351
🏁 Script executed:
#!/bin/bash
# Check if executeRollback is called in executeRollbackToPoint and how undoGroupActivityIds is passed
sed -n '299,350p' apps/web/src/services/api/rollback-to-point-service.tsRepository: 2witstudios/PageSpace
Length of output: 2073
🏁 Script executed:
#!/bin/bash
# Check the executeRollback function signature and usage of undoGroupActivityIds
rg -nB 5 -A 15 'async function executeRollback' apps/web/src/services/api/rollback-service.ts | head -80Repository: 2witstudios/PageSpace
Length of output: 981
Pass complete undoGroupActivityIds to execute phase, not a filtered subset.
In previewRollbackToPoint, undoGroupActivityIds is built from all activities in the range (line 150), but in executeRollbackToPoint (line 291), it's rebuilt from only preview.activitiesAffected—which excludes no-ops. Since executeRollback calls previewRollback again for each activity (line 1584 in rollback-service.ts), the second preview pass uses incomplete undoGroupActivityIds. This means activities marked as no-ops during preview won't be in the undo group during execute, causing their resource modifications to be flagged as external conflicts instead of internal to the batch. Pass the original complete list (line 150's activities) to executeRollback instead of the filtered list.
🤖 Prompt for AI Agents
In apps/web/src/services/api/rollback-to-point-service.ts around lines 290 to
294, executeRollbackToPoint reconstructs undoGroupActivityIds from
preview.activitiesAffected (which excludes no-ops) causing the execute phase to
receive an incomplete undo group; instead pass the original complete list of
activity IDs collected in the previewRollbackToPoint phase (the activities
variable built at line ~150) into executeRollbackToPoint so you use the full
undoGroupActivityIds (including no-ops) and ensure executeRollback’s internal
preview sees the same complete undo group.
Summary
This PR contains several related fixes for the rollback/undo system and simplifies the activity feed real-time updates:
Rollback System Fixes
Real-time Activity Feed Simplification
useActivitySocket.tshook andinstrumentation.tsAI Chat Fixes
Test Plan
🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.