Skip to content

Sheets - #3

Closed
2witstudios wants to merge 8 commits into
masterfrom
Sheets
Closed

2witstudios wants to merge 8 commits into
masterfrom
Sheets

Conversation

@2witstudios

Copy link
Copy Markdown
Owner

No description provided.

@2witstudios
2witstudios deleted the Sheets branch October 17, 2025 19:57
2witstudios added a commit that referenced this pull request Dec 22, 2025
- 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>
2witstudios added a commit that referenced this pull request Dec 23, 2025
* 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>
2witstudios added a commit that referenced this pull request Dec 23, 2025
* 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>

---------

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
2witstudios added a commit that referenced this pull request Dec 24, 2025
* 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>
2witstudios added a commit that referenced this pull request May 4, 2026
…adcast

Addresses 3 Codex P1/P2 review comments.

P1 #1: PATCH /events/[id] now wraps event update + agent-trigger upsert in a single db.transaction so a failed trigger upsert rolls back the event update too — no more partial-write where the title moved but the trigger didn't. update_calendar_event AI tool gets the same atomic shape. New helper upsertCalendarTriggerWorkflowInTx runs against the caller's tx; the standalone upsertCalendarTriggerWorkflow continues to open its own tx for PUT /triggers.

P1 #2: PUT /events/[id]/triggers and PATCH /events/[id] now reject agent-trigger upserts on recurring events, mirroring POST /events. Cron poller fires one-shot occurrences, so attaching one trigger to a recurring event silently misfired on every occurrence past the first.

P2 #3: PUT/DELETE /triggers broadcast now includes the event's attendee user IDs so personal-calendar clients (which only join their own user channel) get immediate trigger-state updates instead of waiting for periodic polling.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2witstudios added a commit that referenced this pull request May 4, 2026
* feat(calendar-triggers): validation + upsert + remove

Adds validateCalendarAgentTrigger, upsertCalendarTriggerWorkflow, and removeCalendarTrigger to mirror the task-trigger-helpers surface so the REST + AI tool calendar paths share one validator and the new triggers endpoints have a clean upsert/delete primitive.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(calendar): wire agent trigger context on POST

POST /api/calendar/events now accepts agentTrigger.instructionPageId and contextPageIds and forwards them to createCalendarTriggerWorkflow. Routes through validateCalendarAgentTrigger so the REST path enforces the same drive/agent checks as the AI tool. Loosens prompt to optional when an instructionPageId is set.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(calendar): PATCH accepts agentTrigger upsert+remove

PATCH /api/calendar/events/[eventId] now accepts agentTrigger with three semantics: undefined leaves any existing trigger alone, null drops the trigger, an object upserts via upsertCalendarTriggerWorkflow. Personal events (no driveId) reject upserts because the executor needs a drive context to resolve agent/instruction/context pages.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(calendar): /events/:id/triggers REST endpoints

GET returns the event's trigger row joined with the latest workflow_runs status (lastFiredAt / lastFireError / lastRunStatus). PUT upserts via upsertCalendarTriggerWorkflow. DELETE removes via removeCalendarTrigger. Permission check mirrors PATCH on the event: creator or drive owner/admin.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* refactor(triggers): extract AgentTriggerSection

Pulls the agent / prompt / instruction-page / context-pages form out of TaskAgentTriggersDialog into a shared presentational component so the new EventAgentTriggerDialog can reuse the same UX. Behavior unchanged for tasks.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(calendar-ui): EventAgentTriggerDialog

New dialog mirrors TaskAgentTriggersDialog UX for calendar events: toggle, agent select, prompt, instruction page, context pages (max 10), last-run status, save / remove. Hooks the new /api/calendar/events/[eventId]/triggers endpoints. Pauses SWR via useEditingSession so a remote calendar broadcast cannot clobber unsaved typing.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(calendar-ui): linked page + agent trigger button

EventModal now exposes the existing calendarEvents.pageId column as a 'Linked page' picker (drive context, both create and edit) so events can carry a doc the same way task items do via taskItems.pageId. Replaces the inline agent toggle with a 'Configure agent trigger' button that opens EventAgentTriggerDialog on saved drive events. Drops the !isEditing constraint that previously blocked managing triggers on existing events.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(calendar): drop unused imports flagged by lint

CI lint surfaced three unused symbols left over from the inline-validation removal: pages from route.ts (validation moved into validateCalendarAgentTrigger), and mockSelectFrom / mockUpdateWhere from the trigger-helpers test. The test mocks are still wired into the chain (.from / .where) but no longer need to be re-exported from the hoist block. Unblocks Lint, Unit Tests, and Static Security CI.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(calendar): agentTrigger on update AI tool + safer PATCH

PATCH /api/calendar/events/[eventId] now pre-validates agentTrigger before opening the event update tx so a bad payload (off-drive agent / context page, missing prompt-or-instruction) returns 400 without dirtying event state. update_calendar_event AI tool gains an agentTrigger field with the same three-state semantics (undefined no-op, null remove, object upsert) as the HTTP PATCH, closing the AI-vs-HTTP parity gap. Three new tests cover the AI path.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(calendar): atomic event+trigger tx, recurring guard, attendee broadcast

Addresses 3 Codex P1/P2 review comments.

P1 #1: PATCH /events/[id] now wraps event update + agent-trigger upsert in a single db.transaction so a failed trigger upsert rolls back the event update too — no more partial-write where the title moved but the trigger didn't. update_calendar_event AI tool gets the same atomic shape. New helper upsertCalendarTriggerWorkflowInTx runs against the caller's tx; the standalone upsertCalendarTriggerWorkflow continues to open its own tx for PUT /triggers.

