Skip to content

Conversation

@emir-karabeg
Copy link
Collaborator

Summary

Adds a generic workflow notification store.

Type of Change

  • New feature

Testing

Solo testing.

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel
Copy link

vercel bot commented Nov 17, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Preview Comments Updated (UTC)
docs Skipped Skipped Nov 18, 2025 6:12am

@emir-karabeg emir-karabeg marked this pull request as draft November 17, 2025 09:22
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Nov 17, 2025

Greptile Summary

  • Implements generic notification store using Zustand with persistence for workflow-specific notifications and memory-only storage for global notifications
  • Replaces TriggerWarningDialog with notification system across workflow, integrates error notifications from terminal console and offline mode detection

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk.
  • Clean architecture following established patterns, proper TypeScript types, good error handling, no breaking changes, and well-integrated across the codebase. The notification system is a solid improvement over the previous dialog approach.
  • No files require special attention.

Important Files Changed

Filename Overview
apps/sim/stores/notifications/store.ts New notification store with Zustand, proper persistence strategy, and clean architecture.
apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/notifications/notifications.tsx Notification UI component with stacked display, action handlers, and proper memoization.
apps/sim/app/workspace/[workspaceId]/w/[workflowId]/workflow.tsx Replaced TriggerWarningDialog with notification store calls, cleaner approach for user feedback.
apps/sim/stores/terminal/console/store.ts Integrated notification system to surface block errors with copilot action, improving error visibility.
apps/sim/app/workspace/[workspaceId]/providers/workspace-permissions-provider.tsx Added offline mode notification with refresh action, deduplication logic prevents spam.

Sequence Diagram

sequenceDiagram
    participant User
    participant Workflow
    participant NotificationStore
    participant NotificationsUI
    participant Copilot
    
    User->>Workflow: Attempts to add duplicate trigger block
    Workflow->>NotificationStore: addNotification(error, message, action)
    NotificationStore->>NotificationStore: Create notification with UUID
    NotificationStore->>NotificationsUI: State update triggers re-render
    NotificationsUI->>User: Display error notification with "Fix in Copilot" button
    User->>NotificationsUI: Clicks "Fix in Copilot" button
    NotificationsUI->>Copilot: openCopilotWithMessage(errorMessage)
    Copilot->>Copilot: Switch to copilot tab
    Copilot->>Copilot: Send message with instructions
    NotificationsUI->>NotificationStore: removeNotification(id)
    NotificationStore->>NotificationsUI: State update removes notification
Loading

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile
React with 👍 or 👎 to share your feedback on this new summary format

Comment on lines 89 to 94
removedNotifications.forEach((n) => {
const timeout = state.timeouts.get(n.id)
if (timeout) {
clearTimeout(timeout)
state.timeouts.delete(n.id)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: State mutation detected in Zustand setter - state.timeouts.delete(n.id) directly mutates the state Map inside the setter callback, violating immutability principles.

Suggested change
removedNotifications.forEach((n) => {
const timeout = state.timeouts.get(n.id)
if (timeout) {
clearTimeout(timeout)
state.timeouts.delete(n.id)
}
removedNotifications.forEach((n) => {
const timeout = state.timeouts.get(n.id)
if (timeout) {
clearTimeout(timeout)
}
})
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/stores/notifications/store.ts
Line: 89:94

Comment:
**logic:** State mutation detected in Zustand setter - `state.timeouts.delete(n.id)` directly mutates the state Map inside the setter callback, violating immutability principles.

```suggestion
        removedNotifications.forEach((n) => {
          const timeout = state.timeouts.get(n.id)
          if (timeout) {
            clearTimeout(timeout)
          }
        })
```

How can I resolve this? If you propose a fix, please make it concise.

@emir-karabeg emir-karabeg marked this pull request as ready for review November 18, 2025 18:09
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

17 files reviewed, no comments

Edit Code Review Agent Settings | Greptile
React with 👍 or 👎 to share your feedback on this new summary format

@emir-karabeg emir-karabeg merged commit e0aade8 into staging Nov 18, 2025
9 checks passed
@emir-karabeg emir-karabeg deleted the feat/notifs branch November 18, 2025 18:43
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.

2 participants