Skip to content

feat(Notifications): Platform notifications for CLI and Dashboard #3254

Merged
0ski merged 7 commits intomainfrom
oskar/feat-platform-notifications
Mar 26, 2026
Merged

feat(Notifications): Platform notifications for CLI and Dashboard #3254
0ski merged 7 commits intomainfrom
oskar/feat-platform-notifications

Conversation

@0ski
Copy link
Collaborator

@0ski 0ski commented Mar 24, 2026

For human reviewer:

  • Check if Redis connection + code makes sense
  • Check CLI methods (it's on a hotpath)
  • Check DB Migrations and new tables

✅ Checklist

  • I have followed every step in the contributing guide
  • The PR title follows the convention.
  • I ran and tested the code works

Testing

Spawning new CLI / Dashboard notifications, check MVP, check if failures not produce any problems with CLI/Dashboard


Changelog

Added notifications mechanism for Dashboard and CLI


Screenshots

💯

@changeset-bot
Copy link

changeset-bot bot commented Mar 24, 2026

🦋 Changeset detected

Latest commit: 75b39e3

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 29 packages
Name Type
trigger.dev Patch
d3-chat Patch
references-d3-openai-agents Patch
references-nextjs-realtime Patch
references-realtime-hooks-test Patch
references-realtime-streams Patch
references-telemetry Patch
@trigger.dev/build Patch
@trigger.dev/core Patch
@trigger.dev/python Patch
@trigger.dev/react-hooks Patch
@trigger.dev/redis-worker Patch
@trigger.dev/rsc Patch
@trigger.dev/schema-to-json Patch
@trigger.dev/sdk Patch
@trigger.dev/database Patch
@trigger.dev/otlp-importer Patch
@internal/cache Patch
@internal/clickhouse Patch
@internal/llm-model-catalog Patch
@internal/redis Patch
@internal/replication Patch
@internal/run-engine Patch
@internal/schedule-engine Patch
@internal/testcontainers Patch
@internal/tracing Patch
@internal/tsql Patch
@internal/zod-worker Patch
@internal/sdk-compat-tests Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 24, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Adds a platform notifications feature: DB migration and Prisma models for notifications and interactions; server services for creating, querying, and recording interactions (web and CLI); multiple Remix routes for admin UI, public APIs, resource polling, and interaction beacons; admin notifications UI and a webapp NotificationPanel plus changelog polling and Help popover updates; CLI client/commands to fetch and display notifications with discovery and color-markup utilities and tests; a Redis-backed per-user CLI request counter; and adds react-markdown dependency.

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.34% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: adding a platform notifications feature for both CLI and Dashboard. It is concise, descriptive, and directly related to the primary objective of the changeset.
Description check ✅ Passed The PR description includes all required checklist items completed and provides testing/changelog details, but lacks the issue reference and detailed testing steps.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch oskar/feat-platform-notifications

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@0ski 0ski force-pushed the oskar/feat-platform-notifications branch from 7e4eac9 to dc9619f Compare March 24, 2026 09:39
devin-ai-integration[bot]

This comment was marked as resolved.

coderabbitai[bot]

This comment was marked as resolved.

@0ski 0ski force-pushed the oskar/feat-platform-notifications branch from dc9619f to abc723f Compare March 24, 2026 10:48
github-advanced-security[bot]

This comment was marked as resolved.

coderabbitai[bot]

This comment was marked as resolved.

@0ski 0ski force-pushed the oskar/feat-platform-notifications branch from abc723f to 1980052 Compare March 24, 2026 19:05
devin-ai-integration[bot]

This comment was marked as resolved.

coderabbitai[bot]

This comment was marked as resolved.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

♻️ Duplicate comments (6)
apps/webapp/app/services/platformNotifications.server.ts (6)

513-533: ⚠️ Potential issue | 🟠 Major

Surface/type mismatch is not validated.

validateSurfaceFields blocks CLI-only numeric fields for WEBAPP surface but doesn't validate that the payload type matches the surface. For example, surface: "CLI" with type: "card" or "changelog" (webapp-only types) would be accepted.

Consider adding type validation to ensure CLI surfaces only allow CLI types (info, warn, error, success) and WEBAPP surfaces only allow webapp types (card, changelog).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/webapp/app/services/platformNotifications.server.ts` around lines 513 -
533, validateSurfaceFields currently blocks CLI-only numeric fields for WEBAPP
but doesn't enforce that the payload "type" matches the "surface"; update
validateSurfaceFields to also validate data.type against allowed sets depending
on data.surface: define CLI_ALLOWED_TYPES = ["info","warn","error","success"]
and WEBAPP_ALLOWED_TYPES = ["card","changelog"] and when data.surface ===
"WEBAPP" ensure data.type is one of WEBAPP_ALLOWED_TYPES (otherwise call
ctx.addIssue with code "custom", path ["type"], and a clear message), and when
data.surface === "CLI" ensure data.type is one of CLI_ALLOWED_TYPES (similarly
report via ctx.addIssue); keep the existing CLI_ONLY_FIELDS checks intact so
both field-level and type-level mismatches are reported.

9-27: ⚠️ Potential issue | 🟠 Major

ReDoS vulnerability in user-supplied regex patterns.

The contentPattern refinement only validates that the regex compiles but doesn't prevent catastrophically backtracking patterns. A malicious or poorly written regex can hang CLI clients when executed against local files.

Consider constraining discovery to literal matching, limiting pattern complexity, or using a safe-regex validation library.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/webapp/app/services/platformNotifications.server.ts` around lines 9 -
27, The contentPattern refinement in DiscoverySchema only checks that
user-supplied regexes compile (symbol: contentPattern inside DiscoverySchema)
but allows ReDoS-prone patterns; replace this unsafe behavior by either (a)
switching contentPattern to a literal substring match (e.g., rename to
contentLiteral and validate as plain string), or (b) validate/whitelist the
pattern using a safe-regex checker (e.g., integrate a library such as safe-regex
or use an RE2 binding) and reject/limit patterns that are too complex (max
length, disallow nested quantifiers/backreferences), or (c) refuse regexes
entirely and require user-provided globs/strings; update DiscoverySchema
validation and any code paths that execute the pattern (search/match logic that
reads contentPattern) to use the chosen safe approach so runtime matching cannot
be hijacked by catastrophic backtracking.

363-372: ⚠️ Potential issue | 🟠 Major

Multi-org users only receive notifications from one arbitrary organization.

When projectRef is absent, findFirst returns a single membership, so users in multiple organizations will only see ORG-scoped notifications from whichever org is returned first. Consider using findMany and including all organization IDs in the scope filter.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/webapp/app/services/platformNotifications.server.ts` around lines 363 -
372, The code uses prisma.orgMember.findFirst which picks a single organization
for multi-org users; change this to prisma.orgMember.findMany to fetch all
membership records (e.g., assign to memberships) and derive an array of
organizationIds instead of a single organizationId, then update the subsequent
notification scope/filter logic to use that array (e.g., pass organizationIds or
use where organizationId IN organizationIds) so ORG-scoped notifications are
evaluated against all user organizations; adjust any variable names (membership
-> memberships) and downstream uses (organizationId -> organizationIds) in the
surrounding functions (including platformNotifications.server.ts scope/filter
logic) accordingly.

472-485: ⚠️ Potential issue | 🟡 Minor

Allow creating notifications with endsAt <= startsAt.

The schema doesn't validate that endsAt is after startsAt (or after now when startsAt is omitted). This allows creating notification windows that can never be active.

Add a refinement to ensure endsAt > (startsAt ?? now).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/webapp/app/services/platformNotifications.server.ts` around lines 472 -
485, The schema's superRefine currently calls validateScopeForeignKeys,
validateSurfaceFields, and validateStartsAt but doesn't ensure endsAt is after
startsAt (or now when startsAt is missing), allowing invalid windows; inside the
existing .superRefine((data, ctx) => { ... }) add a check that compares
data.endsAt (a Date from the transform) to (data.startsAt ?? new Date()), and if
endsAt <= baseTime call ctx.addIssue (ZodIssueCode.custom) with a clear message
and path ['endsAt'] so the schema rejects creations where endsAt is not strictly
greater than the effective start time.

69-89: ⚠️ Potential issue | 🟠 Major

Avoid loading all interaction rows for aggregate stats.

The query includes the full interactions array (lines 79-85) only to count clicks and dismissals in memory (lines 110-113). For high-reach notifications, this could load thousands of rows per admin page load.

Consider using Prisma's _count with filtered relations or a separate aggregation query.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/webapp/app/services/platformNotifications.server.ts` around lines 69 -
89, The current prisma.platformNotification.findMany loads full interactions
(the interactions include select block) just to compute click/dismiss counts and
risks O(N) rows per notification; change it to avoid loading interaction rows by
removing the interactions include and instead fetch aggregated counts—either
extend the query to use prisma.platformNotification.findMany with relation
counts (use _count where possible) or run a separate aggregation using
prisma.notificationInteraction.count or groupBy filtered by notificationId and
flags (e.g., webappClickedAt not null / webappDismissedAt not null) for the
page’s notification IDs, then map those aggregated counts back to notifications;
update the code that currently reads interactions (the places using interactions
to compute clicks/dismissals) to read from the aggregated counts instead.

278-298: ⚠️ Potential issue | 🟡 Minor

Scope filtering is missing for changelogs.

While the comment explains the intentional omission of time/archived filters, scoped changelogs (USER, ORGANIZATION, PROJECT) will still be visible to all users. If changelogs are only meant to be GLOBAL, consider adding a scope: "GLOBAL" filter; otherwise, add scope filtering similar to getActivePlatformNotifications.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/webapp/app/services/platformNotifications.server.ts` around lines 278 -
298, getRecentChangelogs currently queries prisma.platformNotification.findMany
without scoping, so changelogs targeted to USER/ORGANIZATION/PROJECT will leak
to everyone; update the where clause in getRecentChangelogs to include a scope
filter (either scope: "GLOBAL" if changelogs should be global-only, or accept a
scope parameter and mirror the scope filtering logic used in
getActivePlatformNotifications) to restrict results appropriately while keeping
PayloadV1Schema.safeParse and the existing mapping logic intact.
🧹 Nitpick comments (4)
apps/webapp/app/components/navigation/NotificationPanel.tsx (2)

55-65: Add dismissFetcher to the dependency array or document the intentional omission.

The handleDismiss callback uses dismissFetcher.submit but has an empty dependency array. While useFetcher provides stable references, this may trigger lint warnings.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/webapp/app/components/navigation/NotificationPanel.tsx` around lines 55
- 65, The useCallback for handleDismiss captures dismissFetcher but currently
has an empty dependency array which can trigger lint warnings; update the
dependencies to include dismissFetcher (and setDismissedIds if not stable) so
the array becomes [dismissFetcher] or, if you intentionally rely on
dismissFetcher being stable from useFetcher, add a brief inline comment and an
eslint-disable-next-line react-hooks/exhaustive-deps above the useCallback to
document the omission; locate the handleDismiss definition and update its
dependency array or add the explanatory eslint-disable and comment accordingly.

94-99: Add fireSeenBeacon to the effect's dependency array.

The effect calls fireSeenBeacon but it's not listed in the dependencies. Since fireSeenBeacon is memoized with useCallback, adding it won't cause extra effect runs but will satisfy exhaustive-deps linting.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/webapp/app/components/navigation/NotificationPanel.tsx` around lines 94
- 99, The useEffect that calls fireSeenBeacon when mounting references
fireSeenBeacon but doesn't include it in the dependency array; update the
dependency array for the useEffect (the effect that checks notification and
hasIncident) to include fireSeenBeacon alongside notification?.id and
hasIncident so exhaustive-deps is satisfied — locate the useEffect block in
NotificationPanel.tsx (the one using notification, hasIncident and calling
fireSeenBeacon) and add fireSeenBeacon to its dependencies.
apps/webapp/app/services/platformNotificationCounter.server.ts (1)

9-25: Consider sharing the existing Redis client instead of creating a duplicate connection.

This initialization is nearly identical to inputStreamWaitpointCache.server.ts (same host, port, credentials, keyPrefix: "tr:", and TLS settings). Each singleton creates a separate connection to the same Redis instance.

Consider extracting a shared cache Redis client or reusing the existing one to reduce connection overhead.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/webapp/app/services/platformNotificationCounter.server.ts` around lines
9 - 25, The current initializeRedis function (used to create the
singleton("platformNotificationCounter", initializeRedis) client) duplicates the
same connection settings used elsewhere (e.g., inputStreamWaitpoint cache);
extract the Redis creation into a single shared factory and reuse that singleton
instead of creating another one. Specifically, move the Redis initialization
logic (host/port/username/password/keyPrefix/tls/enableAutoPipelining and
connectionName selection) into a shared module (e.g., getSharedRedisClient or
sharedRedisSingleton) and have platformNotificationCounter.server.ts call that
shared factory (or accept the shared client via injection) so both consumers
reuse the same Redis instance rather than instantiating a new client. Ensure the
shared factory preserves the keyPrefix and TLS conditional and uses a
descriptive singleton key to avoid duplicate connections.
apps/webapp/app/routes/resources.platform-notifications.tsx (1)

39-54: Consider adding fetcher to the dependency array.

The useEffect references fetcher.state and fetcher.load but doesn't include fetcher in the dependency array. While useFetcher returns a stable reference in Remix, this may trigger ESLint's exhaustive-deps warning.

If adding fetcher causes unwanted interval recreation, you can suppress with // eslint-disable-next-line react-hooks/exhaustive-deps with a comment explaining the stable reference.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/webapp/app/routes/resources.platform-notifications.tsx` around lines 39
- 54, The effect uses fetcher.state and fetcher.load but omits fetcher from the
dependency array; update the useEffect dependency list to include fetcher
(alongside organizationId and projectId) or, if you rely on Remix's stable
useFetcher reference and want to avoid recreating the interval, add a clear
comment and suppress the exhaustive-deps rule with // eslint-disable-next-line
react-hooks/exhaustive-deps; make the change where useEffect is defined
(referencing useEffect, lastLoadedUrl.current, fetcher.load, fetcher.state, and
POLL_INTERVAL_MS).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@apps/webapp/app/services/platformNotifications.server.ts`:
- Around line 513-533: validateSurfaceFields currently blocks CLI-only numeric
fields for WEBAPP but doesn't enforce that the payload "type" matches the
"surface"; update validateSurfaceFields to also validate data.type against
allowed sets depending on data.surface: define CLI_ALLOWED_TYPES =
["info","warn","error","success"] and WEBAPP_ALLOWED_TYPES =
["card","changelog"] and when data.surface === "WEBAPP" ensure data.type is one
of WEBAPP_ALLOWED_TYPES (otherwise call ctx.addIssue with code "custom", path
["type"], and a clear message), and when data.surface === "CLI" ensure data.type
is one of CLI_ALLOWED_TYPES (similarly report via ctx.addIssue); keep the
existing CLI_ONLY_FIELDS checks intact so both field-level and type-level
mismatches are reported.
- Around line 9-27: The contentPattern refinement in DiscoverySchema only checks
that user-supplied regexes compile (symbol: contentPattern inside
DiscoverySchema) but allows ReDoS-prone patterns; replace this unsafe behavior
by either (a) switching contentPattern to a literal substring match (e.g.,
rename to contentLiteral and validate as plain string), or (b)
validate/whitelist the pattern using a safe-regex checker (e.g., integrate a
library such as safe-regex or use an RE2 binding) and reject/limit patterns that
are too complex (max length, disallow nested quantifiers/backreferences), or (c)
refuse regexes entirely and require user-provided globs/strings; update
DiscoverySchema validation and any code paths that execute the pattern
(search/match logic that reads contentPattern) to use the chosen safe approach
so runtime matching cannot be hijacked by catastrophic backtracking.
- Around line 363-372: The code uses prisma.orgMember.findFirst which picks a
single organization for multi-org users; change this to
prisma.orgMember.findMany to fetch all membership records (e.g., assign to
memberships) and derive an array of organizationIds instead of a single
organizationId, then update the subsequent notification scope/filter logic to
use that array (e.g., pass organizationIds or use where organizationId IN
organizationIds) so ORG-scoped notifications are evaluated against all user
organizations; adjust any variable names (membership -> memberships) and
downstream uses (organizationId -> organizationIds) in the surrounding functions
(including platformNotifications.server.ts scope/filter logic) accordingly.
- Around line 472-485: The schema's superRefine currently calls
validateScopeForeignKeys, validateSurfaceFields, and validateStartsAt but
doesn't ensure endsAt is after startsAt (or now when startsAt is missing),
allowing invalid windows; inside the existing .superRefine((data, ctx) => { ...
}) add a check that compares data.endsAt (a Date from the transform) to
(data.startsAt ?? new Date()), and if endsAt <= baseTime call ctx.addIssue
(ZodIssueCode.custom) with a clear message and path ['endsAt'] so the schema
rejects creations where endsAt is not strictly greater than the effective start
time.
- Around line 69-89: The current prisma.platformNotification.findMany loads full
interactions (the interactions include select block) just to compute
click/dismiss counts and risks O(N) rows per notification; change it to avoid
loading interaction rows by removing the interactions include and instead fetch
aggregated counts—either extend the query to use
prisma.platformNotification.findMany with relation counts (use _count where
possible) or run a separate aggregation using
prisma.notificationInteraction.count or groupBy filtered by notificationId and
flags (e.g., webappClickedAt not null / webappDismissedAt not null) for the
page’s notification IDs, then map those aggregated counts back to notifications;
update the code that currently reads interactions (the places using interactions
to compute clicks/dismissals) to read from the aggregated counts instead.
- Around line 278-298: getRecentChangelogs currently queries
prisma.platformNotification.findMany without scoping, so changelogs targeted to
USER/ORGANIZATION/PROJECT will leak to everyone; update the where clause in
getRecentChangelogs to include a scope filter (either scope: "GLOBAL" if
changelogs should be global-only, or accept a scope parameter and mirror the
scope filtering logic used in getActivePlatformNotifications) to restrict
results appropriately while keeping PayloadV1Schema.safeParse and the existing
mapping logic intact.

---

Nitpick comments:
In `@apps/webapp/app/components/navigation/NotificationPanel.tsx`:
- Around line 55-65: The useCallback for handleDismiss captures dismissFetcher
but currently has an empty dependency array which can trigger lint warnings;
update the dependencies to include dismissFetcher (and setDismissedIds if not
stable) so the array becomes [dismissFetcher] or, if you intentionally rely on
dismissFetcher being stable from useFetcher, add a brief inline comment and an
eslint-disable-next-line react-hooks/exhaustive-deps above the useCallback to
document the omission; locate the handleDismiss definition and update its
dependency array or add the explanatory eslint-disable and comment accordingly.
- Around line 94-99: The useEffect that calls fireSeenBeacon when mounting
references fireSeenBeacon but doesn't include it in the dependency array; update
the dependency array for the useEffect (the effect that checks notification and
hasIncident) to include fireSeenBeacon alongside notification?.id and
hasIncident so exhaustive-deps is satisfied — locate the useEffect block in
NotificationPanel.tsx (the one using notification, hasIncident and calling
fireSeenBeacon) and add fireSeenBeacon to its dependencies.

In `@apps/webapp/app/routes/resources.platform-notifications.tsx`:
- Around line 39-54: The effect uses fetcher.state and fetcher.load but omits
fetcher from the dependency array; update the useEffect dependency list to
include fetcher (alongside organizationId and projectId) or, if you rely on
Remix's stable useFetcher reference and want to avoid recreating the interval,
add a clear comment and suppress the exhaustive-deps rule with //
eslint-disable-next-line react-hooks/exhaustive-deps; make the change where
useEffect is defined (referencing useEffect, lastLoadedUrl.current,
fetcher.load, fetcher.state, and POLL_INTERVAL_MS).

In `@apps/webapp/app/services/platformNotificationCounter.server.ts`:
- Around line 9-25: The current initializeRedis function (used to create the
singleton("platformNotificationCounter", initializeRedis) client) duplicates the
same connection settings used elsewhere (e.g., inputStreamWaitpoint cache);
extract the Redis creation into a single shared factory and reuse that singleton
instead of creating another one. Specifically, move the Redis initialization
logic (host/port/username/password/keyPrefix/tls/enableAutoPipelining and
connectionName selection) into a shared module (e.g., getSharedRedisClient or
sharedRedisSingleton) and have platformNotificationCounter.server.ts call that
shared factory (or accept the shared client via injection) so both consumers
reuse the same Redis instance rather than instantiating a new client. Ensure the
shared factory preserves the keyPrefix and TLS conditional and uses a
descriptive singleton key to avoid duplicate connections.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 4a236ddf-3f0c-4c16-b168-96ce9e94a4d1

📥 Commits

Reviewing files that changed from the base of the PR and between 1980052 and a6f6bb1.

📒 Files selected for processing (5)
  • apps/webapp/app/components/navigation/NotificationPanel.tsx
  • apps/webapp/app/routes/admin.api.v1.platform-notifications.ts
  • apps/webapp/app/routes/resources.platform-notifications.tsx
  • apps/webapp/app/services/platformNotificationCounter.server.ts
  • apps/webapp/app/services/platformNotifications.server.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • apps/webapp/app/routes/admin.api.v1.platform-notifications.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
  • GitHub Check: typecheck / typecheck
  • GitHub Check: sdk-compat / Node.js 22.12 (ubuntu-latest)
  • GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
  • GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
  • GitHub Check: sdk-compat / Cloudflare Workers
  • GitHub Check: sdk-compat / Node.js 20.20 (ubuntu-latest)
  • GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
  • GitHub Check: sdk-compat / Bun Runtime
  • GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
  • GitHub Check: sdk-compat / Deno Runtime
  • GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead

**/*.{ts,tsx}: For apps and internal packages (apps/*, internal-packages/*), use pnpm run typecheck --filter <package> for verification, never use build as it proves almost nothing about correctness
Use testcontainers helpers (redisTest, postgresTest, containerTest from @internal/testcontainers) for integration tests with Redis and PostgreSQL instead of mocking
When writing Trigger.dev tasks, always import from @trigger.dev/sdk - never use @trigger.dev/sdk/v3 or deprecated client.defineJob

Files:

  • apps/webapp/app/services/platformNotificationCounter.server.ts
  • apps/webapp/app/components/navigation/NotificationPanel.tsx
  • apps/webapp/app/routes/resources.platform-notifications.tsx
  • apps/webapp/app/services/platformNotifications.server.ts
{packages/core,apps/webapp}/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use zod for validation in packages/core and apps/webapp

Files:

  • apps/webapp/app/services/platformNotificationCounter.server.ts
  • apps/webapp/app/components/navigation/NotificationPanel.tsx
  • apps/webapp/app/routes/resources.platform-notifications.tsx
  • apps/webapp/app/services/platformNotifications.server.ts
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use function declarations instead of default exports

**/*.{ts,tsx,js,jsx}: Use pnpm for package management in this monorepo (version 10.23.0) with Turborepo for orchestration - run commands from root with pnpm run
Add crumbs as you write code for debug tracing using // @Crumbs comments or `// `#region` `@crumbs blocks - they stay on the branch throughout development and are stripped via agentcrumbs strip before merge

Files:

  • apps/webapp/app/services/platformNotificationCounter.server.ts
  • apps/webapp/app/components/navigation/NotificationPanel.tsx
  • apps/webapp/app/routes/resources.platform-notifications.tsx
  • apps/webapp/app/services/platformNotifications.server.ts
apps/webapp/app/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)