P1 #2: PUT /events/[id]/triggers and PATCH /events/[id] now reject agent-trigger upserts on recurring events, mirroring POST /events. Cron poller fires one-shot occurrences, so attaching one trigger to a recurring event silently misfired on every occurrence past the first.

P2 #3: PUT/DELETE /triggers broadcast now includes the event's attendee user IDs so personal-calendar clients (which only join their own user channel) get immediate trigger-state updates instead of waiting for periodic polling.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* test(calendar): cover validator-throws PATCH path

New test confirms PATCH /events/[id] returns 400 cleanly when validateCalendarAgentTrigger throws (e.g. off-drive agent), without ever opening the event update transaction. Pins the partial-write protection added in the previous commit.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2witstudios added a commit that referenced this pull request May 15, 2026
* feat(calendar-triggers): validation + upsert + remove

Adds validateCalendarAgentTrigger, upsertCalendarTriggerWorkflow, and removeCalendarTrigger to mirror the task-trigger-helpers surface so the REST + AI tool calendar paths share one validator and the new triggers endpoints have a clean upsert/delete primitive.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(calendar): wire agent trigger context on POST

POST /api/calendar/events now accepts agentTrigger.instructionPageId and contextPageIds and forwards them to createCalendarTriggerWorkflow. Routes through validateCalendarAgentTrigger so the REST path enforces the same drive/agent checks as the AI tool. Loosens prompt to optional when an instructionPageId is set.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(calendar): PATCH accepts agentTrigger upsert+remove

PATCH /api/calendar/events/[eventId] now accepts agentTrigger with three semantics: undefined leaves any existing trigger alone, null drops the trigger, an object upserts via upsertCalendarTriggerWorkflow. Personal events (no driveId) reject upserts because the executor needs a drive context to resolve agent/instruction/context pages.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(calendar): /events/:id/triggers REST endpoints

GET returns the event's trigger row joined with the latest workflow_runs status (lastFiredAt / lastFireError / lastRunStatus). PUT upserts via upsertCalendarTriggerWorkflow. DELETE removes via removeCalendarTrigger. Permission check mirrors PATCH on the event: creator or drive owner/admin.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* refactor(triggers): extract AgentTriggerSection

Pulls the agent / prompt / instruction-page / context-pages form out of TaskAgentTriggersDialog into a shared presentational component so the new EventAgentTriggerDialog can reuse the same UX. Behavior unchanged for tasks.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(calendar-ui): EventAgentTriggerDialog

New dialog mirrors TaskAgentTriggersDialog UX for calendar events: toggle, agent select, prompt, instruction page, context pages (max 10), last-run status, save / remove. Hooks the new /api/calendar/events/[eventId]/triggers endpoints. Pauses SWR via useEditingSession so a remote calendar broadcast cannot clobber unsaved typing.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(calendar-ui): linked page + agent trigger button

EventModal now exposes the existing calendarEvents.pageId column as a 'Linked page' picker (drive context, both create and edit) so events can carry a doc the same way task items do via taskItems.pageId. Replaces the inline agent toggle with a 'Configure agent trigger' button that opens EventAgentTriggerDialog on saved drive events. Drops the !isEditing constraint that previously blocked managing triggers on existing events.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(calendar): drop unused imports flagged by lint

CI lint surfaced three unused symbols left over from the inline-validation removal: pages from route.ts (validation moved into validateCalendarAgentTrigger), and mockSelectFrom / mockUpdateWhere from the trigger-helpers test. The test mocks are still wired into the chain (.from / .where) but no longer need to be re-exported from the hoist block. Unblocks Lint, Unit Tests, and Static Security CI.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(calendar): agentTrigger on update AI tool + safer PATCH

PATCH /api/calendar/events/[eventId] now pre-validates agentTrigger before opening the event update tx so a bad payload (off-drive agent / context page, missing prompt-or-instruction) returns 400 without dirtying event state. update_calendar_event AI tool gains an agentTrigger field with the same three-state semantics (undefined no-op, null remove, object upsert) as the HTTP PATCH, closing the AI-vs-HTTP parity gap. Three new tests cover the AI path.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(calendar): atomic event+trigger tx, recurring guard, attendee broadcast

Addresses 3 Codex P1/P2 review comments.

P1 #1: PATCH /events/[id] now wraps event update + agent-trigger upsert in a single db.transaction so a failed trigger upsert rolls back the event update too — no more partial-write where the title moved but the trigger didn't. update_calendar_event AI tool gets the same atomic shape. New helper upsertCalendarTriggerWorkflowInTx runs against the caller's tx; the standalone upsertCalendarTriggerWorkflow continues to open its own tx for PUT /triggers.

P1 #2: PUT /events/[id]/triggers and PATCH /events/[id] now reject agent-trigger upserts on recurring events, mirroring POST /events. Cron poller fires one-shot occurrences, so attaching one trigger to a recurring event silently misfired on every occurrence past the first.

P2 #3: PUT/DELETE /triggers broadcast now includes the event's attendee user IDs so personal-calendar clients (which only join their own user channel) get immediate trigger-state updates instead of waiting for periodic polling.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* test(calendar): cover validator-throws PATCH path

New test confirms PATCH /events/[id] returns 400 cleanly when validateCalendarAgentTrigger throws (e.g. off-drive agent), without ever opening the event update transaction. Pins the partial-write protection added in the previous commit.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2witstudios added a commit that referenced this pull request May 29, 2026
…t privilege)

