Repository navigation
fix: resolve AI chat undo feature not working - #122
Conversation
Implement comprehensive version history browsing and rollback capabilities for PageSpace, allowing users to restore resources to previous states. ## Schema Changes - Add 'rollback' operation to activity_operation enum - Add rollbackFromActivityId and contentFormat fields to activity_logs - Create retention_policies table for plan-based history retention ## Core Features - RBAC-based rollback permissions (edit access = rollback access) - Resource-specific rollback handlers (pages, drives, agents, etc.) - Plan-based retention limits (7/30/90/unlimited days) - Rollback creates new activity entry (history never erased) ## API Endpoints - GET /api/activities/[activityId] - Single activity with rollback eligibility - POST /api/activities/[activityId]/rollback - Execute rollback - GET /api/pages/[pageId]/history - Page version history - GET /api/drives/[driveId]/history - Drive version history (admin) ## UI Components - VersionHistoryPanel - Slide-out panel with timeline and filters - VersionHistoryItem - Activity item with "Restore" action - RollbackConfirmDialog - Confirmation modal with warnings 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Fully implement rollbackPermissionChange for grant/revoke/update operations - Fully implement rollbackMemberChange for add/remove/role change operations - Fully implement rollbackRoleChange for create/delete/update operations - Fix lint errors: remove unused imports, fix useEffect dependencies - Add package exports for @pagespace/lib/permissions and @pagespace/lib/monitoring - Fix useToast import path to use correct hook location 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add ability to undo AI changes from a specific message point: - Preview what will be affected before undoing - Two modes: revert conversation only OR revert with all tool changes - Activity logging for audit compliance New files: - ai-undo-service.ts: preview and execute functions - UndoAiChangesDialog.tsx: confirmation dialog with mode selection - /api/ai/chat/messages/[messageId]/undo: GET preview, POST execute Changes: - Add conversation_undo operations to activity schema - Add message rollback handler to rollback-service - Wire undo button to MessageActionButtons and MessageRenderer 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Required for UndoAiChangesDialog mode selection. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Required for radio-group UI component used in UndoAiChangesDialog. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The previous lockfile had a malformed git SSH URL that broke CI: - Before: git+https://git@github.com:electron/node-gyp.git (broken) - After: https://codeload.github.com/electron/node-gyp/tar.gz/... (works) Restored lockfile and re-ran pnpm install to properly resolve dependencies. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add CRITICAL rules for package manager (always pnpm, never npm) - Add CRITICAL rules for database migrations (never manually edit) - Fix retention days: founder=90, business=unlimited (was swapped) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Wrap switch case declarations in blocks to prevent variable leakage - Add isActivityEligibleForRollback() helper for DRY rollback checks - Export helper for use in history API routes Addresses PR #118 review issues #5, #13 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add contentFormat field for content type tracking - Add rollbackFromActivityId for rollback chain references - Pass fields as top-level properties to logActivity Addresses PR #118 review issue #3 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add permission check before returning activity details - Implement resourceType query parameter filtering in history routes - Use shared isActivityEligibleForRollback helper - Add case-insensitive resource type matching Addresses PR #118 review issues #6, #7, #13 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Use Drizzle count() instead of .length for efficient queries - Add operation validation against known valid operations - Fix unsafe null type assertion in RollbackPreview - Re-export RollbackContext type for API route usage - Wrap AI undo mutations in database transaction - Use switch statement for context determination Addresses PR #118 review issues #2, #16, #18, #19, #23 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Use post() with CSRF token for rollback requests - Add error toast for failed preview fetch - Handle non-JSON error responses gracefully - Fix non-existent postWithAuth import to use post - Remove re-throw after toast notification - Fix redundant ternary for buttonSize Addresses PR #118 review issues #1, #8, #11, #17, #22, NEW 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Critical & Major fixes: - Add missing .defaultNow() to retention_policies.updatedAt (fixes schema/migration mismatch that would cause insert failures) - Add contentFormatEnum for type-safe content format validation (prevents invalid values during rollback parsing) - Add CHECK constraint: retentionDays >= -1 (where -1 = unlimited) - Add rollback source snapshot fields for audit trail preservation: - rollbackSourceOperation: captures source activity type - rollbackSourceTimestamp: captures when source change occurred - rollbackSourceTitle: captures resource title at time of change These denormalized fields survive retention policy deletion, ensuring complete audit trails even when source activities are purged. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Fixes Major issue #2 from PR #118 review: - Move rollbackFromActivityId from metadata to top-level field - Move contentFormat from metadata to top-level field - Add rollback source snapshot fields to ActivityLogInput interface - Update logRollbackActivity to accept and pass snapshot fields Fields are now stored in dedicated database columns instead of being nested in the metadata JSONB, enabling proper indexing and querying. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Enables atomic rollback operations (Issue #7 from PR #118 review): - Add optional transaction parameter to executeRollback() - Update all internal rollback functions to accept database parameter - Pass rollback source snapshot fields to logRollbackActivity When a transaction is provided, all database operations use it instead of the default db connection, enabling atomic rollback + message deletion in AI undo operations. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Fixes Issues #5 and #7 from PR #118 review: Context determination (#5): - Align executeAiUndo context logic with previewAiUndo - Use 'ai_tool' context for pages (all activities here are AI-generated) - Simplifies from switch statement to direct assignment with drive check Transaction atomicity (#7): - Wrap rollbacks AND message deletion in single transaction - Pass transaction to executeRollback for atomic operations - If any operation fails, entire undo is rolled back This ensures users don't end up in inconsistent states where messages are deleted but only some changes were reverted. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Fixes Issue #6 from PR #118 review: - Replace manual mode validation with Zod schema - Aligns with codebase patterns (see rollback route, history route) - Provides type-safe body parsing with proper TypeScript inference 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add CHECK constraint to retentionPolicies schema definition (aligns Drizzle schema with existing migration constraint) - Change AI undo to all-or-nothing transaction semantics (any rollback failure aborts entire operation) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Create subscription_tier pgEnum with 'free', 'pro', 'business', 'founder' - Convert retentionPolicies.subscriptionTier from text to enum - Adds DB-level validation to prevent invalid tier values 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add comments explaining rollbackFromActivityId intentionally lacks FK (allows provenance to survive source activity deletion for audit) - Add TODO note for contentSnapshot storage considerations - Optimize message fetching: include createdAt in AiUndoPreview to avoid double database query 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
PostgreSQL requires explicit cast when converting text column to enum type. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…messages tables and updating permission logic
- Add route tests for /api/activities/[activityId]/rollback (12 tests) - Add route tests for /api/pages/[pageId]/history (21 tests) - Add service tests for ai-undo-service (16 tests) with @scaffold label - Add service tests for rollback-service (28 tests) with @scaffold label - Add permission tests for rollback-permissions (69 tests) - Fix undo route test: remove duplicate test expecting wrong status - Add fake timers to history route tests for deterministic dates Per rubric v2: service tests use @scaffold labels for ORM chain mocks pending repository seam refactoring. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add "Restore this version" action to activity items in: - ActivityDashboard (middle panel for /dashboard/activity and drive activity) - SidebarActivityTab (right sidebar context-aware activity feed) Changes: - ActivityItem: Add hover dropdown menu with rollback action and confirmation dialog - ActivityTimeline: Pass context and onRollback handler to items - ActivityDashboard: Handle rollback with proper context mapping (user→user_dashboard, drive→drive) - SidebarActivityTab: Add rollback UI with context-aware scoping (page/drive/user_dashboard) Rollback is properly scoped by context to prevent unintended changes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add compliance tests for: - logRollbackActivity: validates rollback operation logging with source activity reference - logConversationUndo: validates conversation undo logging for both modes 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add 'create' as rollbackable operation (trash resource to undo creation) - Support both global messages and page chat messages in rollback - Fix AI undo timing to include tool calls from preceding message - Handle message create rollback by deactivating the message 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Fix build failure: replace dynamic db.query[] bracket notation with explicit conditional to resolve TypeScript union type error - Extract checkUndoPermissions() helper to eliminate 31 lines of duplicated permission logic between GET and POST handlers - Add existingPreview parameter to executeAiUndo() to avoid redundant preview computation (was being called twice per request) - Simplify context determination: remove redundant isAiGenerated check since query already filters for it - Update tests to match new 4-parameter executeAiUndo signature - Add clarifying comment explaining partial failure tests document defensive handling (actual impl uses all-or-nothing transaction) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Update test expectations to match the new behavior where 'create' is a rollbackable operation (rolling back a create = trashing the resource). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
When executeAiUndo catches an error, the transaction has been rolled back so no changes were committed. Reset messagesDeleted and activitiesRolledBack to 0 to accurately reflect this. Also simplify the route handler to always return 500 on failure since partial success (207) is now unreachable with all-or-nothing semantics. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- useBreadcrumbs: Fix isLoading expectation for null pageId When pageId is null, isLoading correctly returns false (nothing to load) - usePermissions: Fix SWR mock to trigger onSuccess callback isPaused requires hasLoadedRef.current=true, which is set by onSuccess 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Remove `&& result.errors.length > 0` condition so any success:false triggers 500 response, not just failures with non-empty errors - Add test for empty errors array edge case - Enhance usePermissions test to properly validate after onSuccess 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The undo feature was always showing "Failed to undo changes" due to incorrect usage of the post() helper from auth-fetch. The post() function: - Returns parsed JSON on success (not a Response object) - Throws an error on non-2xx responses Components were incorrectly checking res.ok (undefined on parsed JSON) and calling res.json() on already-parsed objects, causing the error branch to always trigger. Changes: - Fix UndoAiChangesDialog to properly use post() - await and catch errors - Fix ActivityDashboard, SidebarActivityTab, VersionHistoryPanel (same pattern) - Update page-write-tools activity logging to pass previousValues for rollback support (replace_lines, rename_page, trash/restore, move_page, edit_sheet_cells now store original state for proper undo) 🤖 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 consolidates error handling across undo and rollback operations by simplifying API response processing in multiple components, updating the undo endpoint to uniformly return 500 on failure, expanding activity logging with rollback metadata, and adjusting permission rules to classify 'create' operations as rollbackable. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
apps/web/src/hooks/__tests__/usePermissions.test.ts (2)
41-54: Type assertion for onSuccess config parameter could be improved.Line 45 uses
{} as neverfor the third parameter of theonSuccesscallback. While this works because the actualonSuccesshandler inusePermissionsdoesn't use this parameter, the type assertion is semantically incorrect. The SWRonSuccesssignature expects aReadonly<PublicConfiguration<...>>type for the third parameter.Consider using a more appropriate type assertion for better type safety:
🔎 Suggested improvements
// Mock SWR and trigger onSuccess to set hasLoadedRef.current = true vi.mocked(useSWR).mockImplementation((key, fetcher, config) => { // Trigger onSuccess to simulate initial load completed if (config?.onSuccess) { - config.onSuccess({ canView: true, canEdit: true, canShare: true, canDelete: true }, key as string, {} as never); + config.onSuccess({ canView: true, canEdit: true, canShare: true, canDelete: true }, key as string, {} as any); } return { data: { canView: true, canEdit: true, canShare: true, canDelete: true }, error: undefined, isLoading: false, mutate: vi.fn(), isValidating: false, } as SWRResponse; });This change should be applied to all three test cases (lines 45, 77, and 107).
42-54: Consider extracting common mock implementation into a helper.The same mock implementation pattern is repeated in all three tests. While the current approach is clear and readable, extracting this into a helper function could reduce duplication and improve maintainability.
🔎 Suggested refactor
// Add helper function before the describe block function mockUseSWRWithOnSuccess(permissions = { canView: true, canEdit: true, canShare: true, canDelete: true }) { vi.mocked(useSWR).mockImplementation((key, fetcher, config) => { // Trigger onSuccess to simulate initial load completed if (config?.onSuccess) { config.onSuccess(permissions, key as string, {} as any); } return { data: permissions, error: undefined, isLoading: false, mutate: vi.fn(), isValidating: false, } as SWRResponse; }); } // Then use in tests: it('given user is editing a document, should pause permission revalidation', () => { vi.mocked(useEditingStore).mockReturnValue(true); mockUseSWRWithOnSuccess(); renderHook(() => usePermissions('page-123')); // ... assertions });Also applies to: 74-86, 104-116
apps/web/src/lib/ai/tools/page-write-tools.ts (1)
613-616: Consider adding newValues for completeness.The trash and restore operations correctly log
previousValuesfor rollback support, but unlike other operations (replace_lines, rename_page, move_page, edit_sheet_cells), they don't lognewValues.While the current implementation is sufficient for rollback functionality, adding
newValueswould improve:
- Consistency with other tools that log both before/after states
- Verification that the operation succeeded as expected
- Audit trail completeness
💡 Suggested enhancement
For trash operation (line 613-616):
logPageActivityAsync(userId, 'trash', { id: page.id, title: page.title, driveId: page.driveId, }, context as ToolExecutionContext, { metadata: { withChildren, childrenCount }, previousValues: { isTrashed: false }, + newValues: { isTrashed: true }, + updatedFields: ['isTrashed'], });For restore operation (line 687-689):
logPageActivityAsync(userId, 'restore', { id: page.id, title: page.title, driveId: page.driveId, }, context as ToolExecutionContext, { previousValues: { isTrashed: true }, + newValues: { isTrashed: false }, + updatedFields: ['isTrashed'], });Also applies to: 687-689
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
apps/web/src/app/api/ai/chat/messages/[messageId]/undo/__tests__/route.test.tsapps/web/src/app/api/ai/chat/messages/[messageId]/undo/route.tsapps/web/src/components/activity/ActivityDashboard.tsxapps/web/src/components/ai/shared/chat/UndoAiChangesDialog.tsxapps/web/src/components/layout/middle-content/CenterPanel.tsxapps/web/src/components/layout/right-sidebar/ai-assistant/SidebarActivityTab.tsxapps/web/src/components/version-history/VersionHistoryPanel.tsxapps/web/src/hooks/__tests__/useBreadcrumbs.test.tsapps/web/src/hooks/__tests__/usePermissions.test.tsapps/web/src/lib/ai/tools/page-write-tools.tsapps/web/src/services/api/ai-undo-service.tspackages/lib/src/permissions/__tests__/rollback-permissions.test.ts
💤 Files with no reviewable changes (1)
- apps/web/src/components/layout/middle-content/CenterPanel.tsx
🧰 Additional context used
📓 Path-based instructions (9)
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/messages/[messageId]/undo/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/messages/[messageId]/undo/route.tsapps/web/src/hooks/__tests__/usePermissions.test.tsapps/web/src/components/ai/shared/chat/UndoAiChangesDialog.tsxpackages/lib/src/permissions/__tests__/rollback-permissions.test.tsapps/web/src/components/version-history/VersionHistoryPanel.tsxapps/web/src/app/api/ai/chat/messages/[messageId]/undo/__tests__/route.test.tsapps/web/src/services/api/ai-undo-service.tsapps/web/src/components/layout/right-sidebar/ai-assistant/SidebarActivityTab.tsxapps/web/src/lib/ai/tools/page-write-tools.tsapps/web/src/components/activity/ActivityDashboard.tsxapps/web/src/hooks/__tests__/useBreadcrumbs.test.ts
**/*.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/messages/[messageId]/undo/route.tsapps/web/src/hooks/__tests__/usePermissions.test.tspackages/lib/src/permissions/__tests__/rollback-permissions.test.tsapps/web/src/app/api/ai/chat/messages/[messageId]/undo/__tests__/route.test.tsapps/web/src/services/api/ai-undo-service.tsapps/web/src/lib/ai/tools/page-write-tools.tsapps/web/src/hooks/__tests__/useBreadcrumbs.test.ts
**/*.{ts,tsx,js,jsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Format code with Prettier
Files:
apps/web/src/app/api/ai/chat/messages/[messageId]/undo/route.tsapps/web/src/hooks/__tests__/usePermissions.test.tsapps/web/src/components/ai/shared/chat/UndoAiChangesDialog.tsxpackages/lib/src/permissions/__tests__/rollback-permissions.test.tsapps/web/src/components/version-history/VersionHistoryPanel.tsxapps/web/src/app/api/ai/chat/messages/[messageId]/undo/__tests__/route.test.tsapps/web/src/services/api/ai-undo-service.tsapps/web/src/components/layout/right-sidebar/ai-assistant/SidebarActivityTab.tsxapps/web/src/lib/ai/tools/page-write-tools.tsapps/web/src/components/activity/ActivityDashboard.tsxapps/web/src/hooks/__tests__/useBreadcrumbs.test.ts
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/messages/[messageId]/undo/route.tsapps/web/src/hooks/__tests__/usePermissions.test.tsapps/web/src/components/ai/shared/chat/UndoAiChangesDialog.tsxapps/web/src/components/version-history/VersionHistoryPanel.tsxapps/web/src/app/api/ai/chat/messages/[messageId]/undo/__tests__/route.test.tsapps/web/src/services/api/ai-undo-service.tsapps/web/src/components/layout/right-sidebar/ai-assistant/SidebarActivityTab.tsxapps/web/src/lib/ai/tools/page-write-tools.tsapps/web/src/components/activity/ActivityDashboard.tsxapps/web/src/hooks/__tests__/useBreadcrumbs.test.ts
**/*.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/ai/shared/chat/UndoAiChangesDialog.tsxapps/web/src/components/version-history/VersionHistoryPanel.tsxapps/web/src/components/layout/right-sidebar/ai-assistant/SidebarActivityTab.tsxapps/web/src/components/activity/ActivityDashboard.tsx
**/*.{tsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
Use Tailwind CSS and shadcn/ui components for styling and UI
Files:
apps/web/src/components/ai/shared/chat/UndoAiChangesDialog.tsxapps/web/src/components/version-history/VersionHistoryPanel.tsxapps/web/src/components/layout/right-sidebar/ai-assistant/SidebarActivityTab.tsxapps/web/src/components/activity/ActivityDashboard.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/ai/shared/chat/UndoAiChangesDialog.tsxapps/web/src/components/version-history/VersionHistoryPanel.tsxapps/web/src/components/layout/right-sidebar/ai-assistant/SidebarActivityTab.tsxapps/web/src/components/activity/ActivityDashboard.tsx
**/*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 (7)
📚 Learning: 2025-12-23T18:49:41.955Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-23T18:49:41.955Z
Learning: Applies to apps/web/src/components/**/*.{ts,tsx} : For SWR data fetching with editing protection, use `isPaused: () => hasLoadedRef.current && isEditingActive()` to allow initial fetch and only pause after, with `revalidateOnFocus: false`
Applied to files:
apps/web/src/hooks/__tests__/usePermissions.test.ts
📚 Learning: 2025-12-23T18:49:41.955Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-23T18:49:41.955Z
Learning: Applies to apps/web/src/components/**/*.{ts,tsx} : Use SWR for server state and caching with proper configuration including `revalidateOnFocus: false` for editing protection
Applied to files:
apps/web/src/hooks/__tests__/usePermissions.test.ts
📚 Learning: 2025-12-22T20:04:40.892Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-22T20:04:40.892Z
Learning: Applies to **/*.{ts,tsx} : Use centralized permission functions from `pagespace/lib/permissions` (e.g., `getUserAccessLevel`, `canUserEditPage`) instead of implementing permission logic locally
Applied to files:
apps/web/src/hooks/__tests__/usePermissions.test.ts
📚 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-23T18:49:41.955Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-23T18:49:41.955Z
Learning: Applies to apps/web/src/**/*.{ts,tsx} : Use Vercel AI SDK with async/await for all AI operations and streaming
Applied to files:
apps/web/src/components/ai/shared/chat/UndoAiChangesDialog.tsx
📚 Learning: 2025-12-22T20:04:40.892Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-22T20:04:40.892Z
Learning: Applies to **/*ai*.{ts,tsx} : Use Vercel AI SDK for AI integrations
Applied to files:
apps/web/src/components/ai/shared/chat/UndoAiChangesDialog.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/SidebarActivityTab.tsx
🧬 Code graph analysis (4)
apps/web/src/hooks/__tests__/usePermissions.test.ts (1)
apps/web/src/hooks/usePermissions.ts (1)
usePermissions(33-98)
apps/web/src/app/api/ai/chat/messages/[messageId]/undo/__tests__/route.test.ts (2)
apps/web/src/services/api/ai-undo-service.ts (1)
executeAiUndo(254-368)apps/web/src/app/api/ai/chat/messages/[messageId]/undo/route.ts (1)
POST(112-186)
apps/web/src/lib/ai/tools/page-write-tools.ts (1)
apps/web/src/lib/ai/core/types.ts (1)
ToolExecutionContext(8-37)
apps/web/src/hooks/__tests__/useBreadcrumbs.test.ts (1)
apps/web/src/hooks/useBreadcrumbs.ts (1)
useBreadcrumbs(23-50)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Unit Tests
🔇 Additional comments (19)
apps/web/src/hooks/__tests__/useBreadcrumbs.test.ts (1)
175-182: LGTM! Test correctly fixed to match hook implementation.The change correctly fixes the test assertion. The
useBreadcrumbshook computesisLoadingas!error && !data && !!pageId, which evaluates tofalsewhenpageIdisnull(since!!nullisfalse). The updated test description, comment, and assertion all accurately reflect this behavior.apps/web/src/components/version-history/VersionHistoryPanel.tsx (1)
132-150: LGTM! Correct usage of post() helper.The refactored code correctly uses the
post()helper, which returns parsed JSON on success and throws on non-2xx responses. Error handling is properly delegated to the catch block, and the success path remains unchanged.apps/web/src/services/api/ai-undo-service.ts (1)
354-367: LGTM! Correct handling of transactional failure.Returning zeroed counters on failure correctly reflects the all-or-nothing transaction semantics. Since the entire transaction is rolled back on any error (line 281), no changes are actually applied, so returning 0 for both counters is accurate and prevents misleading partial-success indicators.
apps/web/src/components/ai/shared/chat/UndoAiChangesDialog.tsx (1)
74-92: LGTM! Correct usage with helpful documentation.The refactored code correctly uses the
post()helper and includes helpful comments explaining the behavior. Error handling is properly delegated to the catch block, and the success flow remains intact.apps/web/src/app/api/ai/chat/messages/[messageId]/undo/route.ts (1)
159-168: LGTM! Uniform failure handling.The simplified error handling correctly implements all-or-nothing transaction semantics with a clear, uniform failure message. The 500 status code is appropriate, and including the result object provides debugging information while maintaining a consistent user-facing message.
apps/web/src/components/layout/right-sidebar/ai-assistant/SidebarActivityTab.tsx (1)
250-272: LGTM! Consistent implementation.The refactored rollback handler correctly uses the
post()helper without manual response handling. Error handling is properly delegated to the catch block, and the success path maintains the expected behavior (toast notification and activity list refresh).apps/web/src/components/activity/ActivityDashboard.tsx (1)
212-225: LGTM! Correct implementation.The refactored rollback handler correctly uses the
post()helper. The useCallback dependencies are accurate, error handling is properly delegated to the catch block, and the success path maintains the expected behavior.apps/web/src/app/api/ai/chat/messages/[messageId]/undo/__tests__/route.test.ts (1)
349-381: LGTM! Tests correctly validate the new failure contract.The updated failure tests correctly validate the uniform 500 error response with the message "Undo failed. No changes were applied." The mock data properly reflects transactional all-or-nothing semantics with zeroed counters, and the tests cover both scenarios (with and without specific error messages).
packages/lib/src/permissions/__tests__/rollback-permissions.test.ts (3)
159-167: Correct update of test case to reflect 'create' reclassification.The test correctly switches from 'create' to 'signup' to validate non-rollbackable operation behavior, since 'create' is now classified as rollbackable. This maintains test validity and consistency.
195-209: Parametrized test correctly updated to exclude 'create'.The test array appropriately excludes 'create' and uses representative non-rollbackable operations (signup, login, logout) to validate rejection logic. The subset approach is efficient while maintaining test coverage.
55-73: Implementation correctly marks 'create' as rollbackable.The implementation in
rollback-permissions.tsis consistent with the test changes. TheisRollbackableOperation()function includes 'create' in its rollbackable operations list (line 169), and the nonRollbackableOperations correctly defines only ['signup', 'login', 'logout']. The permission checks across all contexts (page, drive, ai_tool, user_dashboard) are properly configured to allow rollback of create operations for authorized users.apps/web/src/hooks/__tests__/usePermissions.test.ts (1)
36-128: Well-structured tests that properly simulate initial load completion.The changes successfully implement the testing strategy for SWR editing protection by:
- Triggering the
onSuccesscallback to simulate initial load completion (settinghasLoadedRef.current = true)- Verifying
isPausedbehavior in three scenarios: editing active, AI streaming active, and no activity- Updating comments to clarify the "after initial load" behavior
The test logic correctly matches the implementation in
usePermissions.tswhereisPaused: () => hasLoadedRef.current && isAnyActive().Based on learnings, the isPaused implementation tested here follows the documented pattern for SWR data fetching with editing protection.
apps/web/src/lib/ai/tools/page-write-tools.ts (7)
29-78: Excellent implementation of rollback-enabled activity logging!The extension of
logPageActivityAsyncto acceptpreviousValues,newValues, andupdatedFieldsis well-designed:
- Properly typed optional parameter maintains backward compatibility
- Metadata merging correctly preserves agent chain context
- Both success and error branches consistently propagate rollback fields
- Clean separation of concerns between metadata and rollback-specific data
This provides a solid foundation for the undo/rollback feature.
340-354: Perfect rollback logging for content updates!The
replace_linestool correctly captures all necessary data for rollback:
previousValuesstores original content before modificationnewValuesstores new content after changesupdatedFieldsarray accurately identifies the modified field- Metadata provides useful context (linesChanged, changeType)
The timing is correct—original
page.contentis captured before the database update at line 330.
552-556: Well-implemented rollback support for page renaming.The logging correctly captures the title change with proper before/after values:
previousValuespreserves the original title for rollbacknewValuesrecords the new title for verification- State capture timing is accurate relative to the DB update at line 537
795-800: Comprehensive rollback data for page moves.The
move_pagetool properly logs all location changes:
- Both
parentIdandpositionare captured in previousValues/newValuesupdatedFieldscorrectly lists both modified fields- The metadata provides quick reference to the new location
This enables complete restoration of the page's original position during rollback.
898-909: Solid rollback implementation for sheet edits.Similar to
replace_lines, this correctly implements full rollback support for sheet content:
- Original content captured before database update
- Both previousValues and newValues properly populated
- Helpful comment documents rollback usage
- Metadata provides useful context about the scope of changes
463-465: Appropriate logging for create operations.The
create_pagetool logs onlymetadata(pageType, parentId) withoutpreviousValuesornewValues, which is correct for create operations:
- There's no prior state to capture for a newly created page
- Rollback of a create operation means deleting the created page
- The activity log already contains the page ID and operation type, which is sufficient for rollback logic
This aligns with the PR objective to make create operations rollbackable.
1-952: Excellent implementation of rollback-enabled activity logging across all tools!This file successfully implements the PR objectives by enhancing activity logging to support the undo/rollback feature. Key strengths:
✅ Consistent rollback data capture across all content-modifying operations
✅ Proper timing of state capture (before DB updates)
✅ Clean, backward-compatible API design
✅ Strong type safety throughout
✅ Follows coding guidelines (centralized permissions, repository pattern, noanytypes)
✅ Well-documented with helpful comments explaining rollback usageThe implementation provides all necessary data for the rollback endpoints to restore pages to their previous states.
Summary
post()helper from auth-fetchDetails
The
post()function fromauth-fetch.ts:Components were incorrectly:
<Response>type annotationres.okwhich isundefinedon parsed JSON →!res.okwas always trueres.json()which fails becauseresis already a plain objectThis caused the error branch to always trigger, even when undo succeeded.
Changes
Dialog/Component Fixes
UndoAiChangesDialog.tsx- Fixed to properly awaitpost()and catch errorsActivityDashboard.tsx- Same pattern fixSidebarActivityTab.tsx- Same pattern fixVersionHistoryPanel.tsx- Same pattern fixActivity Logging (for proper rollback)
logPageActivityAsynchelper to acceptpreviousValues,newValues, andupdatedFieldsreplace_lines- Now stores originalpage.contentfor rollbackrename_page- Now stores originalpage.titlefor rollbacktrash/restore- Now storesisTrashedstate for rollbackmove_page- Now stores originalparentIdandpositionfor rollbackedit_sheet_cells- Now stores originalpage.contentfor rollbackTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.