Access all environment variables through the env export of env.server.ts instead of directly accessing process.env in the Trigger.dev webapp

Files:

  • apps/webapp/app/services/platformNotificationCounter.server.ts
  • apps/webapp/app/components/navigation/NotificationPanel.tsx
  • apps/webapp/app/routes/resources.platform-notifications.tsx
  • apps/webapp/app/services/platformNotifications.server.ts
apps/webapp/app/services/**/*.server.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)

Separate testable services from configuration files; follow the pattern of realtimeClient.server.ts (testable service) and realtimeClientGlobal.server.ts (configuration) in the webapp

Files:

  • apps/webapp/app/services/platformNotificationCounter.server.ts
  • apps/webapp/app/services/platformNotifications.server.ts
apps/webapp/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)

apps/webapp/**/*.{ts,tsx}: When importing from @trigger.dev/core in the webapp, use subpath exports from the package.json instead of importing from the root path
Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webapp

Files:

  • apps/webapp/app/services/platformNotificationCounter.server.ts
  • apps/webapp/app/components/navigation/NotificationPanel.tsx
  • apps/webapp/app/routes/resources.platform-notifications.tsx
  • apps/webapp/app/services/platformNotifications.server.ts
**/*.ts

📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)

**/*.ts: When creating or editing OTEL metrics (counters, histograms, gauges), ensure metric attributes have low cardinality by using only enums, booleans, bounded error codes, or bounded shard IDs
Do not use high-cardinality attributes in OTEL metrics such as UUIDs/IDs (envId, userId, runId, projectId, organizationId), unbounded integers (itemCount, batchSize, retryCount), timestamps (createdAt, startTime), or free-form strings (errorMessage, taskName, queueName)
When exporting OTEL metrics via OTLP to Prometheus, be aware that the exporter automatically adds unit suffixes to metric names (e.g., 'my_duration_ms' becomes 'my_duration_ms_milliseconds', 'my_counter' becomes 'my_counter_total'). Account for these transformations when writing Grafana dashboards or Prometheus queries

Files:

  • apps/webapp/app/services/platformNotificationCounter.server.ts
  • apps/webapp/app/services/platformNotifications.server.ts
**/*.{js,ts,jsx,tsx,json,md,yaml,yml}

📄 CodeRabbit inference engine (AGENTS.md)

Format code using Prettier before committing

Files:

  • apps/webapp/app/services/platformNotificationCounter.server.ts
  • apps/webapp/app/components/navigation/NotificationPanel.tsx
  • apps/webapp/app/routes/resources.platform-notifications.tsx
  • apps/webapp/app/services/platformNotifications.server.ts
apps/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (CLAUDE.md)

When modifying only server components (apps/webapp/, apps/supervisor/, etc.) with no package changes, add a .server-changes/ file instead of a changeset

Files:

  • apps/webapp/app/services/platformNotificationCounter.server.ts
  • apps/webapp/app/components/navigation/NotificationPanel.tsx
  • apps/webapp/app/routes/resources.platform-notifications.tsx
  • apps/webapp/app/services/platformNotifications.server.ts
apps/webapp/app/**/*.{ts,tsx,server.ts}

📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)

Access environment variables via env export from app/env.server.ts. Never use process.env directly

Files:

  • apps/webapp/app/services/platformNotificationCounter.server.ts
  • apps/webapp/app/components/navigation/NotificationPanel.tsx
  • apps/webapp/app/routes/resources.platform-notifications.tsx
  • apps/webapp/app/services/platformNotifications.server.ts
🧠 Learnings (25)
📚 Learning: 2026-01-12T17:18:09.451Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 2870
File: apps/webapp/app/services/redisConcurrencyLimiter.server.ts:56-66
Timestamp: 2026-01-12T17:18:09.451Z
Learning: In `apps/webapp/app/services/redisConcurrencyLimiter.server.ts`, the query concurrency limiter will not be deployed with Redis Cluster mode, so multi-key operations (keyKey and globalKey in different hash slots) are acceptable and will function correctly in standalone Redis mode.

Applied to files:

  • apps/webapp/app/services/platformNotificationCounter.server.ts
📚 Learning: 2025-11-27T16:26:58.661Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-11-27T16:26:58.661Z
Learning: Applies to apps/webapp/app/services/**/*.server.{ts,tsx} : Separate testable services from configuration files; follow the pattern of `realtimeClient.server.ts` (testable service) and `realtimeClientGlobal.server.ts` (configuration) in the webapp

Applied to files:

  • apps/webapp/app/services/platformNotificationCounter.server.ts
📚 Learning: 2026-03-23T06:24:14.566Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-23T06:24:14.566Z
Learning: Applies to apps/**/*.{ts,tsx,js,jsx} : When modifying only server components (`apps/webapp/`, `apps/supervisor/`, etc.) with no package changes, add a `.server-changes/` file instead of a changeset

Applied to files:

  • apps/webapp/app/services/platformNotificationCounter.server.ts
  • apps/webapp/app/services/platformNotifications.server.ts
📚 Learning: 2026-03-23T06:24:25.029Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: apps/webapp/CLAUDE.md:0-0
Timestamp: 2026-03-23T06:24:25.029Z
Learning: Applies to apps/webapp/app/v3/services/**/*.server.ts : Only modify V2 code paths when editing services that branch on `RunEngineVersion` to support both V1 and V2 (e.g., `cancelTaskRun.server.ts`, `batchTriggerV3.server.ts`)

Applied to files:

  • apps/webapp/app/services/platformNotificationCounter.server.ts
  • apps/webapp/app/services/platformNotifications.server.ts
📚 Learning: 2026-03-22T13:26:12.060Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3244
File: apps/webapp/app/components/code/TextEditor.tsx:81-86
Timestamp: 2026-03-22T13:26:12.060Z
Learning: In the triggerdotdev/trigger.dev codebase, do not flag `navigator.clipboard.writeText(...)` calls for `missing-await`/`unhandled-promise` issues. These clipboard writes are intentionally invoked without `await` and without `catch` handlers across the project; keep that behavior consistent when reviewing TypeScript/TSX files (e.g., usages like in `apps/webapp/app/components/code/TextEditor.tsx`).

Applied to files:

  • apps/webapp/app/services/platformNotificationCounter.server.ts
  • apps/webapp/app/components/navigation/NotificationPanel.tsx
  • apps/webapp/app/routes/resources.platform-notifications.tsx
  • apps/webapp/app/services/platformNotifications.server.ts
📚 Learning: 2026-03-22T19:24:14.403Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3187
File: apps/webapp/app/v3/services/alerts/deliverErrorGroupAlert.server.ts:200-204
Timestamp: 2026-03-22T19:24:14.403Z
Learning: In the triggerdotdev/trigger.dev codebase, webhook URLs are not expected to contain embedded credentials/secrets (e.g., fields like `ProjectAlertWebhookProperties` should only hold credential-free webhook endpoints). During code review, if you see logging or inclusion of raw webhook URLs in error messages, do not automatically treat it as a credential-leak/secrets-in-logs issue by default—first verify the URL does not contain embedded credentials (for example, no username/password in the URL, no obvious secret/token query params or fragments). If the URL is credential-free per this project’s conventions, allow the logging.

Applied to files:

  • apps/webapp/app/services/platformNotificationCounter.server.ts
  • apps/webapp/app/components/navigation/NotificationPanel.tsx
  • apps/webapp/app/routes/resources.platform-notifications.tsx
  • apps/webapp/app/services/platformNotifications.server.ts
📚 Learning: 2026-01-28T16:57:47.620Z
Learnt from: samejr
Repo: triggerdotdev/trigger.dev PR: 2964
File: apps/webapp/app/components/AskAI.tsx:121-141
Timestamp: 2026-01-28T16:57:47.620Z
Learning: In the trigger.dev webapp codebase, the Button component (apps/webapp/app/components/primitives/Buttons) does not spread unknown props to the underlying <button> element—it only passes specific props (type, disabled, onClick, name, value, ref, form, autoFocus). When using Radix UI's TooltipTrigger with asChild, a span wrapper around the Button is necessary to receive Radix props (aria-describedby, onPointerEnter, onPointerLeave, data-state) while the Button handles its own behavior. Directly making the Button the child of TooltipTrigger with asChild will break accessibility and tooltip functionality.

Applied to files:

  • apps/webapp/app/components/navigation/NotificationPanel.tsx
📚 Learning: 2026-02-11T16:37:32.429Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3019
File: apps/webapp/app/components/primitives/charts/Card.tsx:26-30
Timestamp: 2026-02-11T16:37:32.429Z
Learning: In projects using react-grid-layout, avoid relying on drag-handle class to imply draggability. Ensure drag-handle elements only affect dragging when the parent grid item is configured draggable in the layout; conditionally apply cursor styles based on the draggable prop. This improves correctness and accessibility.

Applied to files:

  • apps/webapp/app/components/navigation/NotificationPanel.tsx
  • apps/webapp/app/routes/resources.platform-notifications.tsx
📚 Learning: 2025-11-27T16:26:58.661Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-11-27T16:26:58.661Z
Learning: Applies to apps/webapp/**/*.{ts,tsx} : Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webapp

Applied to files:

  • apps/webapp/app/routes/resources.platform-notifications.tsx
  • apps/webapp/app/services/platformNotifications.server.ts
📚 Learning: 2026-03-23T06:24:25.029Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: apps/webapp/CLAUDE.md:0-0
Timestamp: 2026-03-23T06:24:25.029Z
Learning: Applies to apps/webapp/app/routes/**/*.ts : Use Remix flat-file route convention with dot-separated segments where `api.v1.tasks.$taskId.trigger.ts` maps to `/api/v1/tasks/:taskId/trigger`

Applied to files:

  • apps/webapp/app/routes/resources.platform-notifications.tsx
📚 Learning: 2026-02-03T18:27:40.429Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 2994
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsx:553-555
Timestamp: 2026-02-03T18:27:40.429Z
Learning: In apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsx, the menu buttons (e.g., Edit with PencilSquareIcon) in the TableCellMenu are intentionally icon-only with no text labels as a compact UI pattern. This is a deliberate design choice for this route; preserve the icon-only behavior for consistency in this file.

Applied to files:

  • apps/webapp/app/routes/resources.platform-notifications.tsx
📚 Learning: 2025-11-27T16:26:37.432Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-27T16:26:37.432Z
Learning: Applies to {packages/core,apps/webapp}/**/*.{ts,tsx} : Use zod for validation in packages/core and apps/webapp

Applied to files:

  • apps/webapp/app/services/platformNotifications.server.ts
📚 Learning: 2026-03-02T12:43:37.906Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: packages/core/CLAUDE.md:0-0
Timestamp: 2026-03-02T12:43:37.906Z
Learning: Exercise caution with changes to trigger.dev/core as they affect both the customer-facing SDK and server-side webapp - breaking changes can impact deployed user tasks and the platform simultaneously

Applied to files:

  • apps/webapp/app/services/platformNotifications.server.ts
📚 Learning: 2026-03-02T12:43:34.140Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: packages/cli-v3/CLAUDE.md:0-0
Timestamp: 2026-03-02T12:43:34.140Z
Learning: Applies to packages/cli-v3/src/build/**/* : Build system in `src/build/` should use configuration from `trigger.config.ts` in user projects to determine bundling, build extensions, and output structure

Applied to files:

  • apps/webapp/app/services/platformNotifications.server.ts
📚 Learning: 2026-03-10T12:44:19.869Z
Learnt from: nicktrn
Repo: triggerdotdev/trigger.dev PR: 3200
File: docs/config/config-file.mdx:353-368
Timestamp: 2026-03-10T12:44:19.869Z
Learning: In the triggerdotdev/trigger.dev repository, docs PRs are often written as companions to implementation PRs (e.g., PR `#3200` documents features being added in PR `#3196`). When reviewing docs PRs, the documented features may exist in a companion/companion PR branch rather than main. Always check companion PRs referenced in the PR description before flagging missing implementations.

Applied to files:

  • apps/webapp/app/services/platformNotifications.server.ts
📚 Learning: 2026-03-23T06:24:14.566Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-23T06:24:14.566Z
Learning: Applies to apps/webapp/app/v3/**/*.{ts,tsx} : In the webapp v3 directory, only modify V2 code paths when encountering V1/V2 branching in services - all new work uses Run Engine 2.0 (`internal/run-engine`) and redis-worker, not legacy V1 engine code

Applied to files:

  • apps/webapp/app/services/platformNotifications.server.ts
📚 Learning: 2026-03-22T19:26:49.299Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3187
File: internal-packages/emails/emails/alert-error-group.tsx:58-58
Timestamp: 2026-03-22T19:26:49.299Z
Learning: In the triggerdotdev/trigger.dev codebase, email template files under `internal-packages/emails/emails/` must use `export default function Email(...)` (default export) because the React Email previewer requires a default export to discover and render templates. Do not flag default exports in these files as violations of the "use named exports" coding guideline.