Addresses Codex's fourth P1. The previous fix preserved custom-role agent
memberships to avoid widening (#3), but that left them with stale custom-role
access: getAgentAccessLevel reads driveAgentMembers.customRoleId directly, so an
agent keeps reaching pages the downgraded granter can no longer grant.

Since the safe least-privilege intersection ("view-only on a custom role's
pages") can't be represented with the role/customRoleId fields, recap now, for
each non-home membership the user granted (granter capped to MEMBER):
  - ADMIN role  → reduce to the granter's ceiling (their custom role, or plain
    MEMBER view-only) — always strictly narrower than full access.
  - already at the ceiling → leave unchanged.
  - any other member-level grant whose customRoleId differs from the granter's
    → REVOKE. Removing the membership is the only representable non-widening
    reduction, and it eliminates the stale access.

This satisfies both "never widen" (#3) and "no stale custom-role access" (#4).
Updated the service tests: reduce ADMIN, revoke mismatched custom-role grants,
preserve matching ones, no-op when nothing was granted.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2witstudios added a commit that referenced this pull request May 30, 2026
…wngraded (#1458)

* feat(agents): cascade agent revocation when a member is removed or downgraded

An external agent's drive access is frozen into driveAgentMembers.role at
grant time and read straight back at runtime (getAgentAccessLevel); it is
never re-derived from the granting user's live permissions. So removing a
user from a drive left the agents they had added still able to read it —
for the revoked user and anyone who could run those agents — and downgrading
a user left an agent they had elevated to ADMIN still at ADMIN.

Make agent memberships follow the granting user's drive access, keyed on
driveAgentMembers.addedBy and scoped to the drive (home-drive memberships
are preserved):

- revokeAgentMembershipsGrantedBy: deletes the user's non-home agent
  memberships; accepts a transaction so it runs atomically with the member
  removal in the DELETE handler.
- recapAgentMembershipsGrantedBy: reuses resolveGranterAccess to recompute
  the user's current grant ceiling and downgrades any elevated agents they
  granted to plain MEMBER (no-op on upgrade), invoked from the PATCH handler
  on a role change.

Both cascades emit authz audit events (reason: member_removal /
member_downgrade). No realtime broadcast — consistent with the existing
agent-membership endpoints, which audit only.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(agents): re-cap agents on custom-role change, not only standard-role change

Addresses Codex P1: the PATCH re-cap cascade ran only inside the
`role !== oldRole` block, so a member who stayed MEMBER but moved to a more
restrictive custom role (or had it cleared) left their granted agents with a
stale driveAgentMembers.customRoleId — access to pages the user could no
longer grant.

Compute the old custom role from the pre-update member details and trigger
recapAgentMembershipsGrantedBy on `roleChanged || customRoleChanged`. The
re-cap re-syncs each agent the member granted to the grant they could make
today (resolveGranterAccess), which never escalates beyond the member's own
access. Audit reason renamed member_downgrade -> member_recap to cover both
triggers.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(agents): detect custom-role clears via updateMemberRole's returned old value

Addresses Codex's second P1: the re-cap derived the previous custom role from
memberData.customRole, but getDriveMemberDetails hardcodes `customRole: null`
(and the MemberWithDetails type doesn't expose customRoleId). So
`customRoleChanged` was always false for a custom-role CLEAR
(customRoleId: null over a prior role with the standard role unchanged),
skipping recapAgentMembershipsGrantedBy and leaving agents with stale access.

Make updateMemberRole return the old customRoleId it already reads from the
row (`{ oldRole, oldCustomRoleId }`) and compute `customRoleChanged` from that
reliable value. Adds a regression test for the clear case and switches the
"equals current" / "custom-role change" tests to drive the old value through
updateMemberRole's return instead of the always-null getDriveMemberDetails.customRole.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(agents): recap must not widen restrictive custom-role agents

Addresses Codex's third P1: recapAgentMembershipsGrantedBy reset every elevated
membership (including custom-role ones) to plain MEMBER when the granter dropped
to plain MEMBER. But in the agent permission resolver a plain MEMBER can view
EVERY page in the drive, whereas a custom role grants only its listed pages — so
rewriting a restrictive custom-role agent to plain MEMBER broadened it from a
subset to the whole drive. A downgrade-driven recap must only ever cap DOWN.

Restrict recap to ADMIN agent memberships, which are unambiguously above any
member-level ceiling (full access to all pages), so capping them to the
granter's ceiling (their custom role, or plain MEMBER view-only) is always
strictly narrower. Custom-role memberships are now preserved untouched — the
role/customRoleId fields can't express the safe least-privilege intersection,
and preserving them guarantees recap never widens an agent's page reach.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(agents): revoke stale custom-role agent grants on downgrade (least privilege)

Addresses Codex's fourth P1. The previous fix preserved custom-role agent
memberships to avoid widening (#3), but that left them with stale custom-role
access: getAgentAccessLevel reads driveAgentMembers.customRoleId directly, so an
agent keeps reaching pages the downgraded granter can no longer grant.

Since the safe least-privilege intersection ("view-only on a custom role's
pages") can't be represented with the role/customRoleId fields, recap now, for
each non-home membership the user granted (granter capped to MEMBER):
  - ADMIN role  → reduce to the granter's ceiling (their custom role, or plain
    MEMBER view-only) — always strictly narrower than full access.
  - already at the ceiling → leave unchanged.
  - any other member-level grant whose customRoleId differs from the granter's
    → REVOKE. Removing the membership is the only representable non-widening
    reduction, and it eliminates the stale access.

This satisfies both "never widen" (#3) and "no stale custom-role access" (#4).
Updated the service tests: reduce ADMIN, revoke mismatched custom-role grants,
preserve matching ones, no-op when nothing was granted.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(agents): revoke ADMIN agents on downgrade to plain MEMBER (private-page leak)

Addresses Codex's fifth P1. resolveRolePermissions gives a plain MEMBER
membership canView:true on EVERY page (no isPrivate check), while a plain MEMBER
*user* only sees non-private pages. So reducing an ADMIN agent to
{ role: MEMBER, customRoleId: null } let the agent keep reading private pages the
downgraded (plain-member) granter can no longer access — the plain-MEMBER row
cannot express "non-private only".

recap now only *reduces* an ADMIN agent when the granter still holds a custom
role (a representable, strictly-narrower per-page matrix). When the granter is a
plain MEMBER, ADMIN agents are REVOKED instead. Pre-existing plain-MEMBER agent
memberships (already exactly what a plain-member granter yields via
addAgentToDrive) are left untouched. Updated/added service tests for: ADMIN +
plain-member granter → revoke; ADMIN + custom-role granter → reduce to that role;
plain-MEMBER agent preserved.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(permissions): bottleneck agent access by granting user's access

Root-cause fix for the divergence where a plain-MEMBER *agent* membership
read every page in a drive — including private ones — while a plain-MEMBER
*user* only sees non-private pages. An agent could out-read the member who
granted it.

- agent-permissions.ts: gate the plain-MEMBER path on isPrivate, mirroring
  the user-side membership rule (permissions.ts). getAgentAccessLevel denies
  a plain member on a private page; getAgentAccessiblePagesInDrive filters
  private pages out. ADMIN/OWNER and explicit custom-role grants keep their
  existing parity with the user-side paths.
- drive-agent-service.ts: with the over-grant closed, recapAgentMembershipsGrantedBy
  simplifies to always reduce a downgraded granter's agents to the representable
  cap (role MEMBER + the granter's own custom role / none). The revoke-instead-
  of-reduce special case is gone — a plain-MEMBER row can no longer leak private
  pages, so reduction is always safe.
- Tests: add private-page coverage to agent-permissions; update recap tests to
  assert reduce (not revoke) under a plain-member granter.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(agents): revoke (not rewrite) mismatched custom-role agents on recap

CodeRabbit caught that unconditionally rewriting every mismatched membership
to the granter's new cap can WIDEN an agent on downgrade: an agent scoped by a
restrictive custom role rewritten to plain MEMBER/null would gain visibility to
all non-private pages in the drive. A custom-role matrix is incomparable to the
cap (it may cover fewer pages, or grant private/edit access the cap doesn't).

recapAgentMembershipsGrantedBy now:
- role ADMIN          ⇒ reduce to the cap (ADMIN ⊇ any MEMBER cap ⇒ pure narrow).
- role MEMBER == cap  ⇒ leave.
- role MEMBER != cap  ⇒ REVOKE (not provably a subset ⇒ never rewrite/widen).

The isPrivate fix keeps the common ADMIN→plain-MEMBER case a clean reduction;
only the genuinely incomparable custom-role mismatch is revoked. Tests updated:
mismatched custom role and plain-MEMBER-under-custom-cap both revoke.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(agents): subset-check custom roles on recap (no false revoke on upgrade)

Codex (P2) flagged that revoking every mismatched custom-role membership also
fires on an *upgrade*: moving a member role_a → role_b (broader) would delete
agents granted under role_a even though the user's access wasn't reduced. But an
earlier finding requires recap to still cascade on a custom-role *tightening*.

Reconcile both by comparing the role permission matrices in
recapAgentMembershipsGrantedBy:
- role ADMIN          ⇒ reduce to the cap (pure narrowing).
- role MEMBER == cap  ⇒ leave.
- role MEMBER != cap  ⇒ keep iff the agent's role is provably a subset of the
  granter's new role (memberGrantWithinCap / customRoleSubset); else revoke.

So a broadening change leaves the agent (still within the granter), while a
tightening or otherwise-incomparable change still revokes. Plain-MEMBER (null)
on either side is treated conservatively (fail-closed revoke), since it spans the
whole drive and isn't a per-page matrix. Never rewrites a member row, so it can't
widen (the CodeRabbit concern stays addressed). Trigger unchanged. Added subset
leave/revoke unit tests.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2witstudios added a commit that referenced this pull request Jul 3, 2026
…429 handling (#1763)

Two P2 findings from the automated review on PR #1756 (merged 69ecab2),
verified against source and both real:

1. Sec 1.4 mischaracterized the device-token grace window as delivering the
   replacement token to a concurrent caller. Verified against
   auth-transactions.ts:147-174 and refresh/route.ts:135-137: the grace
   branch returns only replacement metadata, never the new token value — a
   caller whose first response was genuinely lost has no recovery path.
   Sec 3.4 now specifies an explicit ephemeral grace-cache design (30s TTL,
   keyed by old-token hash) so the OAuth flow actually recovers a dropped
   rotation response instead of inheriting that gap, plus a distinct
   grace_cache_miss outcome (infra gap, not theft — no family revocation).

2. classifyRefreshFailure() treated all 4xx as purge-and-reauth, which would
   delete a valid stored credential on a 429 rate-limit response from the
   /token endpoint (F11). 429 now routes through the retryable path with
   the rest of F4, honoring Retry-After.

Updates flow through Sec 3.2/3.4, the F2a/F4/F5 fail-closed ledger rows,
the decideRefresh/classifyRefreshFailure signatures in Sec 6, and the
Sec 7 testable assertions (#3, #10).


Claude-Session: https://claude.ai/code/session_019ovsuAwugHKEdMbd2wyQAk

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
2witstudios added a commit that referenced this pull request Jul 9, 2026
…d, dedupe nonce read

Findings from a proactive multi-angle code-review pass (4 parallel finder
agents) on this branch's own diff. Two confirmed correctness/security bugs,
one empirically-verified non-issue, one empirically-verified false positive,
plus two cleanup items.

1. CONFIRMED — stampScriptNonce() could corrupt unrelated attribute values
   (packages/lib/src/canvas/render-document.ts). The nonce-detection regex
   searched for the raw substring " nonce=" anywhere in the tag, which also
   matched inside a DIFFERENT attribute's own quoted value (e.g.
   `<script data-log="utm_source=x nonce=stale123">`), truncating and
   re-quoting that attribute mid-string — malformed HTML. Rewrote as a real
   attribute tokenizer that consumes one whole attribute (name + value) per
   step, so a value's contents are never re-scanned as if they were tag
   syntax. This also correctly handles a bare/valueless `nonce` attribute
   (previously not detected, would have produced a duplicate attribute).
   New tests cover both cases; all 95 render-document tests pass.

2. CONFIRMED (security) — the connect-src/media-src storage-host wildcard
   (`*.<host>`) was scoped one DNS label too high. For a shared multi-tenant
   S3-compatible host (Tigris, AWS S3), that wildcard permits connect-src to
   ANY other tenant's bucket on the same provider domain — self-serve
   provisionable by an attacker — not just this app's own bucket. Since the
   bucket name is already known (mirrors presigned-url.ts's getS3Bucket()
   env-var precedence — duplicated, not imported, because that module pulls
   in the Node-only AWS SDK which is unsafe to import into this Edge-runtime
   middleware module), narrowed to `<bucket>.<host>` — the exact address
   virtual-hosted-style addressing actually produces, with zero widening
   beyond this app's own storage. New tests assert the wildcard is gone and
   that a configured BUCKET_NAME is respected.

3. REFUTED (verified against Next.js docs) — a reviewer flagged that
   dashboard/layout.tsx reads the CSP nonce via headers() without also
   calling connection(), unlike the root layout, and asked whether that
   risks a stale/cached nonce. Per Next.js's own connection() docs:
   "the function is only necessary when dynamic rendering is required and
   common Request-time APIs are not used" — headers() is itself a
   Request-time API, so it already guarantees per-request freshness on its
   own. No fix needed; documented for the record.

4. EMPIRICALLY VERIFIED (non-issue) — a reviewer flagged that Shiki's JS
   regex engine (switched from the WASM oniguruma engine to avoid needing
   'wasm-unsafe-eval' in script-src) might silently fail to highlight some
   TextMate grammars the app offers beyond the two languages the original
   tests covered (javascript, sudolang). Added
   shiki-js-engine-coverage.test.ts, which tokenizes a real snippet in
   EVERY language LanguageSelector.tsx actually offers (typescript, python,
   html, css, json, markdown, bash, sql, rust, go, yaml) and asserts each
   produces genuine multi-color highlighting, not a silent plain-text
   fallback. All 12 pass — kept as a permanent regression guard.

5. Cleanup — buildStorageConnectSrcEntries() was called twice per
   buildCSPPolicy() invocation (once for connect-src, once for media-src),
   redundantly re-parsing AWS_ENDPOINT_URL_S3 both times; hoisted to a
   single local computed once and spread into both directives.

6. Cleanup — the "read the per-request nonce from headers()" snippet was
   duplicated verbatim between app/layout.tsx and app/dashboard/layout.tsx.
   Extracted to a shared apps/web/src/lib/request-nonce.ts (getRequestNonce()),
   used by both. Root layout's connection() call is untouched (redundant
   per finding #3, but not this PR's place to remove pre-existing, working,
   intentionally-commented code).

Documented but NOT fixed in this PR (out of scope, distinct trade-off):
inline event-handler attributes (onclick, onload, etc.) in canvas author
HTML remain blocked by the inherited outer CSP. Per CSP spec, a nonce
cannot authorize script-src-attr (event-handler) execution — only
<script> elements. The only way to restore this would be loosening
script-src-attr on the app-wide dashboard CSP, which (unlike the narrow,
already-permitted-by-canvas's-own-CSP nonce fix) would grant a genuinely
new capability across the ENTIRE app, not just the isolated canvas iframe —
a materially different security trade-off that shouldn't be made as a
drive-by fix. Flagging as a known follow-up.

All affected suites verified green: render-document.test.ts (95/95),
security-headers.test.ts (65/65), CanvasFrame.test.ts (4/4),
render-published.test.ts (19/19), all code-block/shiki suites (137/137
combined), apps/web tsc --noEmit clean, @pagespace/lib typecheck clean,
full `next build` clean.
2witstudios added a commit that referenced this pull request Jul 9, 2026
… uploads, media, syntax highlighting) (#1959)

* fix(canvas): stamp CSP nonce onto author scripts for srcDoc inheritance

A srcDoc iframe unconditionally inherits its parent (embedder) document's
CSP in addition to its own <meta> CSP, regardless of the sandbox attribute.
The middleware CSP hardening work added a nonce-based script-src to all
dashboard page responses, which is now inherited by CanvasFrame's srcDoc
iframe. Since canvas's own baseline CSP grants script-src 'unsafe-inline'
with no nonce, author <script> tags in the inherited outer policy were
being blocked — a regression from that hardening work, not a pre-existing
canvas bug.

Fix: thread the app's per-request CSP nonce (already generated by
middleware, read in the root layout) down to CanvasFrame via a new
NonceContext, and stamp it onto preserved author <script> tags in
renderCanvasDocument(). This grants no new capability — canvas's own CSP
already allowed unrestricted inline scripts, isolation is by the sandboxed
iframe's opaque origin, not CSP — it just stops the newly-inherited outer
policy from taking away a capability canvas already had.

The publish pipeline (renderPublishedPage) never passes a nonce and is
untouched: published pages are served standalone, never framed via srcDoc,
so they never inherited an outer CSP in the first place.

Verified in a real Chromium engine (Playwright) that reproduces the exact
inheritance behavior: an unnonced script in the srcDoc document is blocked
with a CSP violation when the outer document has a nonce-based policy, and
with the nonce stamped the script runs with zero violations.

* fix(csp): allowlist storage host in connect-src/media-src, use JS-only Shiki engine

Two more regressions from the middleware CSP hardening, same root-cause
family as the canvas srcDoc nonce-inheritance fix in this branch: the
hardening work wasn't audited against direct-to-storage traffic or the
editor's WASM-based syntax highlighter.

1. Direct-to-storage uploads/downloads (connect-src): the browser calls the
   configured S3-compatible storage endpoint directly for presigned
   uploads/downloads (uploadToTigris in upload/orchestrator.ts and
   attachment-client.ts; the direct fetch(presignedUrl) download in
   content-header/index.tsx). connect-src never allowlisted that host, so
   those requests were blocked outright. Fixed by deriving both the literal
   and wildcard-subdomain form of the endpoint host from AWS_ENDPOINT_URL_S3
   at CSP-build time (buildStorageConnectSrcEntries()) — never hardcoded,
   since onprem/tenant deployments may point this at a different
   S3-compatible endpoint. Added unconditionally (not gated on IS_CLOUD):
   uploads are core functionality in every deployment mode.

2. Video/audio playback (media-src): there was no media-src directive at
   all, so <video>/<audio> loading fell back fully to default-src 'self'.
   /api/files/[id]/view 307-redirects a <video> element's default media
   request straight to the presigned storage URL, and CSP fetch directives
   are re-evaluated against the final redirected URL — so it hit the same
   missing-host problem as connect-src. Fixed by adding media-src reusing
   the identical buildStorageConnectSrcEntries() host list.

3. Shiki syntax highlighting (script-src): TipTap's code-block highlighter
   used Shiki's default oniguruma engine, which instantiates a WebAssembly
   module client-side — CSP Level 3 requires 'wasm-unsafe-eval' in
   script-src for that, which the hardened policy doesn't grant (correctly;
   broadening script-src for one feature would cut against the point of the
   hardening). Rather than loosen the CSP, switched to Shiki's pure-JS regex
   engine (createJavaScriptRegexEngine from shiki/engine/javascript), which
   needs no eval-gated capability at all. Failed silently before (try/catch
   in CodeBlockShikiExtension.ts skips highlighting on error) rather than
   crashing, but code blocks lost syntax highlighting.

Also audited and confirmed clean, no action needed: voice/TTS playback
(pure Web Audio API buffer playback, no <audio> element/URL), the PDF
viewer (fetch() into ArrayBuffer, covered by the connect-src fix), Stripe
checkout/billing (hosted-redirect and embedded PaymentElement), and Monaco
(worker loading, theming).

Tests: apps/web/src/middleware/__tests__/security-headers.test.ts gains
connect-src and media-src coverage (configured host allowlisted in both
forms; unset env var produces no malformed entry; malformed URL value
doesn't throw) — 62/62 pass. Shiki-related suites (shiki-highlighter,
code-block-shiki, sudolang-grammar, token-decorations) all pass unchanged
with the JS engine — no fixture adjustments needed.

* fix(canvas): always replace stale author nonces, fix data-nonce false-positive

Addresses two review findings on stampScriptNonce() (packages/lib/src/canvas/render-document.ts):

1. Codex: when an author script already declares its own `nonce` attribute
   (e.g. HTML pasted from a different nonce-protected site), the previous
   behavior left it alone. But a foreign/stale nonce can never match the
   inherited outer CSP's nonce-source (browsers require exact string
   matching), so leaving it in place still got the script blocked — the
   exact bug this function exists to fix. Now the existing nonce value is
   REPLACED with the current per-request nonce instead of left alone. This
   is strictly better than skipping: still exactly one `nonce` attribute
   (valid HTML, no duplication), and now it actually matches the policy that
   will be enforced.

2. CodeRabbit: the nonce-presence check used a bare `\b` word-boundary
   (`/\bnonce\s*=/i`), which also matches right after a hyphen — so
   `data-nonce="foo"` or `aria-nonce="foo"` was wrongly treated as "already
   has a nonce" and the script was skipped entirely, left with no real
   nonce attribute at all. The new check requires nonce to be preceded by
   whitespace (a real, standalone attribute), so data-nonce/aria-nonce are
   never mistaken for it and a real nonce attribute always gets added.

Updated the existing "don't duplicate/overwrite" test to assert the new
(correct) replace behavior, and added a new test for the data-nonce/
aria-nonce false-positive. 93/93 tests pass in
render-document.test.ts; CanvasFrame.test.ts (4/4) and
render-published.test.ts (19/19) unaffected.

* fix(csp): fix nonce-tokenizer HTML corruption, narrow storage wildcard, dedupe nonce read

Findings from a proactive multi-angle code-review pass (4 parallel finder
agents) on this branch's own diff. Two confirmed correctness/security bugs,
one empirically-verified non-issue, one empirically-verified false positive,
plus two cleanup items.

1. CONFIRMED — stampScriptNonce() could corrupt unrelated attribute values
   (packages/lib/src/canvas/render-document.ts). The nonce-detection regex
   searched for the raw substring " nonce=" anywhere in the tag, which also
   matched inside a DIFFERENT attribute's own quoted value (e.g.
   `<script data-log="utm_source=x nonce=stale123">`), truncating and
   re-quoting that attribute mid-string — malformed HTML. Rewrote as a real
   attribute tokenizer that consumes one whole attribute (name + value) per
   step, so a value's contents are never re-scanned as if they were tag
   syntax. This also correctly handles a bare/valueless `nonce` attribute
   (previously not detected, would have produced a duplicate attribute).
   New tests cover both cases; all 95 render-document tests pass.

2. CONFIRMED (security) — the connect-src/media-src storage-host wildcard
   (`*.<host>`) was scoped one DNS label too high. For a shared multi-tenant
   S3-compatible host (Tigris, AWS S3), that wildcard permits connect-src to
   ANY other tenant's bucket on the same provider domain — self-serve
   provisionable by an attacker — not just this app's own bucket. Since the
   bucket name is already known (mirrors presigned-url.ts's getS3Bucket()
   env-var precedence — duplicated, not imported, because that module pulls
   in the Node-only AWS SDK which is unsafe to import into this Edge-runtime
   middleware module), narrowed to `<bucket>.<host>` — the exact address
   virtual-hosted-style addressing actually produces, with zero widening
   beyond this app's own storage. New tests assert the wildcard is gone and
   that a configured BUCKET_NAME is respected.

3. REFUTED (verified against Next.js docs) — a reviewer flagged that
   dashboard/layout.tsx reads the CSP nonce via headers() without also
   calling connection(), unlike the root layout, and asked whether that
   risks a stale/cached nonce. Per Next.js's own connection() docs:
   "the function is only necessary when dynamic rendering is required and
   common Request-time APIs are not used" — headers() is itself a
   Request-time API, so it already guarantees per-request freshness on its
   own. No fix needed; documented for the record.

4. EMPIRICALLY VERIFIED (non-issue) — a reviewer flagged that Shiki's JS
   regex engine (switched from the WASM oniguruma engine to avoid needing
   'wasm-unsafe-eval' in script-src) might silently fail to highlight some
   TextMate grammars the app offers beyond the two languages the original
   tests covered (javascript, sudolang). Added
   shiki-js-engine-coverage.test.ts, which tokenizes a real snippet in
   EVERY language LanguageSelector.tsx actually offers (typescript, python,
   html, css, json, markdown, bash, sql, rust, go, yaml) and asserts each
   produces genuine multi-color highlighting, not a silent plain-text
   fallback. All 12 pass — kept as a permanent regression guard.

5. Cleanup — buildStorageConnectSrcEntries() was called twice per
   buildCSPPolicy() invocation (once for connect-src, once for media-src),
   redundantly re-parsing AWS_ENDPOINT_URL_S3 both times; hoisted to a
   single local computed once and spread into both directives.

6. Cleanup — the "read the per-request nonce from headers()" snippet was
   duplicated verbatim between app/layout.tsx and app/dashboard/layout.tsx.
   Extracted to a shared apps/web/src/lib/request-nonce.ts (getRequestNonce()),
   used by both. Root layout's connection() call is untouched (redundant
   per finding #3, but not this PR's place to remove pre-existing, working,
   intentionally-commented code).

Documented but NOT fixed in this PR (out of scope, distinct trade-off):
inline event-handler attributes (onclick, onload, etc.) in canvas author
HTML remain blocked by the inherited outer CSP. Per CSP spec, a nonce
cannot authorize script-src-attr (event-handler) execution — only
<script> elements. The only way to restore this would be loosening
script-src-attr on the app-wide dashboard CSP, which (unlike the narrow,
already-permitted-by-canvas's-own-CSP nonce fix) would grant a genuinely
new capability across the ENTIRE app, not just the isolated canvas iframe —
a materially different security trade-off that shouldn't be made as a
drive-by fix. Flagging as a known follow-up.

All affected suites verified green: render-document.test.ts (95/95),
security-headers.test.ts (65/65), CanvasFrame.test.ts (4/4),
render-published.test.ts (19/19), all code-block/shiki suites (137/137
combined), apps/web tsc --noEmit clean, @pagespace/lib typecheck clean,
full `next build` clean.

* fix(canvas): fix polynomial ReDoS in stampScriptNonce attribute regex

GitHub Advanced Security's native CodeQL check flagged the new attribute
tokenizer (introduced earlier in this branch to fix the nonce-corruption
bug) as js/polynomial-redos (CWE-1333, high severity): "This regular
expression that depends on library input may run slow on strings with
many repetitions of ' '."

Root cause: the leading-whitespace group was `(\s+)`. On a long run of
whitespace with no valid attribute-name character following (e.g. a
padded/malformed script tag), the regex engine backtracks the `\s+` span
by one character and retries at each of the O(n) starting positions the
global `g` flag walks through — O(n²) overall. Confirmed empirically:
16,000 spaces took 413ms and scaled quadratically (2ms → 7ms → 26ms →
104ms → 413ms as n doubled four times); a ~1MB adversarial script tag
would take tens of minutes. This runs on user-authored canvas HTML
(isomorphic — both server and client per the module's own doc comment),
so it's a real DoS vector, not a theoretical one.

Fix: `(\s+)` → `(\s)` — a single required whitespace character instead of
one-or-more. This produces BYTE-IDENTICAL output (verified: all 95
existing tests still pass unchanged) because any additional whitespace
before the one immediately adjacent to the attribute name is simply left
untouched by `replace()` — it was never part of the match span, so it's
passed through exactly where it already was, same as before. Confirmed
empirically linear afterward: 200,000 spaces in 0ms (vs 413ms for 16,000
under the old regex).

Added a regression test that pads a script tag with 50,000 spaces and
asserts the fix completes in under 1s — verified this test correctly
fails (2.2s) with the vulnerable `\s+` reintroduced, and passes with the
fix. 96/96 tests pass.
2witstudios added a commit that referenced this pull request Jul 29, 2026
…ty, error UX, reconcile resilience

Six findings from the full-branch review. (#1 SSRF, #2 storage metering and #7
concurrency quota were already fixed in the two prior commits; #3's cron suite
now collects and passes 5 tests, so it was fixed by a later phase than the
review sampled.)

#4/#15 — prompt regression on default config. `buildAgentAwarenessPrompt`
unconditionally told the model to delegate with `spawn_session`, but session
tools only exist when CODE_EXECUTION_ENABLED is on (default off), so the
assistant confidently called a tool it did not have and delegation silently
broke. The delegation sentence is now gated on a `canDelegate` flag that both
call sites derive from `isCodeExecutionEnabled()`; without it the section still
lists the agents, it just stops naming a tool that isn't there.

#13/#14 — git argv safety. `git_add` spliced paths straight into argv with no
`--`, unlike its siblings `git status`/`git diff`: a path named `-p` was read as
a flag (interactive add). It now separates, and only when there are paths, so no
bare trailing `--`. `git_reset.ref` and `git_remote_add.name` gained the
`validateFlagSafe` guard that structurally identical sibling fields already
apply.

#10/#11 — infinite spinner on a failed agent load. `AgentView` guarded on
`agentLoading || !agent`, so once SWR gave up retrying, `isLoading` went false,
`agent` stayed null, and the user watched a spinner that would never resolve
with no error text and no escape but a reload. Loading and failure are now
distinct states: the failure surfaces the server's own message and a Try again
button, backed by a new `retry` from `useResolvedAgent`.

#6 — one failing candidate query no longer parks the other pass. The orphan
reconcile listed the reclaim outbox and the teardown-intent rows under
`Promise.all`, so either failing dropped both. Now `allSettled` with per-source
error logs: a degraded query costs its own candidates, not every reclaim, and
those are billing VMs nobody is using.

#8 — the leak signal is no longer silent. When a confirmed-unreferenced Sprite
fails BOTH its kill and its reclaim-outbox insert, nothing in the system knows
that VM exists — no row points at it, so no trigger and no cross-check will find
it. That path swallowed its error, making the one path built to catch a
permanently leaked VM the one path with no signal. It now logs loudly with the
sandbox id and both failure reasons.

#5 — the destructive teardown binding gets tests. 10 cases over `killSprite`
(confirmed kill, replaced-name-as-success, genuine failure, unpinned instance),
`markSessionTornDown` (CAS win/loss), and `listOrphanCandidates` (both sources,
each single-source failure, cap + backlog).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017V3eqwRX5cFy3Tdu2Syoro
2witstudios added a commit that referenced this pull request Aug 9, 2026
Fix the two HIGH defects (#1, #2) and the MEDIUM-HIGH (#3) from
`.pu-reports/pu-rev-phase1.md`. All are latent — nothing writes these
tables yet — and each would be a hand-written DELETE against production
later.

Finding #1 — `applyNodeWrite` prescribed drop-then-put. The
composite self-FK is ON DELETE cascade, so dropping a container took
its whole subtree with it. The collapse path drops a split whose
children are being reparented, not deleted, and those children are
only in `put`. Drop-first cascades them away and `put` cannot
resurrect them. Fixed: put before drop, with the cascade named in the
docblock so nobody tidies it back.

Finding #2 — `create` accepted an empty `nodeId` and a blank
`targetId`. Postgres stores both (text NOT NULL is satisfied by
''), and the read then rejects the whole set rather than filtering,
so the workspace becomes permanently unreadable. Fixed: `create`
refuses both with typed codes; `validateTree` refuses them too (the
gate every write path runs, especially `put(nodes[])`); the false
comment in `bind` is corrected; the untested compensating guard in
`open` is removed, with a new test for the split path it covered.

Finding #3 — `validateTree` skipped the finiteness sweep for parked
panes, so a NaN/Infinity share on a detached pane passed. Fixed:
the sweep is hoisted out of the group loop entirely, ahead of the
per-container fraction rules, with a comment that parking does not
make a share a number.

Also fixed: finding #6 (the byte-identical round-trip claim is
false; corrected to structural identity, with the change-test hazard
named), #10 (the FK's truncated live name is recorded), #11
(`put`'s ordering guarantee is stated), and #12 (`validateTree`'s
cast is removed, using the idiom `descendantsOf` already follows).

Report with mutation table and the deferrable-FK argument (no,
tested against PostgreSQL 17.5):
`.pu-reports/pu-fix-review.md`

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y4EZg67rRbxgutMEFi2UZJ
2witstudios added a commit that referenced this pull request Sep 9, 2026
… before every runner; a newer verified grant is the resume (Codex P1 #3, review round 1)

pausedAt is a high-water mark set before killAll; a grant issued at or before
it is refused paused (audited) at verification, after the terminal prompt,
after the approval writes on both paths, and immediately before any runner.
The server signs only while not paused, so a verified grant with a later
issuedAt proceeds — no resume frame. Mutants: drop either post-await recheck,
skip the latch, off-by-one on the comparison — all red.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01V6iGLmPYL3w565nDECjM5T
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant