Repository navigation
feat: Version History & Rollback - #118
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>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
WalkthroughAdds an AI-driven undo service with preview and execute flows, standardizes undo failure responses to always return HTTP 500 with a single error message, expands rollback-permissions tests, and updates several component/hook tests and a header prop usage. Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant API as Undo API Endpoint
participant Service as AI Undo Service
participant Rollback as Rollback Service
participant DB as Database
rect rgb(235,245,255)
Note over Client,Service: Preview flow
Client->>API: POST /api/ai/chat/messages/[messageId]/undo (preview)
API->>Service: previewAiUndo(messageId, userId)
Service->>DB: fetch target message & subsequent messages
Service->>Service: derive drive/context & collect activities
Service->>Rollback: previewRollback(activities)
Rollback-->>Service: feasibility per activity
Service-->>API: AiUndoPreview (affected messages, rollback feasibility)
API-->>Client: 200 OK + preview
end
rect rgb(235,255,235)
Note over Client,Service: Execute flow
Client->>API: POST /api/ai/chat/messages/[messageId]/undo (execute)
API->>Service: executeAiUndo(messageId, userId, mode)
Service->>DB: BEGIN TRANSACTION
alt mode == messages_and_changes
Service->>Rollback: executeRollback(activities) in reverse order
Rollback->>DB: apply rollback ops
Rollback-->>Service: results
end
Service->>DB: soft-delete messages from target forward
Service->>DB: COMMIT
Service-->>API: AiUndoResult (success)
API-->>Client: 200 OK + result
end
rect rgb(255,235,235)
Note over Client,Service: Error path
Service->>DB: operation fails
Service->>DB: ROLLBACK
Service-->>API: AiUndoResult (success:false, errors)
API-->>Client: 500 Internal Server Error ("Undo failed. No changes were applied.")
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 10
🧹 Nitpick comments (6)
apps/web/src/app/api/pages/[pageId]/history/route.ts (1)
92-97: Rollback eligibility logic is consistent.The
canRollbackcomputation matches the drive history endpoint, checking both operation type and presence of restorable state. This consistency is good for maintainability.Consider extracting this logic into a shared helper to ensure it stays synchronized.
apps/web/src/components/version-history/RollbackConfirmDialog.tsx (2)
28-37:activityIdprop is declared but unused.The
activityIdprop is destructured but never referenced in the component. If it's intended for future use or logging, consider removing it to avoid confusion, or add a comment explaining its purpose.🔎 If not needed, remove from props
export function RollbackConfirmDialog({ open, onOpenChange, - activityId, resourceTitle, operation, timestamp, warnings, onConfirm, }: RollbackConfirmDialogProps) {And update the interface:
interface RollbackConfirmDialogProps { open: boolean; onOpenChange: (open: boolean) => void; - activityId: string; resourceTitle: string | null;
40-50: Error handling only logs to console — consider user feedback.When
onConfirmthrows, the error is logged but the user receives no notification. The dialog remains open due to the error, which is good, but a toast or inline error message would improve UX.🔎 Example with toast notification
+'use client'; + +import { useState } from 'react'; +import { AlertTriangle, History, Loader2 } from 'lucide-react'; +import { toast } from 'sonner'; // or your toast library // ... other imports const handleConfirm = async () => { setIsLoading(true); try { await onConfirm(); onOpenChange(false); } catch (error) { console.error('Rollback failed:', error); + toast.error('Failed to restore version. Please try again.'); } finally { setIsLoading(false); } };apps/web/src/components/version-history/VersionHistoryPanel.tsx (1)
127-155: Consider removing error re-throw or document the intent.The
handleRollbackfunction displays an error toast and then re-throws the error (line 153). This pattern is unusual because:
- The error is already handled and displayed to the user via toast
- Re-throwing causes the error to propagate, but there's no try-catch wrapper in the calling code
- This will result in an unhandled promise rejection
If the intent is to prevent the
RollbackConfirmDialogfrom closing on error, this should be documented. Otherwise, consider removing thethrowstatement.🔎 Proposed fix
} catch (error) { toast({ title: 'Error', description: error instanceof Error ? error.message : 'Failed to rollback', variant: 'destructive', }); - throw error; } };apps/web/src/services/api/rollback-service.ts (2)
361-403: Consider wrapping database updates in transactions for atomicity.The
rollbackPageChangefunction updates the page table directly without using a transaction (lines 393-400). While this may work for simple cases, consider:
- If multiple fields are being restored and the update fails partway through, data could be inconsistent
- The activity logging happens after the update, so they're not atomic
- For complex rollbacks (especially in other handlers), transactions would provide better guarantees
🔎 Example transaction pattern
await db.transaction(async (tx) => { await tx .update(pages) .set({ ...updateData, updatedAt: new Date(), }) .where(eq(pages.id, activity.pageId)); // Any related updates would go here });Note: The activity logging in
executeRollbackwould still be outside the transaction, which is likely fine for audit purposes.
226-238: Consider using deep equality comparison instead of JSON.stringify.The change detection logic uses
JSON.stringifyfor comparison (lines 228-229), which can produce false positives due to:
- Key ordering differences in objects
- Date serialization variations
undefinedvsnulltreatment- Floating-point precision
Since this only affects warnings shown to users, it's not critical, but consider using a proper deep equality function (e.g., from lodash or a similar library) for more accurate detection.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
apps/web/src/app/api/activities/[activityId]/rollback/route.tsapps/web/src/app/api/activities/[activityId]/route.tsapps/web/src/app/api/drives/[driveId]/history/route.tsapps/web/src/app/api/pages/[pageId]/history/route.tsapps/web/src/components/activity/constants.tsapps/web/src/components/version-history/RollbackConfirmDialog.tsxapps/web/src/components/version-history/VersionHistoryItem.tsxapps/web/src/components/version-history/VersionHistoryPanel.tsxapps/web/src/components/version-history/index.tsapps/web/src/services/api/index.tsapps/web/src/services/api/rollback-service.tspackages/db/drizzle/0025_icy_wallflower.sqlpackages/db/drizzle/meta/0025_snapshot.jsonpackages/db/drizzle/meta/_journal.jsonpackages/db/src/schema/monitoring.tspackages/lib/src/monitoring/activity-logger.tspackages/lib/src/permissions/index.tspackages/lib/src/permissions/rollback-permissions.ts
🧰 Additional context used
📓 Path-based instructions (10)
apps/web/src/app/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
apps/web/src/app/**/*.ts: In Next.js 15 route handlers with dynamic routes, you MUST awaitcontext.paramsbefore destructuring because params are Promise objects
Get request body usingconst body = await request.json();
Get search params usingconst { searchParams } = new URL(request.url);
Return JSON responses usingreturn Response.json(data)orreturn NextResponse.json(data)
Files:
apps/web/src/app/api/activities/[activityId]/rollback/route.tsapps/web/src/app/api/pages/[pageId]/history/route.tsapps/web/src/app/api/activities/[activityId]/route.tsapps/web/src/app/api/drives/[driveId]/history/route.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Never use
anytypes in TypeScript code - always use proper TypeScript types
**/*.{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
Files:
apps/web/src/app/api/activities/[activityId]/rollback/route.tsapps/web/src/components/version-history/index.tspackages/lib/src/permissions/index.tspackages/db/src/schema/monitoring.tsapps/web/src/app/api/pages/[pageId]/history/route.tsapps/web/src/components/activity/constants.tsapps/web/src/components/version-history/VersionHistoryItem.tsxapps/web/src/components/version-history/VersionHistoryPanel.tsxpackages/lib/src/monitoring/activity-logger.tsapps/web/src/app/api/activities/[activityId]/route.tsapps/web/src/services/api/rollback-service.tsapps/web/src/services/api/index.tspackages/lib/src/permissions/rollback-permissions.tsapps/web/src/app/api/drives/[driveId]/history/route.tsapps/web/src/components/version-history/RollbackConfirmDialog.tsx
apps/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/**/*.{ts,tsx}: Use centralized permission functions from@pagespace/lib/permissionsfor access control, such asgetUserAccessLevel()andcanUserEditPage()
Always use Drizzle client from@pagespace/dbfor database access instead of direct database connections
Always use message parts structure withpartsarray containing objects withtypeandtextfields when constructing messages for AI
Files:
apps/web/src/app/api/activities/[activityId]/rollback/route.tsapps/web/src/components/version-history/index.tsapps/web/src/app/api/pages/[pageId]/history/route.tsapps/web/src/components/activity/constants.tsapps/web/src/components/version-history/VersionHistoryItem.tsxapps/web/src/components/version-history/VersionHistoryPanel.tsxapps/web/src/app/api/activities/[activityId]/route.tsapps/web/src/services/api/rollback-service.tsapps/web/src/services/api/index.tsapps/web/src/app/api/drives/[driveId]/history/route.tsapps/web/src/components/version-history/RollbackConfirmDialog.tsx
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);
Files:
apps/web/src/app/api/activities/[activityId]/rollback/route.tsapps/web/src/app/api/pages/[pageId]/history/route.tsapps/web/src/app/api/activities/[activityId]/route.tsapps/web/src/app/api/drives/[driveId]/history/route.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/activities/[activityId]/rollback/route.tsapps/web/src/components/version-history/index.tspackages/lib/src/permissions/index.tspackages/db/src/schema/monitoring.tsapps/web/src/app/api/pages/[pageId]/history/route.tsapps/web/src/components/activity/constants.tspackages/lib/src/monitoring/activity-logger.tsapps/web/src/app/api/activities/[activityId]/route.tsapps/web/src/services/api/rollback-service.tsapps/web/src/services/api/index.tspackages/lib/src/permissions/rollback-permissions.tsapps/web/src/app/api/drives/[driveId]/history/route.ts
**/*.{ts,tsx,js,jsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Format code with Prettier
Files:
apps/web/src/app/api/activities/[activityId]/rollback/route.tsapps/web/src/components/version-history/index.tspackages/db/drizzle/meta/_journal.jsonpackages/lib/src/permissions/index.tspackages/db/src/schema/monitoring.tsapps/web/src/app/api/pages/[pageId]/history/route.tsapps/web/src/components/activity/constants.tsapps/web/src/components/version-history/VersionHistoryItem.tsxapps/web/src/components/version-history/VersionHistoryPanel.tsxpackages/lib/src/monitoring/activity-logger.tsapps/web/src/app/api/activities/[activityId]/route.tsapps/web/src/services/api/rollback-service.tsapps/web/src/services/api/index.tspackages/lib/src/permissions/rollback-permissions.tsapps/web/src/app/api/drives/[driveId]/history/route.tsapps/web/src/components/version-history/RollbackConfirmDialog.tsx
packages/db/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Drizzle ORM for database queries with PostgreSQL
Files:
packages/db/src/schema/monitoring.ts
apps/web/src/**/*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
apps/web/src/**/*.tsx: For document editing, register editing state usinguseEditingStore.getState().startEditing()andendEditing()to prevent unwanted UI refreshes
For AI streaming operations, register streaming state usinguseEditingStore.getState().startStreaming()andendStreaming()to prevent unwanted UI refreshes
When using SWR, checkuseEditingStorestate withisAnyActive()and setisPausedto prevent data refreshes during editing or streaming
Files:
apps/web/src/components/version-history/VersionHistoryItem.tsxapps/web/src/components/version-history/VersionHistoryPanel.tsxapps/web/src/components/version-history/RollbackConfirmDialog.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/version-history/VersionHistoryItem.tsxapps/web/src/components/version-history/VersionHistoryPanel.tsxapps/web/src/components/version-history/RollbackConfirmDialog.tsx
**/*.{tsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
Use Tailwind CSS and shadcn/ui components for styling and UI
Files:
apps/web/src/components/version-history/VersionHistoryItem.tsxapps/web/src/components/version-history/VersionHistoryPanel.tsxapps/web/src/components/version-history/RollbackConfirmDialog.tsx
🧠 Learnings (8)
📚 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:
packages/lib/src/permissions/index.tspackages/lib/src/permissions/rollback-permissions.ts
📚 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 app/**/*.{ts,tsx} : Import and use `getUserAccessLevel()` and `canUserEditPage()` from `pagespace/lib/permissions` for centralized permission logic
Applied to files:
packages/lib/src/permissions/index.tspackages/lib/src/permissions/rollback-permissions.ts
📚 Learning: 2025-12-14T14:54:15.319Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T14:54:15.319Z
Learning: Applies to apps/**/*.{ts,tsx} : Use centralized permission functions from `pagespace/lib/permissions` for access control, such as `getUserAccessLevel()` and `canUserEditPage()`
Applied to files:
packages/lib/src/permissions/index.tspackages/lib/src/permissions/rollback-permissions.ts
📚 Learning: 2025-12-14T14:54:38.009Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T14:54:38.009Z
Learning: Applies to apps/**/*.ts : Always use centralized permission functions from `pagespace/lib/permissions` for access control logic (e.g., `getUserAccessLevel`, `canUserEditPage`)
Applied to files:
packages/lib/src/permissions/index.tspackages/lib/src/permissions/rollback-permissions.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 apps/web/src/app/**/route.{ts,tsx} : Get search parameters using `const { searchParams } = new URL(request.url);`
Applied to files:
apps/web/src/app/api/pages/[pageId]/history/route.tsapps/web/src/app/api/activities/[activityId]/route.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 apps/web/src/app/**/route.{ts,tsx} : In Next.js 15 route handlers, `params` in dynamic routes are Promise objects and MUST be awaited before destructuring
Applied to files:
apps/web/src/app/api/activities/[activityId]/route.tsapps/web/src/app/api/drives/[driveId]/history/route.ts
📚 Learning: 2025-12-14T14:54:38.009Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T14:54:38.009Z
Learning: Applies to apps/web/app/**/{route,route.ts,route.js} : In Next.js 15, `params` in dynamic routes are Promise objects. You MUST await `context.params` before destructuring.
Applied to files:
apps/web/src/app/api/activities/[activityId]/route.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 apps/web/src/app/**/route.{ts,tsx} : Get request body using `const body = await request.json();`
Applied to files:
apps/web/src/app/api/activities/[activityId]/route.ts
🧬 Code graph analysis (5)
apps/web/src/components/version-history/VersionHistoryItem.tsx (8)
apps/web/src/components/activity/types.ts (1)
ActivityLog(10-31)apps/web/src/components/version-history/index.ts (2)
VersionHistoryItem(2-2)RollbackConfirmDialog(3-3)apps/web/src/components/activity/constants.ts (3)
operationConfig(17-30)defaultOperationConfig(39-43)resourceTypeIcons(32-37)apps/web/src/components/activity/utils.ts (1)
getInitials(4-16)apps/web/src/components/ui/badge.tsx (1)
Badge(46-46)apps/web/src/components/ui/dropdown-menu.tsx (4)
DropdownMenu(242-242)DropdownMenuTrigger(244-244)DropdownMenuContent(245-245)DropdownMenuItem(248-248)apps/web/src/components/ui/button.tsx (1)
Button(59-59)apps/web/src/components/version-history/RollbackConfirmDialog.tsx (1)
RollbackConfirmDialog(28-113)
apps/web/src/components/version-history/VersionHistoryPanel.tsx (2)
apps/web/src/components/activity/types.ts (1)
ActivityLog(10-31)apps/web/src/components/version-history/VersionHistoryItem.tsx (1)
VersionHistoryItem(26-146)
apps/web/src/app/api/activities/[activityId]/route.ts (3)
apps/web/src/lib/auth/index.ts (2)
authenticateRequestWithOptions(216-271)isAuthError(204-206)packages/lib/src/permissions/rollback-permissions.ts (1)
RollbackContext(15-19)apps/web/src/services/api/rollback-service.ts (2)
getActivityById(87-130)previewRollback(135-255)
packages/lib/src/permissions/rollback-permissions.ts (2)
packages/lib/src/monitoring/activity-logger.ts (1)
ActivityResourceType(88-100)packages/lib/src/permissions/index.ts (1)
isDriveOwnerOrAdmin(13-13)
apps/web/src/components/version-history/RollbackConfirmDialog.tsx (3)
apps/web/src/components/version-history/index.ts (1)
RollbackConfirmDialog(3-3)apps/processor/src/logger.ts (1)
error(57-63)apps/web/src/components/ui/badge.tsx (1)
Badge(46-46)
🪛 Biome (2.1.2)
packages/lib/src/permissions/rollback-permissions.ts
[error] 99-99: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 126-126: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
⏰ 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 (31)
packages/db/drizzle/meta/_journal.json (1)
180-186: LGTM!The migration journal entry is properly structured and follows Drizzle's standard format. The index increments correctly from the previous entry.
packages/lib/src/monitoring/activity-logger.ts (1)
84-86: LGTM!The addition of the
rollbackoperation to theActivityOperationtype is correctly structured and properly categorized with a descriptive comment.packages/lib/src/permissions/rollback-permissions.ts (3)
15-41: LGTM!The type definitions are well-structured and properly documented. The
RollbackContextcovers all expected contexts, and the interfaces correctly model nullable fields to match the database schema.
57-72: LGTM!The guards against non-rollbackable operations are well-reasoned. Preventing rollback of
create,signup,login, andlogoutoperations makes sense since these don't have previous state to restore. The prevention of rollback chains is also appropriate.
146-182: LGTM!The helper functions correctly identify rollbackable resource types and operations. The inclusion of
deletein the rollbackable operations list is appropriate, as delete operations can store the deleted resource's state inpreviousValues, enabling restoration.packages/lib/src/permissions/index.ts (1)
19-20: LGTM!The export statement follows the existing pattern and correctly exposes the rollback permission utilities through the centralized permissions module.
apps/web/src/components/version-history/index.ts (1)
1-3: LGTM!The barrel export file follows standard patterns and simplifies component imports for consumers.
packages/db/drizzle/0025_icy_wallflower.sql (2)
1-1: LGTM!The addition of the
rollbackvalue to theactivity_operationenum is correctly formatted and aligns with the TypeScript type definition.
11-12: Schema columns added correctly, but verify integration with logActivity.The new columns
contentFormatandrollbackFromActivityIdare properly defined as nullable text fields, which supports backward compatibility. However, as noted in the review ofactivity-logger.ts, these columns may not be properly populated by thelogActivityfunction, which could result in these values being stored inmetadatainstead of the dedicated columns.Ensure the
ActivityLogInputinterface andlogActivityfunction are updated to support these new fields directly.apps/web/src/components/activity/constants.ts (2)
6-6: LGTM!The
Historyicon import is properly added and alphabetically ordered with other lucide-react imports.
29-29: LGTM!The rollback operation configuration is well-defined with an appropriate icon, clear label, and consistent variant choice that matches similar restoration operations.
apps/web/src/app/api/activities/[activityId]/rollback/route.ts (5)
1-12: LGTM!The imports are correctly structured, authentication options appropriately require CSRF protection for this state-changing endpoint, and the Zod v4 schema properly validates the request body with sensible defaults.
19-29: LGTM!The function signature correctly handles Next.js 15's async params by awaiting
context.paramsbefore destructuring, as required by the coding guidelines. Authentication is properly enforced.
31-50: LGTM!The request body parsing and validation follow best practices with proper error handling. JSON parsing is safely wrapped in try-catch, and Zod's
safeParseis used for non-throwing validation with clear error messages.
52-59: LGTM!The dry-run preview logic correctly returns early without mutating state, providing a safe preview mechanism. The response properly indicates the dry-run status.
61-77: LGTM!The rollback execution logic properly handles both success and failure cases with appropriate status codes and comprehensive response data including warnings and restored values.
apps/web/src/app/api/drives/[driveId]/history/route.ts (2)
26-35: LGTM on Next.js 15 params handling.The route correctly types
context.paramsasPromise<{ driveId: string }>and awaits it before destructuring. This follows the Next.js 15 requirement for async route params. Based on learnings, this is the correct pattern.
93-97: Rollback eligibility logic is sound.The
canRollbackcomputation correctly checks both that the operation is rollbackable AND that there's recoverable state (previousValues or contentSnapshot). This aligns with the preview logic inrollback-service.ts.packages/db/src/schema/monitoring.ts (3)
390-393: Addition of 'rollback' operation to enum is appropriate.The new
rollbackoperation type enables tracking of rollback activities in the audit trail, supporting the non-destructive rollback approach described in the PR objectives.
444-449: New rollback-related fields are well-designed.The fields support the rollback feature:
contentSnapshot: Stores full content state for restorationcontentFormat: Enables proper parsing during rollback (text/html/json/tiptap)rollbackFromActivityId: Links rollback entries to their source activityAll fields are nullable, making this migration safe for existing data.
485-495: Retention policies table design is clean.The table correctly models plan-based retention with:
- Unique constraint on
subscriptionTierfor fast lookups- Integer
retentionDayswith -1 convention for unlimited (matches PR spec: Free=7d, Pro=30d, Business=90d, Founder=unlimited)- Proper timestamp tracking with
$onUpdateforupdatedAtapps/web/src/app/api/activities/[activityId]/route.ts (2)
18-27: LGTM on Next.js 15 route handler pattern.The handler correctly awaits
context.paramsbefore destructuringactivityId, following Next.js 15 requirements. Based on learnings, this is the correct approach.
57-62: Response structure is well-organized.The response provides all necessary information for the UI to display rollback eligibility and reasons, enabling the
VersionHistoryItemcomponent to show appropriate actions.apps/web/src/app/api/pages/[pageId]/history/route.ts (2)
25-35: LGTM on Next.js 15 route handler and authorization.The handler correctly:
- Awaits
context.paramsbefore destructuring (Next.js 15 requirement)- Uses centralized
canUserViewPagepermission function (per coding guidelines)Based on learnings and coding guidelines, this follows the required patterns.
67-79: Retention logic is consistent across history endpoints.The retention cutoff calculation mirrors the drive history endpoint, ensuring consistent behavior. The logic correctly handles:
- Unlimited retention (
retentionDays <= 0)- User-provided
startDatethat's more recent than retention cutoff- Automatic enforcement of retention window
apps/web/src/components/version-history/RollbackConfirmDialog.tsx (1)
52-112: Clean UI implementation with good accessibility.The dialog properly:
- Disables interactive elements during loading
- Provides clear visual feedback with the spinner
- Supports dark mode with appropriate color classes
- Uses semantic AlertDialog components
- Displays warnings prominently when present
apps/web/src/services/api/index.ts (1)
44-57: LGTM!The rollback service exports follow the established pattern in this barrel export file. Both functions and types are properly exported for consumption by API routes and UI components.
apps/web/src/services/api/rollback-service.ts (2)
278-356: Good integration with activity logging and error handling.The
executeRollbackfunction properly:
- Gets actor info for audit trail (line 280)
- Delegates to resource-specific handlers
- Logs the rollback activity with full context (lines 319-335)
- Handles errors gracefully with appropriate logging
The use of optional chaining to convert
nulltoundefined(e.g., line 325, 327) ensures type safety with the logging API.
700-729: Queryretention_policiestable instead of using hardcoded defaults.The
getUserRetentionDaysfunction uses hardcoded tier-to-retention mappings (lines 702-707) instead of querying theretention_policiestable that exists in the schema. Update the function to fetch retention values from the database, allowing policies to be configured without code changes.apps/web/src/components/version-history/VersionHistoryPanel.tsx (1)
66-67: No action needed. The context value'user_dashboard'is a valid RollbackContext type as defined inpackages/lib/src/permissions/rollback-permissions.tsand is properly handled throughout the codebase for user dashboard activity tracking.Likely an incorrect or invalid review comment.
apps/web/src/components/version-history/VersionHistoryItem.tsx (1)
20-24: No issues found. Thecontextprop values inVersionHistoryItemPropsalign perfectly with theRollbackContexttype definition. All four values ('page','drive','ai_tool','user_dashboard') are properly defined in the rollback permissions module and have complete permission logic implemented for each context.
Code reviewFound 1 issue:
PageSpace/apps/web/src/components/version-history/VersionHistoryPanel.tsx Lines 128 to 134 in c796ac9 🤖 Generated with Claude Code - If this code review was useful, please react with 👍. Otherwise, react with 👎. |
- 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>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
apps/web/src/services/api/rollback-service.ts (2)
818-822: Optimize count query using SQL COUNT() aggregate.The count query selects all activity log IDs and counts the array length. This loads all matching rows into memory, which is inefficient for large activity logs.
🔎 Proposed fix
+ import { sql } from '@pagespace/db'; + const [activities, countResult] = await Promise.all([ db .select() .from(activityLogs) .where(and(...conditions)) .orderBy(desc(activityLogs.timestamp)) .limit(limit) .offset(offset), db - .select({ count: activityLogs.id }) + .select({ count: sql<number>`count(*)` }) .from(activityLogs) .where(and(...conditions)), ]); return { activities: activities.map((a) => ({ // ... mapping })), - total: countResult.length, + total: countResult[0]?.count ?? 0, };Apply the same fix to lines 892-894 in
getDriveVersionHistory.
802-805: Validate operation parameter before type assertion to prevent query errors.The
operationparameter from user input is directly cast to an enum type without validation. Invalid operation values could cause runtime errors or unexpected query behavior.🔎 Proposed fix
+ const validOperations = ['create', 'update', 'delete', 'restore', 'reorder', 'trash', 'move', 'permission_grant', 'permission_update', 'permission_revoke', 'agent_config_update', 'rollback']; + if (operation) { + if (!validOperations.includes(operation)) { + loggers.api.warn('[RollbackService] Invalid operation filter', { operation }); + } else { - // Type assertion needed since operation comes from user input - conditions.push(eq(activityLogs.operation, operation as typeof activityLogs.operation.enumValues[number])); + conditions.push(eq(activityLogs.operation, operation as typeof activityLogs.operation.enumValues[number])); + } }Apply the same fix to line 880 in
getDriveVersionHistory.apps/web/src/components/version-history/VersionHistoryItem.tsx (1)
41-51: Improve error handling and response validation for preview fetch.The preview fetch has several issues:
- No
response.okcheck before parsing JSON - will throw on non-OK responses- Silently catches all errors - users don't know if preview failed
- Uses plain
fetch()instead offetchWithAuth()for authentication🔎 Proposed improvements
+import { fetchWithAuth } from '@/lib/auth/auth-fetch'; + const handleRestoreClick = async () => { // Fetch preview to get warnings try { - const response = await fetch(`/api/activities/${activity.id}?context=${context}`); + const response = await fetchWithAuth(`/api/activities/${activity.id}?context=${context}`); + + if (!response.ok) { + throw new Error('Failed to fetch rollback preview'); + } + const data = await response.json(); setPreviewWarnings(data.warnings || []); - } catch { + } catch (error) { + console.error('Failed to fetch rollback preview:', error); setPreviewWarnings([]); + // Note: still show dialog so user can proceed, but warnings may be incomplete } setShowConfirm(true); };
🧹 Nitpick comments (1)
apps/web/src/services/api/rollback-service.ts (1)
8-8: Consider splitting long import for better readability.The import statement from
@pagespace/dbis very long and includes many named imports, making it harder to read and maintain.🔎 Suggested improvement
-import { db, activityLogs, pages, drives, driveMembers, driveRoles, pagePermissions, users, eq, and, desc, gte, lte } from '@pagespace/db'; +import { + db, + activityLogs, + pages, + drives, + driveMembers, + driveRoles, + pagePermissions, + users, + eq, + and, + desc, + gte, + lte, +} from '@pagespace/db';
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/web/src/components/version-history/RollbackConfirmDialog.tsxapps/web/src/components/version-history/VersionHistoryItem.tsxapps/web/src/components/version-history/VersionHistoryPanel.tsxapps/web/src/services/api/rollback-service.tspackages/lib/package.json
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/src/components/version-history/RollbackConfirmDialog.tsx
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx,js,jsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Format code with Prettier
Files:
packages/lib/package.jsonapps/web/src/components/version-history/VersionHistoryItem.tsxapps/web/src/services/api/rollback-service.tsapps/web/src/components/version-history/VersionHistoryPanel.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Never use
anytypes in TypeScript code - always use proper TypeScript types
**/*.{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
Files:
apps/web/src/components/version-history/VersionHistoryItem.tsxapps/web/src/services/api/rollback-service.tsapps/web/src/components/version-history/VersionHistoryPanel.tsx
apps/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/**/*.{ts,tsx}: Use centralized permission functions from@pagespace/lib/permissionsfor access control, such asgetUserAccessLevel()andcanUserEditPage()
Always use Drizzle client from@pagespace/dbfor database access instead of direct database connections
Always use message parts structure withpartsarray containing objects withtypeandtextfields when constructing messages for AI
Files:
apps/web/src/components/version-history/VersionHistoryItem.tsxapps/web/src/services/api/rollback-service.tsapps/web/src/components/version-history/VersionHistoryPanel.tsx
apps/web/src/**/*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
apps/web/src/**/*.tsx: For document editing, register editing state usinguseEditingStore.getState().startEditing()andendEditing()to prevent unwanted UI refreshes
For AI streaming operations, register streaming state usinguseEditingStore.getState().startStreaming()andendStreaming()to prevent unwanted UI refreshes
When using SWR, checkuseEditingStorestate withisAnyActive()and setisPausedto prevent data refreshes during editing or streaming
Files:
apps/web/src/components/version-history/VersionHistoryItem.tsxapps/web/src/components/version-history/VersionHistoryPanel.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/version-history/VersionHistoryItem.tsxapps/web/src/components/version-history/VersionHistoryPanel.tsx
**/*.{tsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
Use Tailwind CSS and shadcn/ui components for styling and UI
Files:
apps/web/src/components/version-history/VersionHistoryItem.tsxapps/web/src/components/version-history/VersionHistoryPanel.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/services/api/rollback-service.ts
🧠 Learnings (6)
📚 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:
packages/lib/package.json
📚 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 app/**/*.{ts,tsx} : Import and use `getUserAccessLevel()` and `canUserEditPage()` from `pagespace/lib/permissions` for centralized permission logic
Applied to files:
packages/lib/package.json
📚 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 ESM modules throughout the codebase
Applied to files:
packages/lib/package.json
📚 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: Use TypeScript strict mode and ESM modules throughout the codebase
Applied to files:
packages/lib/package.json
📚 Learning: 2025-12-14T14:54:15.319Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T14:54:15.319Z
Learning: Applies to apps/**/*.{ts,tsx} : Use centralized permission functions from `pagespace/lib/permissions` for access control, such as `getUserAccessLevel()` and `canUserEditPage()`
Applied to files:
packages/lib/package.json
📚 Learning: 2025-12-14T14:54:38.009Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T14:54:38.009Z
Learning: Applies to apps/**/*.ts : Always use Drizzle client and queries from `pagespace/db` for database access instead of direct queries
Applied to files:
apps/web/src/services/api/rollback-service.ts
🧬 Code graph analysis (2)
apps/web/src/components/version-history/VersionHistoryItem.tsx (6)
apps/web/src/components/activity/types.ts (1)
ActivityLog(10-31)apps/web/src/components/version-history/index.ts (2)
VersionHistoryItem(2-2)RollbackConfirmDialog(3-3)apps/web/src/components/activity/constants.ts (3)
operationConfig(17-30)defaultOperationConfig(39-43)resourceTypeIcons(32-37)apps/web/src/components/activity/utils.ts (1)
getInitials(4-16)apps/web/src/components/ui/dropdown-menu.tsx (4)
DropdownMenu(242-242)DropdownMenuTrigger(244-244)DropdownMenuContent(245-245)DropdownMenuItem(248-248)apps/web/src/components/version-history/RollbackConfirmDialog.tsx (1)
RollbackConfirmDialog(27-111)
apps/web/src/components/version-history/VersionHistoryPanel.tsx (4)
apps/web/src/components/activity/types.ts (1)
ActivityLog(10-31)apps/web/src/hooks/useToast.ts (1)
useToast(13-43)apps/web/src/lib/auth/auth-fetch.ts (1)
fetch(44-211)apps/web/src/components/version-history/VersionHistoryItem.tsx (1)
VersionHistoryItem(26-145)
⏰ 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 (3)
packages/lib/package.json (2)
130-139: LGTM! New module exports properly structured.The new
./permissionsand./monitoringexports are correctly configured and follow the established pattern of other package exports. These align with the PR's goal of exposing centralized rollback permissions and monitoring/activity logging functionality.Based on learnings, these exports enable consumers to use centralized permission functions from
@pagespace/lib/permissionsas recommended.
249-254: LGTM! TypeScript mappings correctly added.The
typesVersionsentries forpermissionsandmonitoringare properly configured to enable TypeScript type resolution for the new module exports.apps/web/src/components/version-history/VersionHistoryPanel.tsx (1)
88-95: The history endpoints are already properly authenticated. Both/api/pages/[pageId]/historyand/api/drives/[driveId]/historyuseauthenticateRequestWithOptions()to verify requests and include authorization checks (canUserViewPage()andisDriveOwnerOrAdmin()). Browsers automatically send authentication cookies with same-origin fetch requests, so the plainfetch()call is appropriate and secure.Likely an incorrect or invalid review comment.
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>
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
apps/web/src/services/api/rollback-service.ts (1)
863-866: Operation filter still lacks validation before type assertion.The
operationparameter from user input is directly cast without validation. This was flagged in a previous review and marked as addressed, but the current code still performs an unsafe cast.🔎 Proposed fix
+ // Valid operation values from the enum + const validOperations = ['create', 'update', 'delete', 'restore', 'reorder', 'trash', 'move', 'permission_grant', 'permission_update', 'permission_revoke', 'agent_config_update', 'rollback', 'member_add', 'member_remove', 'member_role_change', 'role_reorder', 'message_update', 'message_delete', 'ownership_transfer', 'conversation_undo']; + if (operation) { - // Type assertion needed since operation comes from user input - conditions.push(eq(activityLogs.operation, operation as typeof activityLogs.operation.enumValues[number])); + if (validOperations.includes(operation)) { + conditions.push(eq(activityLogs.operation, operation as typeof activityLogs.operation.enumValues[number])); + } else { + loggers.api.warn('[RollbackService] Invalid operation filter', { operation }); + } }Apply the same fix to
getDriveVersionHistoryat line 940-941.
🧹 Nitpick comments (5)
packages/db/src/schema/monitoring.ts (1)
449-454: Consider adding an index onrollbackFromActivityId.If queries will filter or join on
rollbackFromActivityId(e.g., "find all activities that were rolled back from activity X"), an index would improve performance.🔎 Add index for rollback queries
Add to the index configuration at the end of the
activityLogstable definition (around line 469):}, (table) => ({ timestampIdx: index('idx_activity_logs_timestamp').on(table.timestamp), userTimestampIdx: index('idx_activity_logs_user_timestamp').on(table.userId, table.timestamp), driveTimestampIdx: index('idx_activity_logs_drive_timestamp').on(table.driveId, table.timestamp), pageTimestampIdx: index('idx_activity_logs_page_timestamp').on(table.pageId, table.timestamp), archivedIdx: index('idx_activity_logs_archived').on(table.isArchived), + rollbackFromIdx: index('idx_activity_logs_rollback_from').on(table.rollbackFromActivityId), }));apps/web/src/services/api/rollback-service.ts (2)
879-883: Inefficient count query fetches all rows instead of using SQL COUNT.The count query selects all activity log IDs and counts the array length in JavaScript. For large datasets, this loads unnecessary data into memory.
🔎 Proposed fix using SQL count
+import { sql } from '@pagespace/db'; + const [activities, countResult] = await Promise.all([ db .select() .from(activityLogs) .where(and(...conditions)) .orderBy(desc(activityLogs.timestamp)) .limit(limit) .offset(offset), db - .select({ count: activityLogs.id }) + .select({ count: sql<number>`count(*)` }) .from(activityLogs) .where(and(...conditions)), ]); return { activities: activities.map((a) => ({ // ... mapping })), - total: countResult.length, + total: countResult[0]?.count ?? 0, };Apply the same fix to
getDriveVersionHistoryat lines 952-956 and 980.Also applies to: 907-907
844-848: UnuseduserIdparameter in version history functions.The
userIdparameter is declared but never used ingetPageVersionHistoryandgetDriveVersionHistory. If it's intended for future authorization checks, consider adding them or removing the parameter.Also applies to: 921-926
apps/web/src/components/ai/shared/chat/MessageActionButtons.tsx (1)
22-22: Consider simplifying redundant ternary expression.
buttonSizeis always'sm'regardless of thecompactvalue.🔎 Proposed fix
- const buttonSize = compact ? 'sm' : 'sm'; + const buttonSize = 'sm';apps/web/src/services/api/ai-undo-service.ts (1)
221-227: Consider using if-else-if for clarity.The context determination uses sequential
ifstatements, which works but is less clear than anif-else-ifchain. Each condition should be mutually exclusive.🔎 Suggested refactor
// Determine context based on resource type - let context: RollbackContext = 'ai_tool'; - if (activity.resourceType === 'drive') { + let context: RollbackContext; + if (activity.resourceType === 'drive') { context = 'drive'; } else if (activity.resourceType === 'page') { context = 'page'; + } else { + context = 'ai_tool'; }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
apps/web/src/app/api/ai/chat/messages/[messageId]/undo/route.tsapps/web/src/components/ai/shared/chat/ChatMessagesArea.tsxapps/web/src/components/ai/shared/chat/MessageActionButtons.tsxapps/web/src/components/ai/shared/chat/MessageRenderer.tsxapps/web/src/components/ai/shared/chat/UndoAiChangesDialog.tsxapps/web/src/components/ai/shared/chat/index.tsapps/web/src/services/api/ai-undo-service.tsapps/web/src/services/api/index.tsapps/web/src/services/api/rollback-service.tspackages/db/drizzle/0026_nervous_carnage.sqlpackages/db/drizzle/meta/0026_snapshot.jsonpackages/db/drizzle/meta/_journal.jsonpackages/db/src/schema/monitoring.tspackages/lib/src/monitoring/activity-logger.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/web/src/services/api/index.ts
- packages/db/drizzle/meta/_journal.json
- packages/lib/src/monitoring/activity-logger.ts
🧰 Additional context used
📓 Path-based instructions (11)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Never use
anytypes in TypeScript code - always use proper TypeScript types
**/*.{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
Files:
apps/web/src/components/ai/shared/chat/MessageActionButtons.tsxapps/web/src/components/ai/shared/chat/index.tsapps/web/src/components/ai/shared/chat/ChatMessagesArea.tsxpackages/db/src/schema/monitoring.tsapps/web/src/components/ai/shared/chat/MessageRenderer.tsxapps/web/src/app/api/ai/chat/messages/[messageId]/undo/route.tsapps/web/src/services/api/ai-undo-service.tsapps/web/src/components/ai/shared/chat/UndoAiChangesDialog.tsxapps/web/src/services/api/rollback-service.ts
apps/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/**/*.{ts,tsx}: Use centralized permission functions from@pagespace/lib/permissionsfor access control, such asgetUserAccessLevel()andcanUserEditPage()
Always use Drizzle client from@pagespace/dbfor database access instead of direct database connections
Always use message parts structure withpartsarray containing objects withtypeandtextfields when constructing messages for AI
Files:
apps/web/src/components/ai/shared/chat/MessageActionButtons.tsxapps/web/src/components/ai/shared/chat/index.tsapps/web/src/components/ai/shared/chat/ChatMessagesArea.tsxapps/web/src/components/ai/shared/chat/MessageRenderer.tsxapps/web/src/app/api/ai/chat/messages/[messageId]/undo/route.tsapps/web/src/services/api/ai-undo-service.tsapps/web/src/components/ai/shared/chat/UndoAiChangesDialog.tsxapps/web/src/services/api/rollback-service.ts
apps/web/src/**/*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
apps/web/src/**/*.tsx: For document editing, register editing state usinguseEditingStore.getState().startEditing()andendEditing()to prevent unwanted UI refreshes
For AI streaming operations, register streaming state usinguseEditingStore.getState().startStreaming()andendStreaming()to prevent unwanted UI refreshes
When using SWR, checkuseEditingStorestate withisAnyActive()and setisPausedto prevent data refreshes during editing or streaming
Files:
apps/web/src/components/ai/shared/chat/MessageActionButtons.tsxapps/web/src/components/ai/shared/chat/ChatMessagesArea.tsxapps/web/src/components/ai/shared/chat/MessageRenderer.tsxapps/web/src/components/ai/shared/chat/UndoAiChangesDialog.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/ai/shared/chat/MessageActionButtons.tsxapps/web/src/components/ai/shared/chat/ChatMessagesArea.tsxapps/web/src/components/ai/shared/chat/MessageRenderer.tsxapps/web/src/components/ai/shared/chat/UndoAiChangesDialog.tsx
**/*.{ts,tsx,js,jsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Format code with Prettier
Files:
apps/web/src/components/ai/shared/chat/MessageActionButtons.tsxapps/web/src/components/ai/shared/chat/index.tsapps/web/src/components/ai/shared/chat/ChatMessagesArea.tsxpackages/db/src/schema/monitoring.tsapps/web/src/components/ai/shared/chat/MessageRenderer.tsxapps/web/src/app/api/ai/chat/messages/[messageId]/undo/route.tsapps/web/src/services/api/ai-undo-service.tsapps/web/src/components/ai/shared/chat/UndoAiChangesDialog.tsxapps/web/src/services/api/rollback-service.ts
**/*.{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/MessageActionButtons.tsxapps/web/src/components/ai/shared/chat/ChatMessagesArea.tsxapps/web/src/components/ai/shared/chat/MessageRenderer.tsxapps/web/src/components/ai/shared/chat/UndoAiChangesDialog.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/components/ai/shared/chat/index.tspackages/db/src/schema/monitoring.tsapps/web/src/app/api/ai/chat/messages/[messageId]/undo/route.tsapps/web/src/services/api/ai-undo-service.tsapps/web/src/services/api/rollback-service.ts
packages/db/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Drizzle ORM for database queries with PostgreSQL
Files:
packages/db/src/schema/monitoring.ts
apps/web/src/app/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
apps/web/src/app/**/*.ts: In Next.js 15 route handlers with dynamic routes, you MUST awaitcontext.paramsbefore destructuring because params are Promise objects
Get request body usingconst body = await request.json();
Get search params usingconst { searchParams } = new URL(request.url);
Return JSON responses usingreturn Response.json(data)orreturn NextResponse.json(data)
Files:
apps/web/src/app/api/ai/chat/messages/[messageId]/undo/route.ts
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);
Files:
apps/web/src/app/api/ai/chat/messages/[messageId]/undo/route.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-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/index.tsapps/web/src/components/ai/shared/chat/ChatMessagesArea.tsxapps/web/src/components/ai/shared/chat/MessageRenderer.tsx
📚 Learning: 2025-12-14T14:54:15.319Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T14:54:15.319Z
Learning: Applies to apps/**/*.{ts,tsx} : Always use message parts structure with `parts` array containing objects with `type` and `text` fields when constructing messages for AI
Applied to files:
apps/web/src/components/ai/shared/chat/ChatMessagesArea.tsxapps/web/src/components/ai/shared/chat/MessageRenderer.tsxapps/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 packages/db/src/schema.ts : Maintain the Drizzle ORM database schema in `packages/db/src/schema.ts` as the single entry point for schema definitions
Applied to files:
packages/db/src/schema/monitoring.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 packages/db/src/schema.ts : Update database schema in `packages/db/src/schema.ts` and generate migrations with `pnpm db:generate`
Applied to files:
packages/db/src/schema/monitoring.ts
📚 Learning: 2025-12-14T14:54:15.319Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T14:54:15.319Z
Learning: Applies to apps/web/src/**/*.tsx : For document editing, register editing state using `useEditingStore.getState().startEditing()` and `endEditing()` to prevent unwanted UI refreshes
Applied to files:
apps/web/src/components/ai/shared/chat/MessageRenderer.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 **/*.{ts,tsx} : Message content should always use the message parts structure with `{ parts: [{ type: 'text', text: '...' }] }`
Applied to files:
apps/web/src/components/ai/shared/chat/MessageRenderer.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 message parts structure with `{ parts: [{ type: 'text', text: '...' }] }` format for message content
Applied to files:
apps/web/src/components/ai/shared/chat/MessageRenderer.tsx
📚 Learning: 2025-12-16T19:06:20.385Z
Learnt from: 2witstudios
Repo: 2witstudios/PageSpace PR: 91
File: apps/web/src/components/ai/ui/Image.tsx:2-2
Timestamp: 2025-12-16T19:06:20.385Z
Learning: In apps/web/src/components/ai/ui/Image.tsx (TypeScript/React), the intentional use of `Experimental_GeneratedImage` from the Vercel AI SDK is accepted. This type is the correct and intended way to handle AI-generated images with base64/mediaType properties, and will be updated when the AI SDK stabilizes this API.
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-14T14:54:38.009Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T14:54:38.009Z
Learning: Applies to apps/**/*.ts : Always use Drizzle client and queries from `pagespace/db` for database access instead of direct queries
Applied to files:
apps/web/src/services/api/rollback-service.ts
🧬 Code graph analysis (5)
apps/web/src/components/ai/shared/chat/MessageRenderer.tsx (1)
apps/web/src/components/ai/shared/chat/message-types.ts (1)
isProcessedToolPart(87-89)
apps/web/src/app/api/ai/chat/messages/[messageId]/undo/route.ts (3)
apps/web/src/lib/repositories/chat-message-repository.ts (1)
chatMessageRepository(47-112)apps/web/src/services/api/ai-undo-service.ts (3)
previewAiUndo(82-173)UndoMode(46-46)executeAiUndo(178-297)apps/web/src/services/api/index.ts (3)
previewAiUndo(60-60)UndoMode(65-65)executeAiUndo(61-61)
apps/web/src/services/api/ai-undo-service.ts (3)
packages/lib/src/permissions/rollback-permissions.ts (1)
RollbackContext(15-19)apps/web/src/services/api/rollback-service.ts (2)
previewRollback(134-254)executeRollback(259-359)packages/lib/src/monitoring/activity-logger.ts (1)
logConversationUndo(686-725)
apps/web/src/components/ai/shared/chat/UndoAiChangesDialog.tsx (5)
apps/web/src/components/ai/shared/chat/index.ts (1)
UndoAiChangesDialog(21-21)apps/web/src/services/api/ai-undo-service.ts (2)
AiUndoPreview(25-41)UndoMode(46-46)apps/web/src/services/api/index.ts (2)
AiUndoPreview(64-64)UndoMode(65-65)apps/web/src/lib/auth/auth-fetch.ts (1)
fetchWithAuth(704-705)apps/web/src/components/ui/badge.tsx (1)
Badge(46-46)
apps/web/src/services/api/rollback-service.ts (4)
packages/lib/src/monitoring/activity-logger.ts (3)
ActivityResourceType(91-105)getActorInfo(24-44)logRollbackActivity(632-679)packages/db/src/schema/monitoring.ts (1)
activityLogs(420-470)packages/lib/src/permissions/rollback-permissions.ts (2)
RollbackContext(15-19)isRollbackableOperation(162-182)packages/db/src/schema/core.ts (3)
pages(24-66)drives(7-22)chatMessages(68-89)
⏰ 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 (24)
packages/db/drizzle/0026_nervous_carnage.sql (1)
1-3: LGTM! Migration matches schema definitions.The enum additions are correctly structured and align with the TypeScript schema definitions in
monitoring.ts.packages/db/src/schema/monitoring.ts (3)
390-395: LGTM! Enum additions are well-structured.The new operation types support the version history and AI undo features, and the trailing comma on
ownership_transferimproves consistency.
411-413: LGTM! Resource enum additions are correct.The new
conversationresource type enables AI conversation tracking and rollback functionality.
490-500: No action required. TheretentionPoliciestable is properly exported from the monitoring schema module and correctly integrated into the main schema entry point atpackages/db/src/schema.ts.apps/web/src/services/api/rollback-service.ts (3)
396-406: Spreading user-controlled data directly into database update.The
updateDataobject is built frompreviousValueswhich originates from stored activity logs. While the source is semi-trusted, spreading it directly into the.set()call could allow unexpected fields to be written if the activity log was tampered with or contains fields not in the pages schema.Consider whitelisting allowed fields for page updates similar to how
rollbackAgentConfigChangehandles it:+ const allowedPageFields = ['title', 'content', 'parentId', 'position', 'isTrashed']; + const safeUpdateData: Record<string, unknown> = {}; + for (const [key, value] of Object.entries(updateData)) { + if (allowedPageFields.includes(key)) { + safeUpdateData[key] = value; + } + } + await db .update(pages) .set({ - ...updateData, + ...safeUpdateData, updatedAt: new Date(), }) .where(eq(pages.id, activity.pageId));
86-129: Well-structured activity fetching with proper error handling.The
getActivityByIdfunction correctly maps database results to theActivityLogForRollbackinterface, handles missing records gracefully, and logs errors appropriately.
452-547: Permission rollback handlers are now fully implemented.The
rollbackPermissionChangefunction properly handles all three permission operations (grant, revoke, update) with appropriate database operations and logging. This addresses the concern from the previous review.apps/web/src/components/ai/shared/chat/index.ts (1)
21-21: LGTM!The new export follows the established pattern and correctly exposes
UndoAiChangesDialogfrom the shared chat components.apps/web/src/components/ai/shared/chat/MessageActionButtons.tsx (1)
39-50: LGTM!The undo button implementation follows the established pattern used by other action buttons, with proper conditional rendering, consistent styling, and accessible title attribute.
apps/web/src/components/ai/shared/chat/MessageRenderer.tsx (2)
244-246: LGTM!The
hasToolCallscomputation correctly identifies assistant messages containing tool-call parts, ensuring the undo action is only available when relevant.
354-354: LGTM!The conditional logic
hasToolCalls && onUndoFromHereproperly gates the undo callback, and the inline arrow function correctly passes the message ID to the parent handler.apps/web/src/components/ai/shared/chat/ChatMessagesArea.tsx (2)
66-80: LGTM!The undo dialog state management is well-implemented with proper
useCallbackmemoization for handler stability and clean separation of concerns between opening, closing, and success callbacks.
163-168: LGTM!The
UndoAiChangesDialogis correctly placed outside theScrollAreaand properly receives the controlled open state, message ID, and success callback.apps/web/src/components/ai/shared/chat/UndoAiChangesDialog.tsx (2)
93-237: Well-structured dialog with comprehensive UX.The dialog provides excellent user experience with:
- Clear loading and error states
- Summary statistics for messages and changes
- Mode selection with appropriate disabling when no changes exist
- Warning display with truncation for long lists
- Activity badges showing what will be undone
- Proper button states during execution
18-18: Fix import statement: usepostinstead ofpostWithAuth.The function
postWithAuthdoes not exist in@/lib/auth/auth-fetch. The exported function is namedpost. Update the import at line 18 to usepost.Note: CSRF token protection is properly implemented in the underlying
fetch()method. All POST requests automatically include theX-CSRF-Tokenheader when required (lines 90-99 of auth-fetch.ts).Likely an incorrect or invalid review comment.
apps/web/src/app/api/ai/chat/messages/[messageId]/undo/route.ts (4)
9-10: CSRF protection is properly configured.The
AUTH_OPTIONS_WRITEcorrectly includesrequireCSRF: true, addressing the concern from PR comments about missing CSRF tokens on rollback POST requests. The client-sidepostWithAuthcombined with this server-side check ensures proper CSRF protection.
16-19: Properly handles Next.js 15 async params.Both route handlers correctly
await context.paramsbefore destructuring, following the Next.js 15 coding guidelines where params are Promise objects in dynamic routes.Also applies to: 82-85
96-102: Good input validation for mode parameter.The mode validation correctly rejects invalid values with a 400 status and clear error message before proceeding with the operation.
138-148: Appropriate use of HTTP 207 Multi-Status for partial success.Using 207 when some operations succeed but others fail is semantically correct and allows clients to handle partial success scenarios appropriately.
apps/web/src/services/api/ai-undo-service.ts (5)
1-20: LGTM! Clean imports and documentation.The file header clearly documents the two undo modes, and imports follow the project's ESM conventions using centralized packages.
25-56: LGTM! Well-defined type interfaces.The type definitions are clear and comprehensive, with no
anytypes.
61-77: LGTM! Clean helper functions.Both helper functions use Drizzle correctly and have focused responsibilities.
128-135: Fix context determination logic.The context assignment has a critical flaw:
- If
resourceTypeis'drive', line 131 setscontext = 'drive'- But then line 133's
else if (activity.isAiGenerated)will always be true (since line 118 filters forisAiGenerated: true), overriding the context to'ai_tool'- This means drive activities will incorrectly use
'ai_tool'context instead of'drive'contextAdditionally, the condition at line 133 is redundant since all activities in this loop are already AI-generated.
🔎 Proposed fix
// Determine context based on resource type - let context: RollbackContext = 'page'; + let context: RollbackContext = 'ai_tool'; if (activity.resourceType === 'drive') { context = 'drive'; - } else if (activity.isAiGenerated) { - context = 'ai_tool'; + } else if (activity.resourceType === 'page') { + context = 'page'; }Likely an incorrect or invalid review comment.
178-297: Permission checks are already properly implemented in the API route handler atapps/web/src/app/api/ai/chat/messages/[messageId]/undo/route.ts. Both GET and POST handlers validatecanUserEditPage(userId, message.pageId)before proceeding, using the centralized permission function from@pagespace/lib/serveras required by coding guidelines. No additional permission checks needed inexecuteAiUndo.Likely an incorrect or invalid review comment.
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>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
apps/web/src/services/api/rollback-service.ts (3)
879-883: Optimize count query to use SQL COUNT() instead of fetching all records.The count query selects all activity log IDs and then counts the array length (lines 880-882). This is inefficient for large datasets. Use Drizzle's SQL
count()aggregate instead.🔎 Proposed fix
+ import { sql } from '@pagespace/db'; + const [activities, countResult] = await Promise.all([ db .select() .from(activityLogs) .where(and(...conditions)) .orderBy(desc(activityLogs.timestamp)) .limit(limit) .offset(offset), db - .select({ count: activityLogs.id }) + .select({ count: sql<number>`count(*)` }) .from(activityLogs) .where(and(...conditions)), ]); return { activities: activities.map((a) => ({ // ... mapping })), - total: countResult.length, + total: countResult[0]?.count ?? 0, };Apply the same fix to lines 953-955 in
getDriveVersionHistory.
141-151: Unsafe type assertion for null activity.Casting
nulltoActivityLogForRollback(line 143) violates type safety and could cause runtime issues if consumers access properties on the returnedactivityfield. Update theRollbackPreviewinterface to allowactivity: ActivityLogForRollback | nulland returnactivity: nulldirectly without the unsafe cast.
863-866: Validate user input before type assertion to prevent query failures.The
operationparameter from user input is directly cast to an enum type without validation (line 865). Add validation before the type assertion to prevent runtime errors and unexpected query behavior.🔎 Proposed fix
+ // Valid operation values from the enum + const validOperations = ['create', 'update', 'delete', 'restore', 'reorder', 'trash', 'move', 'permission_grant', 'permission_update', 'permission_revoke', 'agent_config_update', 'rollback', 'member_add', 'member_remove', 'member_role_change', 'role_reorder', 'message_update', 'message_delete', 'signup', 'login', 'logout', 'ownership_transfer']; + if (operation) { + if (!validOperations.includes(operation)) { + loggers.api.warn('[RollbackService] Invalid operation filter', { operation }); + // Skip invalid operation filter + } else { conditions.push(eq(activityLogs.operation, operation as typeof activityLogs.operation.enumValues[number])); + } }Apply the same fix to line 941 in
getDriveVersionHistory.
🧹 Nitpick comments (1)
CLAUDE.md (1)
158-166: Use proper markdown headings instead of bold emphasis.The new critical rule sections use bold text instead of proper markdown headings. For better document structure and to satisfy markdown linting rules, convert these to level 4 headings.
🔎 Proposed fix
-**CRITICAL: Package Manager** +#### CRITICAL: Package Manager - **ALWAYS use `pnpm`** - This is a pnpm workspace project - **NEVER use `npm`** for install, run, or any other commands - All scripts in package.json are designed for pnpm -**CRITICAL: Database Migrations** +#### CRITICAL: Database Migrations - **NEVER manually create or edit SQL migration files** in `packages/db/drizzle/` - **ALWAYS use Drizzle generate commands**: `pnpm db:generate` - Migration files are auto-generated from schema changes in `packages/db/src/schema/`
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (2)
CLAUDE.mdapps/web/src/services/api/rollback-service.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Never use
anytypes in TypeScript code - always use proper TypeScript types
**/*.{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
Files:
apps/web/src/services/api/rollback-service.ts
apps/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/**/*.{ts,tsx}: Use centralized permission functions from@pagespace/lib/permissionsfor access control, such asgetUserAccessLevel()andcanUserEditPage()
Always use Drizzle client from@pagespace/dbfor database access instead of direct database connections
Always use message parts structure withpartsarray containing objects withtypeandtextfields when constructing messages for AI
Files:
apps/web/src/services/api/rollback-service.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/services/api/rollback-service.ts
**/*.{ts,tsx,js,jsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Format code with Prettier
Files:
apps/web/src/services/api/rollback-service.ts
🧠 Learnings (8)
📚 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 packages/db/drizzle/*.ts : Database migrations should be generated into `packages/db/drizzle/` directory using `pnpm db:generate` command
Applied to files:
CLAUDE.md
📚 Learning: 2025-12-14T14:54:38.009Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T14:54:38.009Z
Learning: Applies to packages/db/{src/schema.ts,drizzle/**/*.ts} : Database schema must be defined in `packages/db/src/schema.ts` and migrations must be emitted to `packages/db/drizzle/`
Applied to files:
CLAUDE.md
📚 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:
CLAUDE.md
📚 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: Before opening a pull request, run `pnpm build`, `pnpm typecheck`, `pnpm lint`, and relevant database migration tasks
Applied to files:
CLAUDE.md
📚 Learning: 2025-12-14T14:54:15.319Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T14:54:15.319Z
Learning: Applies to apps/**/*.{ts,tsx} : Always use Drizzle client from `pagespace/db` for database access instead of direct database connections
Applied to files:
CLAUDE.md
📚 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} : Always use Drizzle client from `pagespace/db` package for database access
Applied to files:
CLAUDE.md
📚 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: Run `pnpm build`, `pnpm typecheck`, and relevant `db:*` tasks before opening a pull request
Applied to files:
CLAUDE.md
📚 Learning: 2025-12-14T14:54:38.009Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T14:54:38.009Z
Learning: Applies to apps/**/*.ts : Always use Drizzle client and queries from `pagespace/db` for database access instead of direct queries
Applied to files:
apps/web/src/services/api/rollback-service.ts
🧬 Code graph analysis (1)
apps/web/src/services/api/rollback-service.ts (4)
packages/lib/src/monitoring/activity-logger.ts (2)
ActivityResourceType(91-105)logRollbackActivity(632-679)packages/db/src/schema/monitoring.ts (1)
activityLogs(420-470)packages/lib/src/logging/logger-config.ts (1)
loggers(8-18)packages/lib/src/permissions/rollback-permissions.ts (3)
RollbackContext(15-19)isRollbackableOperation(162-182)canUserRollback(52-141)
🪛 markdownlint-cli2 (0.18.1)
CLAUDE.md
158-158: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
163-163: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
⏰ 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
- 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>
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
apps/web/src/app/api/pages/[pageId]/history/route.ts (1)
67-79: Consider clarifying the retention logic comment.The retention enforcement logic is correct and properly handles unlimited retention (business tier with -1), but the comment could be more explicit about the behavior.
💡 Optional: Make the comment more explicit
- // Apply retention limit to startDate if not unlimited (-1) + // Apply retention limit to startDate (skip if retentionDays <= 0, e.g., -1 for unlimited business tier) let effectiveStartDate = params.startDate; if (retentionDays > 0) {apps/web/src/app/api/ai/chat/messages/[messageId]/undo/route.ts (1)
93-102: Consider using Zod for request body validation.The manual validation works, but for consistency with the rest of the codebase and better type safety, consider using Zod:
🔎 Suggested Zod validation
+const bodySchema = z.object({ + mode: z.enum(['messages_only', 'messages_and_changes']), +}); + export async function POST( request: Request, context: { params: Promise<{ messageId: string }> } ) { try { // Authenticate with CSRF const auth = await authenticateRequestWithOptions(request, AUTH_OPTIONS_WRITE); if (isAuthError(auth)) return auth.error; const userId = auth.userId; const { messageId } = await context.params; - const body = await request.json(); - const mode: UndoMode = body.mode; - - // Validate mode - if (!mode || !['messages_only', 'messages_and_changes'].includes(mode)) { - return NextResponse.json( - { error: 'Invalid mode. Must be "messages_only" or "messages_and_changes"' }, - { status: 400 } - ); - } + const body = await request.json(); + const parseResult = bodySchema.safeParse(body); + if (!parseResult.success) { + return NextResponse.json( + { error: parseResult.error.issues.map(i => i.message).join('. ') }, + { status: 400 } + ); + } + const { mode } = parseResult.data;apps/web/src/services/api/ai-undo-service.ts (1)
212-259: Partial atomicity is acceptable for partial-success semantics, but document the trade-off.The rollback loop (lines 213-245) operates outside the transaction, while only message soft-deletion is wrapped. This design supports partial success (some rollbacks fail, messages still get deleted), but means:
- If rollbacks succeed but message deletion fails, rollbacks won't be reverted
- Rollback failures don't prevent message deletion
This is a reasonable trade-off for UX (users get partial undo rather than all-or-nothing failure), but consider adding a code comment to document this intentional design choice for future maintainers.
🔎 Suggested documentation
const rolledBackActivityIds: string[] = []; - // If mode includes changes, rollback activities in reverse chronological order + // If mode includes changes, rollback activities in reverse chronological order. + // Note: Rollbacks are performed outside the transaction intentionally to support + // partial success - if some rollbacks fail, we still proceed with message deletion. + // This means rollback failures don't prevent the undo, and successful rollbacks + // won't be reverted if message deletion subsequently fails. if (mode === 'messages_and_changes') {
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
apps/web/src/app/api/activities/[activityId]/route.tsapps/web/src/app/api/ai/chat/messages/[messageId]/undo/route.tsapps/web/src/app/api/drives/[driveId]/history/route.tsapps/web/src/app/api/pages/[pageId]/history/route.tsapps/web/src/components/ai/shared/chat/MessageActionButtons.tsxapps/web/src/components/ai/shared/chat/UndoAiChangesDialog.tsxapps/web/src/components/version-history/VersionHistoryItem.tsxapps/web/src/components/version-history/VersionHistoryPanel.tsxapps/web/src/services/api/ai-undo-service.tsapps/web/src/services/api/rollback-service.tspackages/db/drizzle/0027_fix_retention_updated_at.sqlpackages/db/src/schema/monitoring.tspackages/lib/src/monitoring/activity-logger.tspackages/lib/src/permissions/rollback-permissions.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- apps/web/src/components/version-history/VersionHistoryItem.tsx
- apps/web/src/components/version-history/VersionHistoryPanel.tsx
- apps/web/src/app/api/drives/[driveId]/history/route.ts
- apps/web/src/components/ai/shared/chat/UndoAiChangesDialog.tsx
- apps/web/src/components/ai/shared/chat/MessageActionButtons.tsx
- packages/lib/src/permissions/rollback-permissions.ts
- apps/web/src/services/api/rollback-service.ts
🧰 Additional context used
📓 Path-based instructions (8)
apps/web/src/app/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
apps/web/src/app/**/*.ts: In Next.js 15 route handlers with dynamic routes, you MUST awaitcontext.paramsbefore destructuring because params are Promise objects
Get request body usingconst body = await request.json();
Get search params usingconst { searchParams } = new URL(request.url);
Return JSON responses usingreturn Response.json(data)orreturn NextResponse.json(data)
Files:
apps/web/src/app/api/pages/[pageId]/history/route.tsapps/web/src/app/api/activities/[activityId]/route.tsapps/web/src/app/api/ai/chat/messages/[messageId]/undo/route.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Never use
anytypes in TypeScript code - always use proper TypeScript types
**/*.{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
Files:
apps/web/src/app/api/pages/[pageId]/history/route.tspackages/lib/src/monitoring/activity-logger.tsapps/web/src/services/api/ai-undo-service.tsapps/web/src/app/api/activities/[activityId]/route.tspackages/db/src/schema/monitoring.tsapps/web/src/app/api/ai/chat/messages/[messageId]/undo/route.ts
apps/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/**/*.{ts,tsx}: Use centralized permission functions from@pagespace/lib/permissionsfor access control, such asgetUserAccessLevel()andcanUserEditPage()
Always use Drizzle client from@pagespace/dbfor database access instead of direct database connections
Always use message parts structure withpartsarray containing objects withtypeandtextfields when constructing messages for AI
Files:
apps/web/src/app/api/pages/[pageId]/history/route.tsapps/web/src/services/api/ai-undo-service.tsapps/web/src/app/api/activities/[activityId]/route.tsapps/web/src/app/api/ai/chat/messages/[messageId]/undo/route.ts
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);
Files:
apps/web/src/app/api/pages/[pageId]/history/route.tsapps/web/src/app/api/activities/[activityId]/route.tsapps/web/src/app/api/ai/chat/messages/[messageId]/undo/route.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/pages/[pageId]/history/route.tspackages/lib/src/monitoring/activity-logger.tsapps/web/src/services/api/ai-undo-service.tsapps/web/src/app/api/activities/[activityId]/route.tspackages/db/src/schema/monitoring.tsapps/web/src/app/api/ai/chat/messages/[messageId]/undo/route.ts
**/*.{ts,tsx,js,jsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Format code with Prettier
Files:
apps/web/src/app/api/pages/[pageId]/history/route.tspackages/lib/src/monitoring/activity-logger.tsapps/web/src/services/api/ai-undo-service.tsapps/web/src/app/api/activities/[activityId]/route.tspackages/db/src/schema/monitoring.tsapps/web/src/app/api/ai/chat/messages/[messageId]/undo/route.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
packages/db/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Drizzle ORM for database queries with PostgreSQL
Files:
packages/db/src/schema/monitoring.ts
🧠 Learnings (2)
📚 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 apps/web/src/app/**/route.{ts,tsx} : Get search parameters using `const { searchParams } = new URL(request.url);`
Applied to files:
apps/web/src/app/api/pages/[pageId]/history/route.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 apps/web/src/app/**/route.{ts,tsx} : In Next.js 15 route handlers, `params` in dynamic routes are Promise objects and MUST be awaited before destructuring
Applied to files:
apps/web/src/app/api/activities/[activityId]/route.ts
🧬 Code graph analysis (4)
apps/web/src/app/api/pages/[pageId]/history/route.ts (3)
apps/web/src/lib/auth/index.ts (2)
authenticateRequestWithOptions(216-271)isAuthError(204-206)apps/web/src/services/api/rollback-service.ts (2)
getUserRetentionDays(1022-1052)getPageVersionHistory(870-941)packages/lib/src/permissions/rollback-permissions.ts (1)
isActivityEligibleForRollback(194-203)
apps/web/src/services/api/ai-undo-service.ts (2)
apps/web/src/services/api/rollback-service.ts (3)
RollbackContext(16-16)previewRollback(160-280)executeRollback(285-385)packages/lib/src/monitoring/activity-logger.ts (1)
logConversationUndo(692-731)
apps/web/src/app/api/activities/[activityId]/route.ts (3)
apps/web/src/lib/auth/index.ts (2)
authenticateRequestWithOptions(216-271)isAuthError(204-206)packages/lib/src/permissions/rollback-permissions.ts (1)
RollbackContext(15-19)apps/web/src/services/api/index.ts (2)
getActivityById(45-45)previewRollback(46-46)
apps/web/src/app/api/ai/chat/messages/[messageId]/undo/route.ts (3)
apps/web/src/lib/repositories/chat-message-repository.ts (1)
chatMessageRepository(47-112)apps/web/src/lib/logging/mask.ts (1)
maskIdentifier(1-12)apps/web/src/services/api/index.ts (3)
previewAiUndo(60-60)UndoMode(65-65)executeAiUndo(61-61)
⏰ 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 (21)
apps/web/src/app/api/pages/[pageId]/history/route.ts (4)
1-18: LGTM! Well-structured imports and validation schema.The imports are appropriate, authentication options correctly disable CSRF for the GET endpoint, and the Zod schema provides comprehensive query parameter validation with sensible constraints (limit capped at 100, offset minimum 0, etc.).
25-36: Excellent Next.js 15 compliance and authentication handling.The handler correctly awaits
context.paramsbefore destructuring (Next.js 15 requirement), extracts search parameters usingnew URL(request.url)as per guidelines, and properly handles authentication with centralized helpers.
38-65: Proper authorization and input validation.Uses the centralized
canUserViewPage()permission function as required by coding guidelines, returns appropriate HTTP status codes (403 for unauthorized, 400 for validation errors), and safely parses query parameters using Zod'ssafeParse().
81-108: Well-implemented version history fetching and response construction.The code properly:
- Passes the retention-adjusted parameters to the service layer
- Enriches each activity with a
canRollbackflag using the centralizedisActivityEligibleForRollback()helper from the permissions module- Returns a well-structured JSON response with
NextResponse.json()containing versions, pagination metadata, and retention information- Correctly calculates the
hasMorepagination flagpackages/db/src/schema/monitoring.ts (3)
390-396: LGTM! Operation enum values correctly extended.The new rollback and conversation undo operations are properly added and align with the version history feature requirements.
411-413: LGTM! Resource enum extended appropriately.The message and conversation resource types support the AI conversation undo capabilities.
470-470: LGTM! Index properly defined for rollback chain queries.The index on
rollbackFromActivityIdwill optimize queries tracing rollback history.packages/db/drizzle/0027_fix_retention_updated_at.sql (1)
1-5: LGTM! Migration correctly fixesupdatedAtdefault and adds rollback index.Both changes are necessary and properly implemented:
- The
DEFAULT now()ensuresupdatedAtis populated on INSERT- The
IF NOT EXISTSguard prevents conflicts with the index already defined in the schemaNote: Ensure the schema file (
packages/db/src/schema/monitoring.ts) is updated to include.defaultNow()on theupdatedAtfield to match this migration (already flagged in previous review).apps/web/src/app/api/activities/[activityId]/route.ts (4)
1-12: LGTM on imports and configuration.Proper use of Zod for query validation, centralized permission functions from
@pagespace/lib/permissions, and correct import ofRollbackContexttype. TheAUTH_OPTIONScorrectly omits CSRF for this read-only GET endpoint.
19-53: Correct Next.js 15 dynamic route handling and validation.The handler properly:
- Types
paramsasPromise<{ activityId: string }>and awaits before destructuring (line 28)- Extracts
searchParamsvianew URL(request.url)(line 30)- Validates query params with Zod and returns 400 on failure
55-78: Authorization checks are properly implemented.The granular authorization logic correctly:
- Verifies page access via
canUserViewPage()for page-scoped activities- Verifies drive membership via
isUserDriveMember()for drive-scoped activities- Falls back to ownership check for user-level activities
This addresses the previous review concern about missing authorization.
80-89: LGTM on response handling.The endpoint correctly computes rollback eligibility via
previewRollbackand returns a well-structured JSON response usingNextResponse.json().apps/web/src/app/api/ai/chat/messages/[messageId]/undo/route.ts (3)
1-11: Well-structured imports and authentication configuration.Good separation of read vs write auth options. The
AUTH_OPTIONS_WRITEcorrectly enablesrequireCSRF: truefor the POST endpoint, addressing the CSRF concern raised in PR comments.
16-76: GET handler correctly implements preview with authorization.The handler properly:
- Awaits
context.paramsbefore destructuring (Next.js 15 compliant)- Validates message existence before permission check
- Uses centralized
canUserEditPage()for authorization- Logs permission denials with masked identifiers for privacy
- Returns appropriate status codes (404, 403, 500)
138-156: Good use of HTTP 207 for partial success scenarios.The response handling appropriately distinguishes between full success, partial success (207), and complete failure (500). The human-readable messages based on mode provide good UX feedback.
apps/web/src/services/api/ai-undo-service.ts (2)
1-56: Well-defined interfaces and type exports.The
AiUndoPreviewandAiUndoResultinterfaces are clearly structured with appropriate fields. TheUndoModeunion type correctly constrains the two supported modes.
82-173: Preview logic is comprehensive and well-structured.The function correctly:
- Queries affected messages and AI-generated activities from the conversation point forward
- Iterates activities to check individual rollback eligibility
- Aggregates warnings for activities that cannot be rolled back
- Returns a complete preview with all necessary information for the UI
The N+1 pattern on
previewRollbackcalls is acceptable here given the typical small number of activities per undo operation.packages/lib/src/monitoring/activity-logger.ts (4)
84-105: LGTM on new operation and resource types.The new
ActivityOperationvalues ('rollback','conversation_undo','conversation_undo_with_changes') andActivityResourceType('conversation') are properly added to support the version history feature.
127-136: Top-level fields correctly added toActivityLogInput.The
contentFormatandrollbackFromActivityIdfields are now properly defined as top-level optional fields in the interface, addressing the previous review concern about storing them only in metadata.
142-173:logActivitycorrectly threads new fields to database insert.Lines 161 and 166 properly pass
contentFormatandrollbackFromActivityIdas top-level values to the database insert, ensuring they're stored in dedicated columns rather than the generic metadata JSONB field.
687-731: LGTM onlogConversationUndowrapper.This wrapper correctly:
- Determines operation based on undo mode
- Uses
'conversation'as resource type withconversationIdas the resource ID- Stores conversation-undo-specific metadata (messageId, counts, rolledBackActivityIds)
The design appropriately differs from
logRollbackActivitysince conversation undo logs the undo event itself rather than a specific resource rollback.
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>
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
apps/web/src/app/api/ai/chat/messages/[messageId]/undo/route.tsapps/web/src/services/api/ai-undo-service.tsapps/web/src/services/api/rollback-service.tspackages/db/drizzle/0028_version_history_review_fixes.sqlpackages/db/drizzle/meta/0028_snapshot.jsonpackages/db/drizzle/meta/_journal.jsonpackages/db/src/schema/monitoring.tspackages/lib/src/monitoring/activity-logger.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/db/drizzle/meta/_journal.json
- packages/lib/src/monitoring/activity-logger.ts
🧰 Additional context used
📓 Path-based instructions (8)
apps/web/src/app/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
apps/web/src/app/**/*.ts: In Next.js 15 route handlers with dynamic routes, you MUST awaitcontext.paramsbefore destructuring because params are Promise objects
Get request body usingconst body = await request.json();
Get search params usingconst { searchParams } = new URL(request.url);
Return JSON responses usingreturn Response.json(data)orreturn NextResponse.json(data)
Files:
apps/web/src/app/api/ai/chat/messages/[messageId]/undo/route.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Never use
anytypes in TypeScript code - always use proper TypeScript types
**/*.{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
Files:
apps/web/src/app/api/ai/chat/messages/[messageId]/undo/route.tsapps/web/src/services/api/ai-undo-service.tsapps/web/src/services/api/rollback-service.tspackages/db/src/schema/monitoring.ts
apps/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/**/*.{ts,tsx}: Use centralized permission functions from@pagespace/lib/permissionsfor access control, such asgetUserAccessLevel()andcanUserEditPage()
Always use Drizzle client from@pagespace/dbfor database access instead of direct database connections
Always use message parts structure withpartsarray containing objects withtypeandtextfields when constructing messages for AI
Files:
apps/web/src/app/api/ai/chat/messages/[messageId]/undo/route.tsapps/web/src/services/api/ai-undo-service.tsapps/web/src/services/api/rollback-service.ts
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);
Files:
apps/web/src/app/api/ai/chat/messages/[messageId]/undo/route.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/services/api/ai-undo-service.tsapps/web/src/services/api/rollback-service.tspackages/db/src/schema/monitoring.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/services/api/ai-undo-service.tsapps/web/src/services/api/rollback-service.tspackages/db/src/schema/monitoring.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
packages/db/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Drizzle ORM for database queries with PostgreSQL
Files:
packages/db/src/schema/monitoring.ts
🧠 Learnings (5)
📚 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 app/api/**/*.{ts,tsx} : Use `const body = await request.json();` to extract request bodies, `const { searchParams } = new URL(request.url);` for query parameters, and `return Response.json(data)` or `return NextResponse.json(data)` for responses
Applied to files:
apps/web/src/app/api/ai/chat/messages/[messageId]/undo/route.ts
📚 Learning: 2025-12-14T14:54:38.009Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T14:54:38.009Z
Learning: Applies to apps/**/*.ts : Always use Drizzle client and queries from `pagespace/db` for database access instead of direct queries
Applied to files:
apps/web/src/services/api/rollback-service.ts
📚 Learning: 2025-12-22T22:46:00.813Z
Learnt from: 2witstudios
Repo: 2witstudios/PageSpace PR: 118
File: apps/web/src/services/api/rollback-service.ts:994-1024
Timestamp: 2025-12-22T22:46:00.813Z
Learning: In PageSpace, the subscription tier hierarchy is: free < pro < founder < business, where Business is the highest tier with unlimited retention (-1 days), Founder has 90 days, Pro has 30 days, and Free has 7 days retention.
Applied to files:
apps/web/src/services/api/rollback-service.ts
📚 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 packages/db/src/schema.ts : Maintain the Drizzle ORM database schema in `packages/db/src/schema.ts` as the single entry point for schema definitions
Applied to files:
packages/db/src/schema/monitoring.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 packages/db/src/schema.ts : Update database schema in `packages/db/src/schema.ts` and generate migrations with `pnpm db:generate`
Applied to files:
packages/db/src/schema/monitoring.ts
🧬 Code graph analysis (1)
apps/web/src/services/api/ai-undo-service.ts (5)
packages/lib/src/logging/logger-config.ts (1)
loggers(8-18)packages/db/src/schema/monitoring.ts (1)
activityLogs(422-476)apps/web/src/services/api/rollback-service.ts (3)
RollbackContext(16-16)previewRollback(161-281)executeRollback(287-393)apps/processor/src/logger.ts (1)
error(57-63)packages/lib/src/monitoring/activity-logger.ts (2)
getActorInfo(24-44)logConversationUndo(704-743)
⏰ 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 (10)
packages/db/drizzle/0028_version_history_review_fixes.sql (1)
1-25: LGTM! Well-structured migration with proper safety measures.The migration correctly introduces:
- Safe enum creation with duplicate_object exception handling
- Type conversion using USING cast (preserves NULLs, fails on invalid values)
- Denormalized rollback source fields for audit trail preservation
- CHECK constraint enforcing valid retention days (>= -1)
All changes align with the schema updates and rollback infrastructure introduced in this PR.
apps/web/src/app/api/ai/chat/messages/[messageId]/undo/route.ts (2)
22-82: LGTM! GET handler follows Next.js 15 and coding guidelines correctly.The handler properly:
- Awaits
context.paramsbefore destructuring (Next.js 15 requirement)- Uses centralized permission checking via
canUserEditPage()- Returns responses with
NextResponse.json()- Includes structured logging with masked identifiers
88-173: LGTM! POST handler correctly implements CSRF protection and follows all guidelines.The handler properly:
- Awaits
context.paramsandrequest.json()(Next.js 15 patterns)- Requires CSRF protection via
AUTH_OPTIONS_WRITEwithrequireCSRF: true- Validates request body with Zod 4 (correct
z.enum()usage)- Uses centralized permission function
canUserEditPage()- Returns appropriate HTTP status codes (207 for partial success, 500 for failure)
Note: The PR objectives mention frontend POST requests missing CSRF tokens. This is a frontend issue (likely in VersionHistoryPanel.tsx), not an issue with this route handler, which correctly enforces CSRF protection.
packages/db/src/schema/monitoring.ts (2)
390-398: LGTM! Enum additions align with migrations and rollback infrastructure.The new enum values correctly support:
- Rollback operations (
'rollback','conversation_undo','conversation_undo_with_changes')- Content format validation (
contentFormatEnumwith text/html/json/tiptap)- AI conversation resources (
'conversation')All changes match the corresponding SQL migrations (0025, 0026, 0028).
451-476: LGTM! Activity logs schema correctly implements rollback support.The changes properly address previous review feedback:
contentFormatnow uses thecontentFormatEnumfor type safety (previously flagged as needing validation)- Rollback tracking fields are appropriately nullable for audit trail preservation
- Index on
rollbackFromActivityIdsupports efficient rollback queriesAll fields match migration 0028 and support the rollback infrastructure introduced in this PR.
apps/web/src/services/api/rollback-service.ts (5)
28-45: LGTM! Input validation prevents SQL injection and query errors.The
VALID_OPERATIONSconstant andisValidOperation()function correctly address the previous review concern about validating user input before type assertions. This validation is properly applied in both:
getPageVersionHistory()(line 904)getDriveVersionHistory()(line 980)This prevents invalid operation values from causing runtime errors or potential security issues.
287-393: LGTM! Transaction support correctly implemented for atomic rollbacks.The
executeRollback()function properly:
- Accepts optional
txparameter for transaction context (line 291)- Passes transaction to all resource-specific rollback handlers
- Allows atomic rollbacks when called within a transaction (as done in
ai-undo-service.ts)This addresses the previous review feedback about wrapping operations in transactions.
488-822: LGTM! Resource-specific rollback handlers are now fully implemented.The handlers for permissions, members, and roles now correctly perform database operations instead of just logging warnings (as flagged in previous reviews):
rollbackPermissionChange()(lines 488-584): Handles grant/revoke/update with actual DB operationsrollbackMemberChange()(lines 637-730): Handles add/remove/role changes with DB operationsrollbackRoleChange()(lines 735-822): Handles create/delete/update with DB operationsAll handlers properly accept the
databaseparameter for transaction support and include structured logging.
911-923: LGTM! Count queries optimized to use SQL COUNT() aggregate.Both history functions now correctly use Drizzle's
count()aggregate function instead of fetching all rows and counting in JavaScript (lines 919-920, 995-996). This addresses the previous performance concern and prevents memory issues with large datasets.Also applies to: 987-999
1037-1067: LGTM! Retention tier mapping is correct per product specifications.The retention days mapping correctly implements the tier hierarchy:
- Free: 7 days
- Pro: 30 days
- Founder: 90 days
- Business: unlimited (-1)
Based on learnings, this ordering is correct with Business as the highest tier.
- 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>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/db/src/schema/monitoring.ts (1)
457-457: Foreign key constraint for rollback tracking still not added.As noted in previous reviews,
rollbackFromActivityIdlacks a foreign key constraint toactivityLogs.id. This design choice allows rollback provenance to survive source activity deletion, which may be intentional for audit compliance. If intentional, consider adding a comment explaining this decision.
🧹 Nitpick comments (2)
packages/db/src/schema/monitoring.ts (1)
453-453: Consider storage implications for contentSnapshot.The
contentSnapshotfield is unbounded text, which could lead to significant database growth when storing large page content. Consider whether content snapshots need size limits, compression, or external storage for large payloads.apps/web/src/services/api/ai-undo-service.ts (1)
189-200: Optional: Consider reusing message from preview.The message is fetched twice: once inside
previewAiUndo(line 88) and again at line 200. While this doesn't affect correctness, you could optimize by including the message object in theAiUndoPreviewreturn type and reusing it here to save a database query.This is a minor optimization and the current approach maintains clean separation of concerns.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/src/services/api/ai-undo-service.tspackages/db/src/schema/monitoring.ts
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Never use
anytypes in TypeScript code - always use proper TypeScript types
**/*.{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
Files:
apps/web/src/services/api/ai-undo-service.tspackages/db/src/schema/monitoring.ts
apps/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/**/*.{ts,tsx}: Use centralized permission functions from@pagespace/lib/permissionsfor access control, such asgetUserAccessLevel()andcanUserEditPage()
Always use Drizzle client from@pagespace/dbfor database access instead of direct database connections
Always use message parts structure withpartsarray containing objects withtypeandtextfields when constructing messages for AI
Files:
apps/web/src/services/api/ai-undo-service.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/services/api/ai-undo-service.tspackages/db/src/schema/monitoring.ts
**/*.{ts,tsx,js,jsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Format code with Prettier
Files:
apps/web/src/services/api/ai-undo-service.tspackages/db/src/schema/monitoring.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
packages/db/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Drizzle ORM for database queries with PostgreSQL
Files:
packages/db/src/schema/monitoring.ts
🧠 Learnings (11)
📓 Common learnings
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-22T20:04:40.892Z
Learning: Applies to packages/db/src/schema.ts : Update database schema in `packages/db/src/schema.ts` and generate migrations with `pnpm db:generate`
📚 Learning: 2025-12-14T14:54:15.319Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T14:54:15.319Z
Learning: Applies to apps/**/*.{ts,tsx} : Always use message parts structure with `parts` array containing objects with `type` and `text` fields when constructing messages for AI
Applied to files:
apps/web/src/services/api/ai-undo-service.ts
📚 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 packages/db/src/schema.ts : Maintain the Drizzle ORM database schema in `packages/db/src/schema.ts` as the single entry point for schema definitions
Applied to files:
packages/db/src/schema/monitoring.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 packages/db/src/schema.ts : Database schema entry point is at `packages/db/src/schema.ts`; migrations emit to `packages/db/drizzle/`
Applied to files:
packages/db/src/schema/monitoring.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 packages/db/src/schema.ts : Update database schema in `packages/db/src/schema.ts` and generate migrations with `pnpm db:generate`
Applied to files:
packages/db/src/schema/monitoring.ts
📚 Learning: 2025-12-14T14:54:38.009Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T14:54:38.009Z
Learning: Applies to packages/db/{src/schema.ts,drizzle/**/*.ts} : Database schema must be defined in `packages/db/src/schema.ts` and migrations must be emitted to `packages/db/drizzle/`
Applied to files:
packages/db/src/schema/monitoring.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 packages/db/**/*.{ts,tsx} : Use Drizzle ORM for database queries with PostgreSQL
Applied to files:
packages/db/src/schema/monitoring.ts
📚 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:
packages/db/src/schema/monitoring.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} : Always use Drizzle client from `pagespace/db` package for database access
Applied to files:
packages/db/src/schema/monitoring.ts
📚 Learning: 2025-12-14T14:54:38.009Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T14:54:38.009Z
Learning: Applies to apps/**/*.ts : Always use Drizzle client and queries from `pagespace/db` for database access instead of direct queries
Applied to files:
packages/db/src/schema/monitoring.ts
📚 Learning: 2025-12-14T14:54:15.319Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T14:54:15.319Z
Learning: Applies to apps/**/*.{ts,tsx} : Always use Drizzle client from `pagespace/db` for database access instead of direct database connections
Applied to files:
packages/db/src/schema/monitoring.ts
🧬 Code graph analysis (2)
apps/web/src/services/api/ai-undo-service.ts (4)
packages/db/src/schema/core.ts (1)
chatMessages(68-89)packages/db/src/schema/monitoring.ts (1)
activityLogs(423-477)apps/web/src/services/api/rollback-service.ts (3)
RollbackContext(16-16)previewRollback(161-281)executeRollback(287-393)packages/lib/src/monitoring/activity-logger.ts (2)
getActorInfo(24-44)logConversationUndo(704-743)
packages/db/src/schema/monitoring.ts (2)
packages/lib/src/sheets/sheet.ts (1)
check(1428-1440)packages/db/src/index.ts (1)
sql(8-8)
⏰ 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 (11)
packages/db/src/schema/monitoring.ts (6)
15-18: LGTM! Import additions support new schema features.The additional imports (
pgEnum,check,relations,sql) are appropriately used throughout the file for the new enums, constraints, and relations.
391-396: LGTM! Operation enum extended for version history and AI undo features.The new operations ('rollback', 'conversation_undo', 'conversation_undo_with_changes') align with the PR objectives for version history and AI conversation undo functionality.
399-399: LGTM! Content format enum provides type safety.This enum addresses previous review feedback by ensuring only valid content formats ('text', 'html', 'json', 'tiptap') can be stored, providing database-level validation for the
contentFormatfield.
414-416: LGTM! Resource enum extended for messages and conversations.The additions ('message', 'conversation') support the new AI conversation undo functionality and align with the operation enum extensions.
476-476: LGTM! Index improves rollback chain query performance.The index on
rollbackFromActivityIdwill efficiently support queries that traverse or filter rollback chains.
508-508: LGTM! CHECK constraint addresses previous feedback.The
valid_retention_daysconstraint ensuresretentionDays >= -1, which correctly enforces the unlimited (-1) and positive day values as specified in the requirements.apps/web/src/services/api/ai-undo-service.ts (5)
10-20: LGTM! Clean imports and proper package usage.The imports correctly use Drizzle from
@pagespace/db, monitoring utilities from@pagespace/lib/monitoring, and the rollback service for integration. All ESM imports follow the codebase conventions.
22-56: LGTM! Well-defined TypeScript types.The type definitions are clear and comprehensive, with proper JSDoc documentation. All types follow TypeScript best practices with no
anytypes.
58-77: LGTM! Clean helper functions.Both helper functions use Drizzle's query builder correctly and handle missing data gracefully by returning null values.
79-173: LGTM! Comprehensive preview implementation.The preview function correctly:
- Queries all affected messages and AI-generated activities
- Evaluates rollback eligibility for each activity with appropriate context
- Collects warnings for non-rollbackable items
- Handles errors gracefully with logging
The context determination logic properly handles drive, page, and AI-generated activities.
175-299: Excellent implementation! All past issues resolved.The execute function now implements proper all-or-nothing transaction semantics:
- All operations (rollbacks + message deletions) wrapped in a single transaction
- Any failure throws an error to abort the entire transaction
- Context determination is consistent with preview logic
The transaction flow correctly handles all scenarios:
- Non-rollbackable activities abort the transaction
- Failed rollbacks abort the transaction
- Failed message deletions abort the transaction
- Success only when all operations complete
- 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 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>
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
apps/web/src/app/api/ai/chat/messages/[messageId]/undo/__tests__/route.test.ts (1)
342-357: Add clarifying comment for partial failure mock.This test mocks
success: falsewithmessagesDeleted: 3, which may be inconsistent with the service's transactional behavior (where failed transactions would roll back all changes). While valid as a unit test of route handler defensiveness, a brief comment would help future maintainers understand the intent.
🧹 Nitpick comments (7)
apps/web/src/app/api/pages/[pageId]/history/__tests__/route.test.ts (1)
139-149: Consider adding assertion that service is not called on authorization failure.Similar to the authentication test, it would be valuable to verify
getPageVersionHistoryis not called when the user lacks view permission.🔎 Suggested enhancement
it('returns 403 when user cannot view page', async () => { (canUserViewPage as Mock).mockResolvedValue(false); const response = await GET(createRequest(), { params: mockParams }); const body = await response.json(); expect(response.status).toBe(403); expect(body.error).toContain('do not have access'); + expect(getPageVersionHistory).not.toHaveBeenCalled(); });apps/web/src/services/api/__tests__/ai-undo-service.test.ts (2)
93-114: Consider adding return type annotations to factory functions.The factory functions work correctly but lack explicit return types. Adding type annotations would improve type safety and documentation.
🔎 Suggested improvement
-const createMockMessage = (overrides = {}) => ({ +const createMockMessage = (overrides: Partial<ReturnType<typeof createMockMessage>> = {}): { + id: string; + conversationId: string; + pageId: string; + createdAt: Date; + role: string; + content: string; + isActive: boolean; +} => ({ id: mockMessageId, conversationId: mockConversationId, pageId: mockPageId, createdAt: new Date('2024-01-15T10:00:00Z'), role: 'user', content: 'Test message', isActive: true, ...overrides, });Alternatively, if there are shared types in the codebase for these entities, import and use those types directly.
459-463: Redundant try/catch block.The try/catch that only rethrows the error adds no value. The mock can simply await the callback without the try/catch wrapper.
🔎 Proposed simplification
(db.transaction as Mock).mockImplementation(async (callback) => { const tx = { update: vi.fn().mockReturnValue({ set: vi.fn().mockReturnValue({ where: vi.fn().mockResolvedValue(undefined), }), }), }; - try { - await callback(tx); - } catch (e) { - throw e; - } + await callback(tx); });The same pattern appears at lines 504-508.
packages/lib/src/permissions/__tests__/rollback-permissions.test.ts (1)
114-124: Avoidas anycasts per coding guidelines.Lines 115 and 123 use
as anyfor type casting. Consider using a more specific type assertion or adjusting the test approach to satisfy TypeScript's type checker withoutany.🔎 Suggested fix
- it.each(rollbackableTypes)('returns true for %s resource type', (type) => { - expect(isRollbackableResourceType(type as any)).toBe(true); + it.each(rollbackableTypes)('returns true for %s resource type', (type) => { + expect(isRollbackableResourceType(type as ActivityResourceType)).toBe(true); }); }); describe('non-rollbackable resource types', () => { const nonRollbackableTypes = ['user', 'file', 'token', 'device', 'conversation']; it.each(nonRollbackableTypes)('returns false for %s resource type', (type) => { - expect(isRollbackableResourceType(type as any)).toBe(false); + expect(isRollbackableResourceType(type as ActivityResourceType)).toBe(false); });You'll need to import
ActivityResourceTypefrom the appropriate module.apps/web/src/components/activity/ActivityItem.tsx (1)
42-67: Consider extracting shared rollback preview/confirm logic.The
handleRestoreClickandhandleConfirmRollbackpattern here is similar toSidebarActivityTab.tsx. Consider extracting this into a shared hook (e.g.,useRollbackPreview) to reduce duplication.apps/web/src/services/api/__tests__/rollback-service.test.ts (1)
798-801: FragilePromise.allspy approach.Spying on
Promise.allis brittle because:
- It affects all Promise.all calls globally during the test
- Implementation changes that don't use Promise.all would break tests
Consider restructuring the mock to return appropriate data from the database mock chain instead, or acknowledge this as a temporary scaffold.
Also applies to: 824-827, 886-889, 911-914
apps/web/src/app/api/ai/chat/messages/[messageId]/undo/__tests__/route.test.ts (1)
168-192: Consider expanding response field assertions.The success test verifies
messagesAffectedandactivitiesAffected, but doesn't verify other returned fields from the mock (messageId,conversationId,pageId,driveId,warnings). Consider adding assertions for additional fields to strengthen the contract test.🔎 Suggested additional assertions
expect(response.status).toBe(200); expect(body.messagesAffected).toBe(5); expect(body.activitiesAffected).toHaveLength(1); + expect(body.messageId).toBe(mockMessageId); + expect(body.pageId).toBe(mockPageId); + expect(body.warnings).toEqual([]); });
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
apps/web/src/app/api/activities/[activityId]/rollback/__tests__/route.test.tsapps/web/src/app/api/ai/chat/messages/[messageId]/undo/__tests__/route.test.tsapps/web/src/app/api/pages/[pageId]/history/__tests__/route.test.tsapps/web/src/components/activity/ActivityDashboard.tsxapps/web/src/components/activity/ActivityItem.tsxapps/web/src/components/activity/ActivityTimeline.tsxapps/web/src/components/layout/right-sidebar/ai-assistant/SidebarActivityTab.tsxapps/web/src/services/api/__tests__/ai-undo-service.test.tsapps/web/src/services/api/__tests__/rollback-service.test.tspackages/lib/src/permissions/__tests__/rollback-permissions.test.ts
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{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
Always use message parts structure for message content:{ parts: [{ type: 'text', text: 'content' }] }
Use centralized permissions viaimport { getUserAccessLevel, canUserEditPage } from '@pagespace/lib/permissions';
Always use Drizzle client from@pagespace/dbfor database access:import { db, pages } from '@pagespace/db';
Files:
apps/web/src/services/api/__tests__/rollback-service.test.tspackages/lib/src/permissions/__tests__/rollback-permissions.test.tsapps/web/src/components/layout/right-sidebar/ai-assistant/SidebarActivityTab.tsxapps/web/src/app/api/ai/chat/messages/[messageId]/undo/__tests__/route.test.tsapps/web/src/app/api/activities/[activityId]/rollback/__tests__/route.test.tsapps/web/src/services/api/__tests__/ai-undo-service.test.tsapps/web/src/components/activity/ActivityItem.tsxapps/web/src/app/api/pages/[pageId]/history/__tests__/route.test.tsapps/web/src/components/activity/ActivityDashboard.tsxapps/web/src/components/activity/ActivityTimeline.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/services/api/__tests__/rollback-service.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/app/api/activities/[activityId]/rollback/__tests__/route.test.tsapps/web/src/services/api/__tests__/ai-undo-service.test.tsapps/web/src/app/api/pages/[pageId]/history/__tests__/route.test.ts
**/*.{ts,tsx,js,jsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Format code with Prettier
Files:
apps/web/src/services/api/__tests__/rollback-service.test.tspackages/lib/src/permissions/__tests__/rollback-permissions.test.tsapps/web/src/components/layout/right-sidebar/ai-assistant/SidebarActivityTab.tsxapps/web/src/app/api/ai/chat/messages/[messageId]/undo/__tests__/route.test.tsapps/web/src/app/api/activities/[activityId]/rollback/__tests__/route.test.tsapps/web/src/services/api/__tests__/ai-undo-service.test.tsapps/web/src/components/activity/ActivityItem.tsxapps/web/src/app/api/pages/[pageId]/history/__tests__/route.test.tsapps/web/src/components/activity/ActivityDashboard.tsxapps/web/src/components/activity/ActivityTimeline.tsx
apps/web/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/web/src/**/*.{ts,tsx}: Register editing state usinguseEditingStore.getState().startEditing()and.endEditing()to prevent UI refreshes during document editing
Register streaming state usinguseEditingStore.getState().startStreaming()and.endStreaming()to prevent UI refreshes during AI streaming
Use SWR withisPaused()to prevent refetches during editing, but always allow the initial fetch to complete before pausing
Files:
apps/web/src/services/api/__tests__/rollback-service.test.tsapps/web/src/components/layout/right-sidebar/ai-assistant/SidebarActivityTab.tsxapps/web/src/app/api/ai/chat/messages/[messageId]/undo/__tests__/route.test.tsapps/web/src/app/api/activities/[activityId]/rollback/__tests__/route.test.tsapps/web/src/services/api/__tests__/ai-undo-service.test.tsapps/web/src/components/activity/ActivityItem.tsxapps/web/src/app/api/pages/[pageId]/history/__tests__/route.test.tsapps/web/src/components/activity/ActivityDashboard.tsxapps/web/src/components/activity/ActivityTimeline.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/SidebarActivityTab.tsxapps/web/src/components/activity/ActivityItem.tsxapps/web/src/components/activity/ActivityDashboard.tsxapps/web/src/components/activity/ActivityTimeline.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/SidebarActivityTab.tsxapps/web/src/components/activity/ActivityItem.tsxapps/web/src/components/activity/ActivityDashboard.tsxapps/web/src/components/activity/ActivityTimeline.tsx
**/*ai*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Vercel AI SDK for AI integrations
Files:
apps/web/src/services/api/__tests__/ai-undo-service.test.ts
🧠 Learnings (2)
📚 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
📚 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: 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 (9)
apps/web/src/services/api/__tests__/rollback-service.test.ts (3)
packages/db/src/index.ts (1)
db(20-20)packages/lib/src/permissions/rollback-permissions.ts (1)
isRollbackableOperation(167-187)packages/lib/src/monitoring/activity-logger.ts (1)
logRollbackActivity(644-697)
packages/lib/src/permissions/__tests__/rollback-permissions.test.ts (1)
packages/lib/src/permissions/rollback-permissions.ts (5)
ActivityForPermissionCheck(24-33)isRollbackableOperation(167-187)isRollbackableResourceType(151-162)isActivityEligibleForRollback(194-203)canUserRollback(52-146)
apps/web/src/components/layout/right-sidebar/ai-assistant/SidebarActivityTab.tsx (5)
apps/web/src/hooks/useToast.ts (1)
useToast(13-43)apps/web/src/components/activity/ActivityItem.tsx (1)
ActivityItem(29-153)apps/web/src/lib/auth/auth-fetch.ts (2)
fetchWithAuth(704-705)fetch(44-211)apps/web/src/components/ui/dropdown-menu.tsx (4)
DropdownMenu(242-242)DropdownMenuTrigger(244-244)DropdownMenuContent(245-245)DropdownMenuItem(248-248)apps/web/src/components/ui/button.tsx (1)
Button(59-59)
apps/web/src/app/api/ai/chat/messages/[messageId]/undo/__tests__/route.test.ts (2)
apps/web/src/services/api/ai-undo-service.ts (2)
previewAiUndo(131-227)executeAiUndo(232-345)apps/web/src/lib/repositories/global-conversation-repository.ts (1)
globalConversationRepository(135-330)
apps/web/src/services/api/__tests__/ai-undo-service.test.ts (2)
packages/db/src/index.ts (1)
db(20-20)packages/lib/src/monitoring/activity-logger.ts (1)
logConversationUndo(704-743)
apps/web/src/components/activity/ActivityItem.tsx (6)
apps/web/src/components/activity/types.ts (1)
ActivityLog(10-31)apps/web/src/components/activity/index.ts (1)
ActivityItem(3-3)apps/web/src/hooks/useToast.ts (1)
useToast(13-43)apps/web/src/components/activity/constants.ts (2)
operationConfig(17-30)defaultOperationConfig(39-43)apps/web/src/components/ui/dropdown-menu.tsx (4)
DropdownMenu(242-242)DropdownMenuTrigger(244-244)DropdownMenuContent(245-245)DropdownMenuItem(248-248)apps/web/src/components/ui/button.tsx (1)
Button(59-59)
apps/web/src/app/api/pages/[pageId]/history/__tests__/route.test.ts (1)
packages/lib/src/permissions/rollback-permissions.ts (1)
isActivityEligibleForRollback(194-203)
apps/web/src/components/activity/ActivityDashboard.tsx (1)
apps/web/src/components/activity/ActivityItem.tsx (1)
RollbackContext(21-21)
apps/web/src/components/activity/ActivityTimeline.tsx (1)
apps/web/src/components/activity/ActivityItem.tsx (2)
RollbackContext(21-21)ActivityItem(29-153)
⏰ 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 (31)
apps/web/src/app/api/pages/[pageId]/history/__tests__/route.test.ts (8)
1-42: LGTM! Mock setup is well-organized.The mock hoisting pattern with
vi.mockbefore imports is correct for Vitest. The separation of@pagespace/liband@pagespace/lib/permissionsmocks handles the different module entry points appropriately.
43-67: LGTM! Test helpers are well-structured.The
mockParamsas a Promise aligns with Next.js 15's async params pattern. The helper functions provide clean abstractions for test setup.
69-86: LGTM! Factory function provides flexible test data creation.The default values correctly represent a rollback-eligible activity (with
previousValuesset), and the spread pattern allows easy customization per test case.
88-106: LGTM! Test setup with fake timers ensures deterministic retention calculations.The
beforeEach/afterEachpattern properly isolates tests, and the fixed system time (2024-06-15) enables reliable date arithmetic in retention tests.
108-133: LGTM! Authentication tests cover key scenarios.Good practice verifying that
getPageVersionHistoryis not called when authentication fails, ensuring proper request isolation.
155-251: LGTM! Query parameter tests cover the main validation contract.The tests validate bounds (limit ≤ 100, offset ≥ 0) and filter passthrough correctly. Optional enhancement: consider adding tests for malformed inputs (non-numeric strings, invalid date formats) if the route handler is expected to handle them gracefully.
257-358: LGTM! Retention tests comprehensively cover tier-based limits.The use of fake timers ensures deterministic date calculations. The tolerance range (9-11 days) in line 320-321 appropriately handles potential boundary conditions in date arithmetic.
364-416: LGTM! Response format tests verify the API contract accurately.The sequential
mockReturnValueOncecalls correctly test thatcanRollbackis computed per-activity. PaginationhasMorelogic is verified with proper boundary cases.apps/web/src/services/api/__tests__/ai-undo-service.test.ts (2)
1-18: Good documentation of testing approach.The scaffold comment clearly documents the trade-off with order-dependent ORM chain mocks and suggests a future improvement path (repository seam). This transparency helps maintainers understand test fragility.
626-691: Good edge case coverage.The tests for single-message scenarios and pages without driveId (global assistant) cover important boundary conditions that could easily be overlooked.
packages/lib/src/permissions/__tests__/rollback-permissions.test.ts (1)
1-370: Well-structured test suite with comprehensive coverage.The test file follows good practices:
- Clear separation between pure function tests and permission check tests
- Proper mocking at the boundary (permissions module)
- Good use of
beforeEachfor mock cleanup- Comprehensive coverage of edge cases including unknown contexts
apps/web/src/components/layout/right-sidebar/ai-assistant/SidebarActivityTab.tsx (2)
249-277: Rollback execution correctly usespost()helper.The rollback POST request uses the
post()helper which should handle CSRF tokens automatically, unlike the issue reported inVersionHistoryPanel.tsx. This implementation is consistent with the codebase patterns.
411-430: Clean dropdown menu implementation for rollback actions.The rollback action UI is well-implemented with appropriate hover behavior via the
groupclass and transition effects. Good use of accessibility withsr-onlyfor screen readers.apps/web/src/components/activity/ActivityTimeline.tsx (2)
5-31: Clean prop threading for rollback functionality.The changes correctly:
- Import
RollbackContexttype fromActivityItem- Add optional props to maintain backward compatibility
- Thread
contextandonRollbackto child components
61-66: Props correctly passed to ActivityItem.The
contextandonRollbackprops are properly propagated to eachActivityItem, enabling context-aware rollback behavior.apps/web/src/components/activity/ActivityDashboard.tsx (2)
209-230: Rollback handler correctly implemented.The
handleRollbackfunction:
- Uses
post()helper which handles CSRF automatically- Maps context correctly (
user→user_dashboard)- Has proper error handling with user feedback via toast
- Refreshes the activity list after successful rollback
327-328: Rollback props correctly wired to ActivityTimeline.The
contextandonRollbackprops are properly passed to enable rollback functionality throughout the timeline.apps/web/src/app/api/activities/[activityId]/rollback/__tests__/route.test.ts (3)
77-92: Good CSRF validation test.This test verifies that the route handler requires CSRF protection, which aligns with the PR requirement. The test confirms
requireCSRF: trueis passed to the authentication options.
130-146: Comprehensive context validation testing.Tests all valid context values (
page,drive,ai_tool,user_dashboard) ensuring the route accepts each. Good coverage of the validation boundary.
1-271: Well-structured contract test suite.The test file follows good practices:
- Clear documentation of test contract
- Proper mocking at service boundaries
- Comprehensive coverage of auth, validation, dry-run, and execution paths
- Helper functions for request/auth creation improve readability
apps/web/src/components/activity/ActivityItem.tsx (2)
21-21: Good type export for reuse.Exporting
RollbackContextas a type allows other components (likeActivityTimelineandActivityDashboard) to import and use it, promoting type consistency across the rollback feature.
119-150: Well-implemented rollback UI with confirmation dialog.Good implementation:
- Conditional rendering based on
canRestore- Proper accessibility with
sr-onlytext- Clean integration with
RollbackConfirmDialog- Fragment wrapper handles multiple root elements correctly
apps/web/src/services/api/__tests__/rollback-service.test.ts (2)
1-15: Good scaffold documentation.The comment clearly explains the mocking strategy and references the rubric for future repository abstraction. This helps future maintainers understand why the complex mocking approach was chosen.
402-458: Comprehensive rollback execution test with audit logging verification.Good test that verifies:
- Database update is performed
- Audit log (
logRollbackActivity) is called with correct parameters- Success response includes restored values
apps/web/src/app/api/ai/chat/messages/[messageId]/undo/__tests__/route.test.ts (7)
1-55: Well-structured mock setup.The mock configuration properly isolates the route handler from its dependencies, covering the service boundary, authentication, repository, permissions, and logging. This follows good testing patterns.
57-88: LGTM!Test helpers are well-typed and follow good patterns. The
mockParamsas a Promise correctly matches Next.js 13+ dynamic route params API.
90-162: Good coverage of authentication and authorization paths.Tests properly verify:
- 401 when unauthenticated (and service not invoked)
- 404 when preview returns null
- 403 for both page edit permission and global conversation ownership
The authorization tests cover both
page_chatandglobal_chatsources, which is thorough.
222-229: Good CSRF requirement verification.This test confirms the endpoint requires CSRF protection by verifying
requireCSRF: trueis passed toauthenticateRequestWithOptions. This aligns with the PR comment noting that frontend POST requests need to include CSRF tokens.
236-279: Comprehensive input validation coverage.The validation tests appropriately cover:
- Invalid mode rejection (400)
- Missing mode rejection (400)
- Both valid modes accepted (
messages_only,messages_and_changes)Including "accepts valid input" tests in the validation block is a good practice for completeness.
301-335: LGTM!Success tests properly verify both modes return 200 with appropriate response bodies, including the human-readable message format.
359-372: LGTM!The complete failure test correctly verifies that when no operations succeed (
messagesDeleted: 0,activitiesRolledBack: 0), the endpoint returns 500 withsuccess: false.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/lib/src/monitoring/__tests__/activity-logger-compliance.test.ts (1)
499-625: Comprehensive rollback activity logging tests.The test suite thoroughly validates
logRollbackActivitywith excellent coverage of all key scenarios: basic operation logging, value restoration mapping, audit trail preservation, and content snapshots. The comment on line 556 helpfully clarifies the previousValues/newValues mapping semantics.Optional: Add test for minimal parameters
Consider adding a test case that calls
logRollbackActivitywithout the optionaloptionsparameter to explicitly verify it handles minimal parameters gracefully:it('should handle rollback with minimal parameters', async () => { // Arrange & Act logRollbackActivity( 'user-123', 'source-activity-456', { resourceType: 'page', resourceId: 'page-1', driveId: 'drive-1', }, { actorEmail: 'john@example.com' } // No options parameter ); // Wait for async execution await vi.waitFor(() => { expect(capturedInsertValues).not.toBeNull(); }); // Assert - verify basic fields are logged without options expect(capturedInsertValues).toMatchObject({ operation: 'rollback', resourceType: 'page', resourceId: 'page-1', driveId: 'drive-1', rollbackFromActivityId: 'source-activity-456', }); });This explicitly documents the minimal usage pattern, though the current implementation already handles this safely via optional chaining.
apps/web/src/services/api/ai-undo-service.ts (2)
253-258: Vestigialerrorsarray is unused in success path.The
errorsarray (line 253) is never populated in the try block after adopting all-or-nothing semantics. Consequently,success: errors.length === 0(line 342) will always betruein the success path.Consider simplifying to
success: truedirectly, or removing theerrorsarray from the success result since failures now throw.🔎 Proposed simplification
return { - success: errors.length === 0, + success: true, messagesDeleted, activitiesRolledBack, - errors, + errors: [], };Also applies to: 341-346
354-359: Return values may be misleading after transaction rollback.When the transaction fails and rolls back,
activitiesRolledBack(line 357) may contain a non-zero value from iterations before the failure (line 297), even though no changes were actually persisted. This could confuse callers.Consider resetting counters to 0 in the error path to accurately reflect the rolled-back state:
🔎 Proposed fix
return { success: false, - messagesDeleted, - activitiesRolledBack, + messagesDeleted: 0, + activitiesRolledBack: 0, errors: [...errors, error instanceof Error ? error.message : 'Unknown error'], };
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/web/src/services/api/ai-undo-service.tsapps/web/src/services/api/rollback-service.tspackages/lib/src/monitoring/__tests__/activity-logger-compliance.test.tspackages/lib/src/permissions/rollback-permissions.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/web/src/services/api/rollback-service.ts
- packages/lib/src/permissions/rollback-permissions.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{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
Always use message parts structure for message content:{ parts: [{ type: 'text', text: 'content' }] }
Use centralized permissions viaimport { getUserAccessLevel, canUserEditPage } from '@pagespace/lib/permissions';
Always use Drizzle client from@pagespace/dbfor database access:import { db, pages } from '@pagespace/db';
Files:
apps/web/src/services/api/ai-undo-service.tspackages/lib/src/monitoring/__tests__/activity-logger-compliance.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/services/api/ai-undo-service.tspackages/lib/src/monitoring/__tests__/activity-logger-compliance.test.ts
**/*.{ts,tsx,js,jsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Format code with Prettier
Files:
apps/web/src/services/api/ai-undo-service.tspackages/lib/src/monitoring/__tests__/activity-logger-compliance.test.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
apps/web/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/web/src/**/*.{ts,tsx}: Register editing state usinguseEditingStore.getState().startEditing()and.endEditing()to prevent UI refreshes during document editing
Register streaming state usinguseEditingStore.getState().startStreaming()and.endStreaming()to prevent UI refreshes during AI streaming
Use SWR withisPaused()to prevent refetches during editing, but always allow the initial fetch to complete before pausing
Files:
apps/web/src/services/api/ai-undo-service.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-22T20:04:40.892Z
Learning: Applies to packages/db/src/schema.ts : Update database schema in `packages/db/src/schema.ts` and generate migrations with `pnpm db:generate`
📚 Learning: 2025-12-23T04:55:07.402Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-23T04:55:07.402Z
Learning: Applies to packages/db/**/*.{ts,tsx} : Use Drizzle ORM for all database operations in the centralized `packages/db` package
Applied to files:
apps/web/src/services/api/ai-undo-service.ts
🧬 Code graph analysis (2)
apps/web/src/services/api/ai-undo-service.ts (4)
packages/db/src/schema/core.ts (2)
chatMessages(68-89)pages(24-66)packages/db/src/schema/conversations.ts (1)
messages(30-46)apps/web/src/services/api/rollback-service.ts (3)
RollbackContext(16-16)previewRollback(161-281)executeRollback(287-393)packages/lib/src/permissions/rollback-permissions.ts (1)
RollbackContext(15-19)
packages/lib/src/monitoring/__tests__/activity-logger-compliance.test.ts (1)
packages/lib/src/monitoring/activity-logger.ts (2)
logRollbackActivity(644-697)logConversationUndo(704-743)
⏰ 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 (7)
packages/lib/src/monitoring/__tests__/activity-logger-compliance.test.ts (2)
39-40: LGTM: New wrapper imports added correctly.The new convenience wrapper imports are properly integrated into the existing import block.
627-768: Excellent conversation undo logging tests.The test suite provides comprehensive validation of
logConversationUndowith strong coverage of both operation modes, metadata handling, and edge cases. Particularly well done:
- Tests verify correct operation type mapping based on mode (lines 661-686)
- Tests validate the semantic meaning of
previousValuesindicating active messages (lines 718-741)- Tests cover the null driveId edge case for global conversations (lines 743-767)
apps/web/src/services/api/ai-undo-service.ts (5)
1-76: LGTM! Well-structured types and imports.The module header, imports, and type definitions are clean. Imports follow the coding guidelines (Drizzle from
@pagespace/db, loggers from@pagespace/lib/server), and all interfaces are properly typed without anyanyusage.
81-115: LGTM!The dual-table lookup is a clean abstraction that normalizes messages from both
chat_messagesandmessagestables into a consistentAiMessageshape.
120-126: LGTM!Efficient helper that selects only the required
driveIdcolumn.
174-177: Acknowledge the timing edge case.The comment at lines 175-176 correctly notes that tool calls in the first turn might be missed if there's no preceding message. This is acceptable since user messages typically precede AI responses. The fallback to
createdAtis a reasonable default.
272-316: Transaction implementation correctly addresses past review concerns.The all-or-nothing transaction semantics are properly implemented:
- All operations (rollbacks + message soft-deletes) are wrapped in a single transaction.
- Any failure throws to abort the entire transaction.
- Context determination aligns with preview logic (comment at lines 284-285 explains this).
This addresses the previous review feedback about partial success risks.
- 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>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/web/src/app/api/ai/chat/messages/[messageId]/undo/route.ts (1)
159-169: Consider simplifying status code logic given all-or-nothing semantics.With the all-or-nothing transaction semantics in
executeAiUndo, partial success is no longer possible—the operation either fully succeeds or fully fails. Once the counter reset issue inai-undo-service.tsis addressed (see my comment on that file), bothmessagesDeletedandactivitiesRolledBackwill always be0whensuccessisfalse, making the 207 status code unreachable.Consider simplifying to always return 500 on failure:
🔎 Simplified approach
if (!result.success && result.errors.length > 0) { - // Partial success or failure + // Operation failed - transaction was rolled back return NextResponse.json( { ...result, success: false, message: 'Some operations could not be completed', }, - { status: result.messagesDeleted > 0 || result.activitiesRolledBack > 0 ? 207 : 500 } + { status: 500 } ); }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
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/services/api/ai-undo-service.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/src/app/api/ai/chat/messages/[messageId]/undo/tests/route.test.ts
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{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
Always use message parts structure for message content:{ parts: [{ type: 'text', text: 'content' }] }
Use centralized permissions viaimport { getUserAccessLevel, canUserEditPage } from '@pagespace/lib/permissions';
Always use Drizzle client from@pagespace/dbfor database access:import { db, pages } from '@pagespace/db';
Files:
apps/web/src/services/api/ai-undo-service.tsapps/web/src/app/api/ai/chat/messages/[messageId]/undo/route.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/services/api/ai-undo-service.tsapps/web/src/app/api/ai/chat/messages/[messageId]/undo/route.ts
**/*.{ts,tsx,js,jsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Format code with Prettier
Files:
apps/web/src/services/api/ai-undo-service.tsapps/web/src/app/api/ai/chat/messages/[messageId]/undo/route.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
apps/web/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/web/src/**/*.{ts,tsx}: Register editing state usinguseEditingStore.getState().startEditing()and.endEditing()to prevent UI refreshes during document editing
Register streaming state usinguseEditingStore.getState().startStreaming()and.endStreaming()to prevent UI refreshes during AI streaming
Use SWR withisPaused()to prevent refetches during editing, but always allow the initial fetch to complete before pausing
Files:
apps/web/src/services/api/ai-undo-service.tsapps/web/src/app/api/ai/chat/messages/[messageId]/undo/route.ts
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,paramsin dynamic routes are Promise objects. You MUST awaitcontext.paramsbefore destructuring in route handlers.
Get request body usingconst body = await request.json();
Get search params usingconst { searchParams } = new URL(request.url);
Return JSON usingResponse.json(data)orNextResponse.json(data)
Files:
apps/web/src/app/api/ai/chat/messages/[messageId]/undo/route.ts
🧠 Learnings (4)
📚 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/app/api/ai/chat/messages/[messageId]/undo/route.ts
📚 Learning: 2025-12-14T14:54:38.009Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T14:54:38.009Z
Learning: Applies to apps/**/*.ts : Always use centralized permission functions from `pagespace/lib/permissions` for access control logic (e.g., `getUserAccessLevel`, `canUserEditPage`)
Applied to files:
apps/web/src/app/api/ai/chat/messages/[messageId]/undo/route.ts
📚 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 app/**/*.{ts,tsx} : Import and use `getUserAccessLevel()` and `canUserEditPage()` from `pagespace/lib/permissions` for centralized permission logic
Applied to files:
apps/web/src/app/api/ai/chat/messages/[messageId]/undo/route.ts
📚 Learning: 2025-12-23T04:55:07.402Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-23T04:55:07.402Z
Learning: Applies to **/*.{ts,tsx} : Use centralized permissions via `import { getUserAccessLevel, canUserEditPage } from 'pagespace/lib/permissions';`
Applied to files:
apps/web/src/app/api/ai/chat/messages/[messageId]/undo/route.ts
🧬 Code graph analysis (2)
apps/web/src/services/api/ai-undo-service.ts (2)
packages/lib/src/permissions/rollback-permissions.ts (1)
RollbackContext(15-19)apps/web/src/services/api/rollback-service.ts (3)
RollbackContext(16-16)previewRollback(161-281)executeRollback(287-393)
apps/web/src/app/api/ai/chat/messages/[messageId]/undo/route.ts (2)
apps/web/src/services/api/ai-undo-service.ts (3)
AiUndoPreview(25-43)previewAiUndo(131-248)executeAiUndo(254-368)apps/web/src/lib/logging/mask.ts (1)
maskIdentifier(1-12)
⏰ 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 (3)
apps/web/src/app/api/ai/chat/messages/[messageId]/undo/route.ts (3)
22-60: Excellent refactoring to eliminate duplication.The extraction of permission checking logic into a shared helper successfully addresses the previous review feedback about DRY violation. The implementation correctly uses the centralized
canUserEditPagefunction and verifies conversation ownership for global chats.
66-106: GET handler correctly implements Next.js 15 patterns.The handler properly awaits the
paramsPromise (line 76), uses appropriate authentication options, and leverages the shared permission helper. Error handling and response codes are appropriate.
112-158: POST handler correctly implements authentication, validation, and optimization.The handler properly:
- Awaits the
paramsPromise per Next.js 15 requirements (line 122)- Uses CSRF-protected authentication for writes (line 118)
- Validates the request body with Zod (lines 126-132)
- Passes the preview to
executeAiUndo(line 147) to avoid redundant database queries—a good optimization
* feat: add version history and rollback functionality 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> * fix: complete rollback handler implementations and fix lint errors - 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> * feat: add undo AI changes feature for conversations 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> * fix: add missing radio-group UI component Required for UndoAiChangesDialog mode selection. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: add @radix-ui/react-radio-group dependency 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> * fix: restore correct @electron/node-gyp resolution in lockfile 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> * fix: add critical rules to CLAUDE.md and fix retention tier order - 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> * fix(db): add missing default and rollback index - Add DEFAULT now() for retention_policies.updatedAt column - Add index on activity_logs.rollbackFromActivityId for queries - Create migration 0027_fix_retention_updated_at.sql Addresses PR #118 review issues #10, #20 * fix(lib/permissions): add block scoping and eligibility helper - 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> * fix(lib/monitoring): add rollback fields to ActivityLogInput - 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> * fix(api): add authorization and resourceType filtering - 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> * fix(services): improve type safety and add transactions - 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> * fix(ui): add CSRF protection and improve error handling - 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> * fix(db): address schema issues from PR #118 review 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> * fix(lib): move rollback fields to top-level in activity logger 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> * fix(services): add transaction support to rollback service 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> * fix(services): improve atomicity and context logic in AI undo 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> * fix(api): use Zod schema for undo route validation 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> * fix: address CodeRabbit review feedback - 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> * fix(db): add subscriptionTier enum for retention policies - 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> * fix: address final CodeRabbit review comments - 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> * fix(db): add USING clause for text-to-enum cast 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> * fix(ai): support undo for global assistant messages by checking both messages tables and updating permission logic * fix(ai): resolve lint errors in undo route and tests * test: add contract tests for version history rollback - 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> * feat(ui): add rollback capability to activity views 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> * test: add activity logger tests for rollback and undo operations 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> * fix(rollback): support create operation rollback and global messages - 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: address PR #118 code review feedback - 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> * fix: resolve header loading and rendering issues across all page types * style: add truncation for page titles and breadcrumb items --------- 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>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/lib/src/permissions/__tests__/rollback-permissions.test.ts (1)
110-126: Consider removing unnecessary type assertion for valid types.Line 115 uses
as anyfor types that should already be validActivityResourceTypevalues. This weakens type safety unnecessarily.Line 123's
as anyis acceptable for testing runtime behavior with invalid inputs, but could be documented with a comment explaining the intent.🔎 Proposed improvement
it.each(rollbackableTypes)('returns true for %s resource type', (type) => { - expect(isRollbackableResourceType(type as any)).toBe(true); + expect(isRollbackableResourceType(type as ActivityResourceType)).toBe(true); }); }); describe('non-rollbackable resource types', () => { const nonRollbackableTypes = ['user', 'file', 'token', 'device', 'conversation']; it.each(nonRollbackableTypes)('returns false for %s resource type', (type) => { + // Testing runtime behavior with invalid types expect(isRollbackableResourceType(type as any)).toBe(false); });As per coding guidelines, avoid using
anytypes where possible.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/lib/src/permissions/__tests__/rollback-permissions.test.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{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:
packages/lib/src/permissions/__tests__/rollback-permissions.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:
packages/lib/src/permissions/__tests__/rollback-permissions.test.ts
**/*.{ts,tsx,js,jsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Format code with Prettier
Files:
packages/lib/src/permissions/__tests__/rollback-permissions.test.ts
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-22T20:04:40.892Z
Learning: Applies to packages/db/src/schema.ts : Update database schema in `packages/db/src/schema.ts` and generate migrations with `pnpm db:generate`
🧬 Code graph analysis (1)
packages/lib/src/permissions/__tests__/rollback-permissions.test.ts (1)
packages/lib/src/permissions/rollback-permissions.ts (4)
ActivityForPermissionCheck(24-33)isRollbackableOperation(167-188)isRollbackableResourceType(151-162)isActivityEligibleForRollback(195-204)
🔇 Additional comments (6)
packages/lib/src/permissions/__tests__/rollback-permissions.test.ts (6)
1-31: LGTM! Well-structured test setup.The imports, mocking setup, and test fixtures are properly organized. The mock is correctly placed before the imports that depend on it, and the fixture data provides clear test values.
32-42: LGTM! Excellent test fixture pattern.The factory function provides type-safe activity creation with sensible defaults and flexible overrides.
44-47: LGTM! Proper test isolation.The
beforeEachhook correctly clears mocks between tests.
53-108: LGTM! Comprehensive operation coverage.The tests cover all rollbackable and non-rollbackable operations, including the 'create' operation mentioned in the commit message. The inline comment on line 94 helpfully documents the rollback prevention logic.
128-188: LGTM! Thorough eligibility checks.The tests comprehensively cover all eligibility conditions, including the critical check that prevents rolling back a rollback operation (lines 179-187), which prevents infinite rollback chains.
194-369: LGTM! Comprehensive permission coverage.The tests thoroughly cover all rollback contexts (AI tool, page, drive, user dashboard, unknown) with proper verification of both success and failure cases. The tests correctly:
- Verify permission checks are called with expected arguments
- Test missing identifier scenarios (pageId, driveId)
- Validate both
canRollbackboolean andreasonmessages- Prevent rollback-of-rollback infinite chains
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/web/src/app/api/ai/chat/messages/[messageId]/undo/route.ts (1)
159-168: Error message misleading given all-or-nothing transaction semantics.Since
executeAiUndonow uses all-or-nothing transaction semantics (verified in ai-undo-service.ts lines 281-323), whenresult.successis false, the entire operation failed and was rolled back—no partial changes were applied. The message "Some operations could not be completed" incorrectly suggests partial success.🔎 Proposed fix to clarify failure message
if (!result.success && result.errors.length > 0) { return NextResponse.json( { ...result, success: false, - message: 'Some operations could not be completed', + message: 'Operation failed', }, { status: 500 } ); }Additionally, the condition at line 159 can be simplified since
errors.length > 0is always true whensuccessis false:- if (!result.success && result.errors.length > 0) { + if (!result.success) { return NextResponse.json( { ...result, success: false, message: 'Operation failed', }, { status: 500 } ); }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/src/app/api/ai/chat/messages/[messageId]/undo/route.tsapps/web/src/services/api/ai-undo-service.ts
🧰 Additional context used
📓 Path-based instructions (6)
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/services/api/ai-undo-service.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/services/api/ai-undo-service.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/services/api/ai-undo-service.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/services/api/ai-undo-service.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 (5)
📓 Common learnings
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T14:54:45.713Z
Learning: Tech stack: Next.js 15 App Router + TypeScript + Tailwind + shadcn/ui (frontend), PostgreSQL + Drizzle ORM (database), Ollama + Vercel AI SDK + OpenRouter + Google AI SDK (AI), custom JWT auth, local filesystem storage, Socket.IO for real-time, Docker deployment
📚 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/app/api/ai/chat/messages/[messageId]/undo/route.ts
📚 Learning: 2025-12-14T14:54:38.009Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T14:54:38.009Z
Learning: Applies to apps/**/*.ts : Always use centralized permission functions from `pagespace/lib/permissions` for access control logic (e.g., `getUserAccessLevel`, `canUserEditPage`)
Applied to files:
apps/web/src/app/api/ai/chat/messages/[messageId]/undo/route.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/app/**/route.{ts,tsx} : For permission logic, use centralized functions from `pagespace/lib/permissions`: `getUserAccessLevel()`, `canUserEditPage()`
Applied to files:
apps/web/src/app/api/ai/chat/messages/[messageId]/undo/route.ts
📚 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 app/**/*.{ts,tsx} : Import and use `getUserAccessLevel()` and `canUserEditPage()` from `pagespace/lib/permissions` for centralized permission logic
Applied to files:
apps/web/src/app/api/ai/chat/messages/[messageId]/undo/route.ts
🧬 Code graph analysis (1)
apps/web/src/app/api/ai/chat/messages/[messageId]/undo/route.ts (1)
apps/web/src/services/api/ai-undo-service.ts (3)
AiUndoPreview(25-43)previewAiUndo(131-248)executeAiUndo(254-368)
🔇 Additional comments (4)
apps/web/src/services/api/ai-undo-service.ts (1)
1-368: LGTM! All previous issues have been successfully addressed.The service implementation is well-structured with:
- Proper all-or-nothing transaction semantics (lines 281-323) with throws on failure
- Consistent context determination between preview and execute functions
- Correct counter reset to 0 in the catch block (lines 363-364)
- Clean separation of concerns with helper functions
- Comprehensive error handling and logging
The code follows TypeScript best practices and integrates properly with Drizzle ORM and the rollback service.
apps/web/src/app/api/ai/chat/messages/[messageId]/undo/route.ts (3)
1-60: LGTM! Excellent implementation of authentication and permission checking.The code properly:
- Requires CSRF protection for write operations (line 16)
- Uses centralized
canUserEditPagepermission function (line 32)- Eliminates duplication with the
checkUndoPermissionshelper- Follows Next.js 15 conventions
66-106: LGTM! GET handler follows Next.js 15 best practices.The preview endpoint correctly:
- Awaits
context.paramsper Next.js 15 requirements (line 76)- Uses read-only authentication without CSRF
- Checks permissions before returning preview
- Handles errors appropriately
112-186: POST handler implementation is solid.The execution endpoint correctly:
- Requires CSRF protection via AUTH_OPTIONS_WRITE
- Awaits
context.paramsper Next.js 15 requirements (line 122)- Validates request body with Zod
- Reuses preview to avoid redundant computation (line 147)
- Returns appropriate success messages based on mode
🤖 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>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/web/src/hooks/__tests__/usePermissions.test.ts (1)
99-120: Enhance test coverage for the "not editing after load" scenario.The test currently validates
isPaused()returnsfalsebefore the initial load completes (sinceonSuccessisn't called,hasLoadedRef.currentremainsfalse). However, it doesn't test the intended scenario described in the test name: after initial load completes, when not editing,isPausedshould returnfalse.For comprehensive coverage, this test should also trigger
onSuccess(like tests 1 and 2) to sethasLoadedRef.current = true, then verifyisPaused()returnsfalsewhenisAnyActive = false. This would properly test the logic:hasLoadedRef.current && isAnyActive→true && false = false.🔎 Suggested enhancement
it('given user is not editing or streaming, should allow permission revalidation', () => { // Arrange: No active editing/streaming vi.mocked(useEditingStore).mockReturnValue(false); // isAnyActive returns false - vi.mocked(useSWR).mockReturnValue({ - data: undefined, - error: undefined, - isLoading: false, - mutate: vi.fn(), - isValidating: false, - } as SWRResponse); + // 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); + } + return { + data: { canView: true, canEdit: true, canShare: true, canDelete: true }, + error: undefined, + isLoading: false, + mutate: vi.fn(), + isValidating: false, + } as SWRResponse; + }); // Act: Render hook renderHook(() => usePermissions('page-123')); - // Assert: SWR isPaused returns false + // Assert: SWR isPaused returns false (after initial load, when not editing) const swrCall = vi.mocked(useSWR).mock.calls[0]; const swrConfig = swrCall[2] as { isPaused?: () => boolean }; expect(swrConfig.isPaused).toBeDefined(); expect(swrConfig.isPaused!()).toBe(false); });apps/web/src/app/api/ai/chat/messages/[messageId]/undo/__tests__/route.test.ts (1)
349-365: Test updates align with standardized error handling.The test correctly validates the new 500 response with the "Undo failed. No changes were applied." message.
Consider adding a test case for the edge scenario where
executeAiUndoreturns{ success: false, errors: [] }to ensure it's handled correctly (this is especially relevant given the condition on route.ts line 159).🔎 Optional additional test
it('returns 500 when operation fails without error details', async () => { (executeAiUndo as Mock).mockResolvedValue({ success: false, messagesDeleted: 0, activitiesRolledBack: 0, errors: [], }); const response = await POST(createPostRequest({ mode: 'messages_only' }), { params: mockParams }); const body = await response.json(); expect(response.status).toBe(500); expect(body.success).toBe(false); });
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
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/layout/middle-content/CenterPanel.tsxapps/web/src/hooks/__tests__/useBreadcrumbs.test.tsapps/web/src/hooks/__tests__/usePermissions.test.ts
💤 Files with no reviewable changes (1)
- apps/web/src/components/layout/middle-content/CenterPanel.tsx
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{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/hooks/__tests__/useBreadcrumbs.test.tsapps/web/src/hooks/__tests__/usePermissions.test.tsapps/web/src/app/api/ai/chat/messages/[messageId]/undo/route.tsapps/web/src/app/api/ai/chat/messages/[messageId]/undo/__tests__/route.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/hooks/__tests__/useBreadcrumbs.test.tsapps/web/src/hooks/__tests__/usePermissions.test.tsapps/web/src/app/api/ai/chat/messages/[messageId]/undo/route.tsapps/web/src/app/api/ai/chat/messages/[messageId]/undo/__tests__/route.test.ts
**/*.{ts,tsx,js,jsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Format code with Prettier
Files:
apps/web/src/hooks/__tests__/useBreadcrumbs.test.tsapps/web/src/hooks/__tests__/usePermissions.test.tsapps/web/src/app/api/ai/chat/messages/[messageId]/undo/route.tsapps/web/src/app/api/ai/chat/messages/[messageId]/undo/__tests__/route.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/hooks/__tests__/useBreadcrumbs.test.tsapps/web/src/hooks/__tests__/usePermissions.test.tsapps/web/src/app/api/ai/chat/messages/[messageId]/undo/route.tsapps/web/src/app/api/ai/chat/messages/[messageId]/undo/__tests__/route.test.ts
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
🧠 Learnings (6)
📚 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-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.tsapps/web/src/app/api/ai/chat/messages/[messageId]/undo/route.ts
📚 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 app/**/*.{ts,tsx} : Import and use `getUserAccessLevel()` and `canUserEditPage()` from `pagespace/lib/permissions` for centralized permission logic
Applied to files:
apps/web/src/hooks/__tests__/usePermissions.test.tsapps/web/src/app/api/ai/chat/messages/[messageId]/undo/route.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-14T14:54:38.009Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T14:54:38.009Z
Learning: Applies to apps/**/*.ts : Always use centralized permission functions from `pagespace/lib/permissions` for access control logic (e.g., `getUserAccessLevel`, `canUserEditPage`)
Applied to files:
apps/web/src/hooks/__tests__/usePermissions.test.tsapps/web/src/app/api/ai/chat/messages/[messageId]/undo/route.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/app/**/route.{ts,tsx} : For permission logic, use centralized functions from `pagespace/lib/permissions`: `getUserAccessLevel()`, `canUserEditPage()`
Applied to files:
apps/web/src/hooks/__tests__/usePermissions.test.tsapps/web/src/app/api/ai/chat/messages/[messageId]/undo/route.ts
🧬 Code graph analysis (2)
apps/web/src/hooks/__tests__/useBreadcrumbs.test.ts (1)
apps/web/src/hooks/useBreadcrumbs.ts (1)
useBreadcrumbs(23-50)
apps/web/src/hooks/__tests__/usePermissions.test.ts (1)
apps/web/src/hooks/usePermissions.ts (1)
usePermissions(33-98)
⏰ 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 (2)
apps/web/src/hooks/__tests__/usePermissions.test.ts (1)
41-86: LGTM! Proper simulation of SWR load lifecycle.The
mockImplementationapproach correctly simulates the initial load completion by invoking theonSuccesscallback, which setshasLoadedRef.current = truein the actual hook. This allows the tests to properly validate thatisPausedreturnstruewhen editing is active after the initial load, matching the intended behavior from the hook implementation.apps/web/src/hooks/__tests__/useBreadcrumbs.test.ts (1)
175-180: LGTM! Test correctly validates null pageId behavior.The updated assertion and descriptive test name accurately reflect the hook's implementation, where
isLoadingevaluates tofalsewhenpageIdis null (!error && !data && !!pageId = false). The inline comment clearly documents the boolean logic evaluation.
- 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>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/web/src/hooks/__tests__/usePermissions.test.ts (1)
45-45: Consider using a more specific type cast for test clarity.The
{} as nevercast for the third parameter toonSuccessworks but could be more explicit. Consider using{} as anyor defining a minimal mock config object for better readability.🔎 Alternative approach
- 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);Or define a reusable mock config:
const mockSWRConfig = {} as SWRConfiguration; // Then use: config.onSuccess(..., key as string, mockSWRConfig);Also applies to: 77-77, 108-108
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
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/hooks/__tests__/usePermissions.test.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{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/hooks/__tests__/usePermissions.test.tsapps/web/src/app/api/ai/chat/messages/[messageId]/undo/__tests__/route.test.tsapps/web/src/app/api/ai/chat/messages/[messageId]/undo/route.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/hooks/__tests__/usePermissions.test.tsapps/web/src/app/api/ai/chat/messages/[messageId]/undo/__tests__/route.test.tsapps/web/src/app/api/ai/chat/messages/[messageId]/undo/route.ts
**/*.{ts,tsx,js,jsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Format code with Prettier
Files:
apps/web/src/hooks/__tests__/usePermissions.test.tsapps/web/src/app/api/ai/chat/messages/[messageId]/undo/__tests__/route.test.tsapps/web/src/app/api/ai/chat/messages/[messageId]/undo/route.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/hooks/__tests__/usePermissions.test.tsapps/web/src/app/api/ai/chat/messages/[messageId]/undo/__tests__/route.test.tsapps/web/src/app/api/ai/chat/messages/[messageId]/undo/route.ts
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
🧠 Learnings (6)
📚 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-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.tsapps/web/src/app/api/ai/chat/messages/[messageId]/undo/route.ts
📚 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 app/**/*.{ts,tsx} : Import and use `getUserAccessLevel()` and `canUserEditPage()` from `pagespace/lib/permissions` for centralized permission logic
Applied to files:
apps/web/src/hooks/__tests__/usePermissions.test.tsapps/web/src/app/api/ai/chat/messages/[messageId]/undo/route.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-14T14:54:38.009Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T14:54:38.009Z
Learning: Applies to apps/**/*.ts : Always use centralized permission functions from `pagespace/lib/permissions` for access control logic (e.g., `getUserAccessLevel`, `canUserEditPage`)
Applied to files:
apps/web/src/hooks/__tests__/usePermissions.test.tsapps/web/src/app/api/ai/chat/messages/[messageId]/undo/route.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/app/**/route.{ts,tsx} : For permission logic, use centralized functions from `pagespace/lib/permissions`: `getUserAccessLevel()`, `canUserEditPage()`
Applied to files:
apps/web/src/hooks/__tests__/usePermissions.test.tsapps/web/src/app/api/ai/chat/messages/[messageId]/undo/route.ts
🧬 Code graph analysis (2)
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 (3)
apps/web/src/services/api/ai-undo-service.ts (1)
executeAiUndo(254-368)apps/web/src/services/api/index.ts (1)
executeAiUndo(61-61)apps/web/src/app/api/ai/chat/messages/[messageId]/undo/route.ts (1)
POST(112-186)
⏰ 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 (5)
apps/web/src/hooks/__tests__/usePermissions.test.ts (3)
37-67: LGTM! Test correctly simulates initial load and verifies isPaused behavior.The mock implementation properly triggers the
onSuccesscallback to sethasLoadedRef.current = true, then correctly verifies thatisPaused()returnstruewhen editing is active. This aligns with the learned pattern for SWR editing protection.
69-97: LGTM! Correct verification of isPaused during AI streaming.The test correctly simulates the initial load completion and verifies that permission revalidation is paused when AI streaming is active, matching the expected behavior from the hook implementation.
99-127: LGTM! Correctly validates revalidation when no editing activity.The test properly simulates the loaded state and verifies that
isPaused()returnsfalsewhen no editing or streaming is active. The comment on line 121 accurately describes the logic:hasLoadedRef.current=true && isAnyActive=false => false.apps/web/src/app/api/ai/chat/messages/[messageId]/undo/route.ts (1)
159-168: LGTM! Critical logic gap fixed.The simplified failure condition now correctly catches all failure cases, including when
success: falsewith an emptyerrorsarray. This prevents failures from falling through to the success response path and aligns with the all-or-nothing transaction semantics inexecuteAiUndo.The standardized failure response (
message: 'Undo failed. No changes were applied.',status: 500) provides a clear, consistent API contract.apps/web/src/app/api/ai/chat/messages/[messageId]/undo/__tests__/route.test.ts (1)
349-381: LGTM! Comprehensive failure test coverage.The test suite now correctly validates both failure scenarios:
- Failures with error details (
errors: ['Complete failure'])- Failures with empty errors array (defensive edge case)
The mock values (
messagesDeleted: 0,activitiesRolledBack: 0) accurately reflect the all-or-nothing transaction behavior inexecuteAiUndo, where any failure aborts the entire operation before counters are incremented.Both tests verify the standardized failure response:
status: 500andmessage: 'Undo failed. No changes were applied.'
* feat: add version history and rollback functionality 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> * fix: complete rollback handler implementations and fix lint errors - 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> * feat: add undo AI changes feature for conversations 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> * fix: add missing radio-group UI component Required for UndoAiChangesDialog mode selection. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: add @radix-ui/react-radio-group dependency 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> * fix: restore correct @electron/node-gyp resolution in lockfile 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> * fix: add critical rules to CLAUDE.md and fix retention tier order - 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> * fix(db): add missing default and rollback index - Add DEFAULT now() for retention_policies.updatedAt column - Add index on activity_logs.rollbackFromActivityId for queries - Create migration 0027_fix_retention_updated_at.sql Addresses PR #118 review issues #10, #20 * fix(lib/permissions): add block scoping and eligibility helper - 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> * fix(lib/monitoring): add rollback fields to ActivityLogInput - 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> * fix(api): add authorization and resourceType filtering - 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> * fix(services): improve type safety and add transactions - 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> * fix(ui): add CSRF protection and improve error handling - 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> * fix(db): address schema issues from PR #118 review 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> * fix(lib): move rollback fields to top-level in activity logger 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> * fix(services): add transaction support to rollback service 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> * fix(services): improve atomicity and context logic in AI undo 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> * fix(api): use Zod schema for undo route validation 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> * fix: address CodeRabbit review feedback - 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> * fix(db): add subscriptionTier enum for retention policies - 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> * fix: address final CodeRabbit review comments - 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> * fix(db): add USING clause for text-to-enum cast 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> * fix(ai): support undo for global assistant messages by checking both messages tables and updating permission logic * fix(ai): resolve lint errors in undo route and tests * test: add contract tests for version history rollback - 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> * feat(ui): add rollback capability to activity views 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> * test: add activity logger tests for rollback and undo operations 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> * fix(rollback): support create operation rollback and global messages - 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: address PR #118 code review feedback - 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> * test: update rollback-permissions tests for create operation 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> * fix: reset undo counters on transaction failure 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> * fix: remove unused params variable in OptimizedViewHeader 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix(web): refine AI undo error message to reflect atomic transaction semantics * fix(tests): correct hook test expectations for SWR behavior - 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> * fix: handle all failure cases in AI undo route - 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> * fix: resolve AI chat undo feature not working 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> --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
1 participant
Summary
Add comprehensive version history browsing and rollback capabilities for PageSpace, allowing users to restore resources to previous states based on the existing activity logging infrastructure.
Key Features
Changes
Schema (
packages/db/)rollbackoperation to activity enumrollbackFromActivityId,contentFormatfields to activity_logsretention_policiestableBackend (
packages/lib/,apps/web/src/services/)logRollbackActivity()functionAPI Endpoints (
apps/web/src/app/api/)GET /api/activities/[activityId]- Single activity with rollback eligibilityPOST /api/activities/[activityId]/rollback- Execute rollbackGET /api/pages/[pageId]/history- Page version historyGET /api/drives/[driveId]/history- Drive version history (admin)UI Components (
apps/web/src/components/version-history/)VersionHistoryPanel- Main slide-out panelVersionHistoryItem- Activity item with "Restore" actionRollbackConfirmDialog- Confirmation modalTest plan
pnpm db:migrateto apply schema changesGET /api/pages/{id}/history)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.