Applied to files:

  • apps/webapp/app/services/platformNotifications.server.ts
📚 Learning: 2025-11-27T16:26:58.661Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-11-27T16:26:58.661Z
Learning: Applies to apps/webapp/app/**/*.{ts,tsx} : Access all environment variables through the `env` export of `env.server.ts` instead of directly accessing `process.env` in the Trigger.dev webapp

Applied to files:

  • apps/webapp/app/services/platformNotifications.server.ts
📚 Learning: 2026-01-28T14:15:15.011Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 2953
File: apps/webapp/app/components/runs/v3/SharedFilters.tsx:441-452
Timestamp: 2026-01-28T14:15:15.011Z
Learning: In apps/webapp/app/components/runs/v3/SharedFilters.tsx, the maxPeriodDays limit for date ranges should only check the from date (via dateRangeToDays(fromValue)) because it enforces data retention limits—how far back in history queries can reach. The to date is irrelevant for retention-based limits.

Applied to files:

  • apps/webapp/app/services/platformNotifications.server.ts
📚 Learning: 2026-03-22T19:27:29.014Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3187
File: apps/webapp/app/v3/services/alerts/createAlertChannel.server.ts:104-112
Timestamp: 2026-03-22T19:27:29.014Z
Learning: In `apps/webapp/app/v3/services/alerts/createAlertChannel.server.ts`, the `#scheduleErrorAlertEvaluation` helper intentionally uses the same job id (`evaluateErrorAlerts:${projectId}`) as the evaluator's periodic self-chain. The deduplication is desired: if a future run is already queued, the immediate enqueue becomes a no-op, preventing two evaluations firing in quick succession. Do not flag this as a bug or suggest a unique/timestamped id.

Applied to files:

  • apps/webapp/app/services/platformNotifications.server.ts
📚 Learning: 2026-03-22T19:32:16.231Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3187
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.errors/route.tsx:82-151
Timestamp: 2026-03-22T19:32:16.231Z
Learning: In `apps/webapp/app/v3/services/alerts/createAlertChannel.server.ts` and `apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.errors/route.tsx`, the `errorAlertConfig` field on `ProjectAlertChannel` is intentionally `Json?` (nullable). The `ErrorAlertEvaluator.computeMinInterval()` in `apps/webapp/app/v3/services/alerts/errorAlertEvaluator.server.ts` uses `ErrorAlertConfig.safeParse(ch.errorAlertConfig)` and falls back to `DEFAULT_INTERVAL_MS = 300_000` when `errorAlertConfig` is null. No UI currently collects this value — it is scaffolding for a future per-channel evaluation interval feature. Do not flag the absence of `errorAlertConfig` in `CreateAlertChannelOptions` or the action handler as a bug; null configs are safe and expected.

Applied to files:

  • apps/webapp/app/services/platformNotifications.server.ts
📚 Learning: 2026-02-04T16:34:48.876Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 2994
File: apps/webapp/app/routes/vercel.connect.tsx:13-27
Timestamp: 2026-02-04T16:34:48.876Z
Learning: In apps/webapp/app/routes/vercel.connect.tsx, configurationId may be absent for "dashboard" flows but must be present for "marketplace" flows. Enforce this with a Zod superRefine and pass installationId to repository methods only when configurationId is defined (omit the field otherwise).

Applied to files:

  • apps/webapp/app/services/platformNotifications.server.ts
📚 Learning: 2026-02-11T16:50:14.167Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3019
File: apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dashboards.$dashboardId.widgets.tsx:126-131
Timestamp: 2026-02-11T16:50:14.167Z
Learning: In apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dashboards.$dashboardId.widgets.tsx, MetricsDashboard entities are intentionally scoped to the organization level, not the project level. The dashboard lookup should filter by organizationId only (not projectId), allowing dashboards to be accessed across projects within the same organization. The optional projectId field on MetricsDashboard serves other purposes and should not be used as an authorization constraint.

Applied to files:

  • apps/webapp/app/services/platformNotifications.server.ts
📚 Learning: 2026-03-10T17:56:26.581Z
Learnt from: samejr
Repo: triggerdotdev/trigger.dev PR: 3201
File: apps/webapp/app/v3/services/setSeatsAddOn.server.ts:25-29
Timestamp: 2026-03-10T17:56:26.581Z
Learning: In the `triggerdotdev/trigger.dev` webapp, service classes such as `SetSeatsAddOnService` and `SetBranchesAddOnService` do NOT need to perform their own userId-to-organizationId authorization checks. Auth is enforced at the route layer: `requireUserId(request)` authenticates the user, and the `_app.orgs.$organizationSlug` layout route enforces that the authenticated user is a member of the org. Any `userId` and `organizationId` reaching these services from org-scoped routes are already validated. This is the consistent pattern used across all org-scoped services in the codebase.

Applied to files:

  • apps/webapp/app/services/platformNotifications.server.ts
📚 Learning: 2026-03-13T13:42:59.104Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3213
File: apps/webapp/app/routes/admin.api.v1.llm-models.$modelId.ts:40-43
Timestamp: 2026-03-13T13:42:59.104Z
Learning: In `apps/webapp/app/routes/admin.api.v1.llm-models.$modelId.ts` and `apps/webapp/app/routes/admin.api.v1.llm-models.ts`, the `startDate` field in `UpdateModelSchema` and `CreateModelSchema` intentionally uses `z.string().optional()` (or `.nullable().optional()`) without strict ISO datetime validation. Invalid date strings are rejected at the Prisma/DB layer. This is acceptable because these are admin-only API routes protected by Personal Access Token (PAT) authentication and are not user-facing.

Applied to files:

  • apps/webapp/app/services/platformNotifications.server.ts
🪛 ast-grep (0.41.1)
apps/webapp/app/services/platformNotifications.server.ts

[warning] 17-17: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(val)
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html

(regexp-from-variable)

🔇 Additional comments (8)
apps/webapp/app/services/platformNotificationCounter.server.ts (1)

28-45: LGTM!

The counter logic is straightforward. The potential race between incr and set during wrap-around is acceptable for this throttling use case since the worst case is multiple near-simultaneous resets, which doesn't affect correctness.

apps/webapp/app/services/platformNotifications.server.ts (4)

29-45: LGTM!

The payload schema structure is well-defined with appropriate optional fields and URL validation.


125-191: LGTM!

The webapp notification retrieval logic correctly filters by scope, time window, and dismissal state. Safe payload parsing ensures malformed payloads don't crash the service.


205-274: LGTM!

The interaction upsert pattern is clean and reusable across seen, dismiss, and click operations.


405-442: LGTM on the showCount fix.

The separation of requestCounter (global per-user request cadence) from showCount (per-notification display count) correctly ensures that cliMaxShowCount tracks actual displays rather than API encounters. Good implementation.

apps/webapp/app/routes/resources.platform-notifications.tsx (1)

18-31: LGTM!

The loader correctly requires authentication, handles missing organizationId gracefully, and delegates to the service layer.

apps/webapp/app/components/navigation/NotificationPanel.tsx (2)

168-269: LGTM!

The NotificationCard component handles dismiss, expand/collapse, and click interactions cleanly with proper event propagation. The conditional wrapper pattern for links is well-implemented.


272-299: LGTM!

The markdown components are well-styled and the anchor click handler correctly stops propagation to prevent triggering the parent card's click handler.

devin-ai-integration[bot]

This comment was marked as resolved.

@0ski
Copy link
Collaborator Author

0ski commented Mar 25, 2026

Code review

Found 1 issue:

  1. The fireSeenBeacon effect fires on mount regardless of whether the sidebar is collapsed. When the sidebar is collapsed, the notification card is hidden (height: 0, opacity: 0), but fireSeenBeacon still fires, incrementing showCount for a notification the user never actually saw. This inflates the admin "Seen" stat and — more importantly — consumes cliMaxShowCount budget without a real impression. The fix is to check isCollapsed in the condition and add it to the dependency array, e.g. if (notification && !hasIncident && !isCollapsed).

// Beacon current notification on mount
useEffect(() => {
if (notification && !hasIncident) {
fireSeenBeacon(notification);
}
}, [notification?.id, hasIncident]);

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

@0ski 0ski force-pushed the oskar/feat-platform-notifications branch 2 times, most recently from 6be61b0 to 2360973 Compare March 26, 2026 09:45
devin-ai-integration[bot]

This comment was marked as resolved.

coderabbitai[bot]

This comment was marked as resolved.

@0ski 0ski force-pushed the oskar/feat-platform-notifications branch from 2360973 to c9c44c1 Compare March 26, 2026 10:06
devin-ai-integration[bot]

This comment was marked as resolved.

@0ski 0ski force-pushed the oskar/feat-platform-notifications branch from c9c44c1 to fd57fff Compare March 26, 2026 10:18
devin-ai-integration[bot]

This comment was marked as resolved.

@0ski 0ski force-pushed the oskar/feat-platform-notifications branch from fd57fff to 3c54860 Compare March 26, 2026 11:44
0ski added 5 commits March 26, 2026 12:44
Add PlatformNotification and PlatformNotificationInteraction Prisma models,
including enums PlatformNotificationSurface and PlatformNotificationScope,
indexes, timestamps, and fields for scoping (user, project, organization),
lifecycle (startsAt, endsAt, archivedAt) and delivery behavior (CLI-specific
fields, priority). Wire new relations into User, Organization, Project,
OrgMember and Workspace models so notifications and interactions can be
queried from those entities.

Add SQL migration to create the new enums and adjust schema constraints and
indexes required for the migration. The change enables admin-created,
scoped platform notifications with per-user interaction tracking for both
webapp and CLI surfaces.
feat(notifications): Add a way to notify users on platform
feat(notifications): Enforce notifications expired date, add CLI improvements

CLI colors
CLI show every n-th notification
feat(webapp): Platform notifications ui/ux improvements
feat(CLI): CLI notifications v1
feat(webapp): add dashboard platform notifications service & UI

Introduce a new server-side service to read and record platform notifications
targeted at the webapp.

- Add payload schema (v1) using zod and typed PayloadV1.
- Define PlatformNotificationWithPayload type and scope priority map.
- Implement getActivePlatformNotifications to:
  - query active WEBAPP notifications with scope/org/project/user filters,
  - include user interactions and validate payloads,
  - filter dismissed items, compute unreadCount, and return sorted results.
- Add helper functions:
  - findInteraction to match global/org interactions,
  - compareNotifications to sort by scope, priority, then recency.
- Implement upsertInteraction to create or update platform notification
  interactions, handling GLOBAL-scoped interactions per organization.

These changes centralize notification read/write logic, enforce payload
validation, and provide deterministic ordering and unread counts for the
webapp UI.
Introduce a new server-side service to read and record platform notifications
targeted at the webapp.

- Add payload schema (v1) using zod and typed PayloadV1.
- Define PlatformNotificationWithPayload type and scope priority map.
- Implement getActivePlatformNotifications to:
  - query active WEBAPP notifications with scope/org/project/user filters,
  - include user interactions and validate payloads,
  - filter dismissed items, compute unreadCount, and return sorted results.
- Add helper functions:
  - findInteraction to match global/org interactions,
  - compareNotifications to sort by scope, priority, then recency.
- Implement upsertInteraction to create or update platform notification
  interactions, handling GLOBAL-scoped interactions per organization.

These changes centralize notification read/write logic, enforce payload
validation, and provide deterministic ordering and unread counts for the
webapp UI.
@0ski 0ski force-pushed the oskar/feat-platform-notifications branch from 3c54860 to e4393ed Compare March 26, 2026 11:44
devin-ai-integration[bot]

This comment was marked as resolved.

@0ski 0ski force-pushed the oskar/feat-platform-notifications branch from e4393ed to 4343887 Compare March 26, 2026 12:05
@0ski 0ski added the ready label Mar 26, 2026
@0ski 0ski force-pushed the oskar/feat-platform-notifications branch from 1e0345e to 75b39e3 Compare March 26, 2026 12:51
Copy link
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 1 new potential issue.

View 19 additional findings in Devin Review.

Open in Devin Review

@0ski 0ski merged commit 8244ac6 into main Mar 26, 2026
40 checks passed
@0ski 0ski deleted the oskar/feat-platform-notifications branch March 26, 2026 13:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants