feat(webapp): self-serve private Slack support channel - #4593
Open
isshaddad wants to merge 28 commits into
Open
Conversation
Adds provisionOrganizationSupportChannel, which creates a private Slack channel for an organization, invites the org owner by email, and records the outcome on OrganizationSupportChannel. Idempotent when a channel already exists (INVITED or LINKED), and records FAILED with the error message when the owner email is missing or Slack calls throw.
…on worker Wires the support-channel provisioning orchestrator into the background job system so callers can enqueue it by organization id, keeping Slack API calls off the request path.
Adds the loader and action for the organization Support settings page, gated so only paid plans can connect a Slack support channel.
Shows the Slack support channel connection state on the organization settings page: an upgrade prompt for free orgs, a connect button for paid orgs without a channel yet, and status for invited, linked, and in-progress channels.
Adds discovery helpers for finding existing customer support Slack channels: isCustomerSupportChannel identifies cus- prefixed Connect channels, and pickExternalTeamId resolves the customer's workspace id from a channel's connected teams. Extends SupportSlackClientLive with ownTeamId, listCustomerChannels, and getTeamDomains so a later step can propose links between organizations and existing channels.
Proposes which organization a discovered cus- Slack channel likely belongs to, scoring on channel/org name similarity and email domain match, so an admin can review and approve links instead of us guessing silently.
Adds linkSupportChannel, which records an admin-approved match between an organization and a Slack support channel. Handles idempotent re-linking, and refuses to overwrite an org's existing link or steal a channel already linked to another org unless explicitly reassigning.
Adds a super-admin page at /admin/slack-channels that lists existing customer Slack Connect channels, proposes an organization match for each using name and email-domain heuristics, and lets an admin approve or reassign the link with one click.
…urface enqueue failures Persist the Slack channel id right after creation instead of only on final invite success, so a redis-worker retry reuses the existing channel instead of hitting Slack's name_taken error and orphaning it. Also let enqueue failures in the settings action surface as an error instead of silently stranding the row at PROVISIONING with no way to retry.
…-upgrade Adds an ARCHIVED status to OrganizationSupportChannel, archive/unarchive methods on the Slack client, and unlinkSupportChannel to disconnect a support channel. Re-provisioning an archived channel unarchives and reuses it instead of creating a new one, avoiding a Slack name_taken error on the cus-<slug> channel name.
…page Admins can now see which orgs kept a linked Slack support channel after downgrading off a paid plan, and unlink a channel directly from the admin page instead of going through a script.
Collapse the release note back to a single entry, add a confirmation to the admin unlink action, and clarify the best-effort team-domain lookup.
…settings layout Switch the org support-channel gate from generic paid-plan status to a data-driven Pro/Enterprise entitlement (v3Subscription.plan.limits.supportChannel), and remove the temporary loader override that had been forcing the paid view locally. Rename isPaying to hasSupportAccess throughout the route to reflect that it is an entitlement check, not a payment check.
… main's The two migrations were authored in July and now sit behind a month of migrations that are already applied, so they would apply out of order. Re-dated to keep their relative order.
Line-wrapping only; oxfmt --check was failing on this file.
The route used a raw loader/action, so provisioning was reachable by any org member. Both now go through the dashboard route builders, with the role check on the action and a disabled button mirroring it in the UI. The plan gate still runs first, so unentitled orgs see the upsell whatever their role.
The owner lookup had no orderBy, so which admin received the Slack Connect invite varied between runs and a retry could email someone else. Orders by createdAt, matching the admin page's lookup.
…page The dropdown defaulted to the first organization in the list when no match was proposed, so a single click on Approve could link the wrong org. Adds a placeholder default, disables the buttons until one is chosen, and rejects the sentinel server-side.
The entitlement gate changed behaviour users never saw, since the feature has not shipped. The remaining note covers it.
|
Contributor
WalkthroughAdded database models and migrations for organization support-channel lifecycle states. Added feature-flag resolution and settings navigation. Added Slack discovery, provisioning, linking, unlinking, reuse, and organization matching services. Added background provisioning with retries. Added plan-aware organization settings and super-admin channel management. Added unit, integration, model, path, and end-to-end tests. Mergeability Score: 🟡 Moderate · up to The provisioning flow can leave an active Slack channel recorded as archived after an invite failure, or become unrecoverable if saving the channel fails immediately after creation. This can strand organizations with unusable or unlinked support channels, so the PR is not merge-ready until retry recovery is corrected. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Contributor
Observability mapAs of 20/100 over 427 measured of 443 entry points (base 20, no change) What this PR changed
FIX FIRST
AUDIT 3 of 50 sensitive mutations record an actor. 47 without one. What the score is made ofThe score and findings here are report-only and never gate the merge. Separately, a required test suite keeps this tool's symbol and route lists in sync with the code they name, and can fail a pull request that renames or removes a symbol they reference, or that adds the first route with a segment they anticipate. Each failure names the list to edit. The rules and their reasons: internal-packages/observability-map/README.md. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
The feature needs a plan entitlement and Slack app scopes that ship separately, so it must stay dark until both are live. Off by default, with a per-organization override so one org can be switched on first. When off the route 404s and the nav item is hidden, rather than showing an upsell for something that cannot be bought yet.
An out-of-band POST would flip a LINKED row back to PROVISIONING and re-send the Slack invite. Redirects instead, and gates both the loader and the action on the feature flag.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
- Persist PROVISIONING before enqueueing, so a fast worker run cannot be clobbered back to PROVISIONING and stick (the job dedupes, so nothing retried). - Mark the row FAILED when the worker refuses to provision, instead of leaving it PROVISIONING forever with no error and no retry. - Classify provisioning failures and rethrow retryable ones, so the configured maxAttempts actually applies. A missing owner or org stays permanent. - Keep the row ARCHIVED when unarchiving fails, so the unarchive is retried rather than falling through to a path that invites into an archived channel. - Recheck the entitlement in the worker; an org can lose it while queued. - Read connected_team_ids from conversations.info — users.conversations does not return it, so the domain matching hint was always empty. - Show the Slack Connect invite while INVITED; the deep link 404s until the owner joins. Drop the duplicated channel name. - Surface action errors, which were returned but never rendered.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/webapp/app/services/supportSlackChannel.server.ts (1)
383-389: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftRecover a channel when the post-create persistence write fails.
If the
setStatuscall aftercreatePrivateChannelfails, the catch block recordsFAILEDwithoutslackChannelId. A retry then callscreatePrivateChannelagain with the same name, receives Slack’sname_takenerror, and cannot recover. Add channel lookup by canonical name, persist the recovered ID, continue the invitation, and add a regression test for this persistence failure.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: ed1acd73-e6b5-4cec-baaa-5224c54c05fa
📒 Files selected for processing (4)
apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.support.tsxapps/webapp/app/services/supportSlackChannel.server.tsapps/webapp/app/v3/commonWorker.server.tsapps/webapp/test/supportSlackChannel.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/webapp/test/supportSlackChannel.test.ts
- apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.support.tsx
📜 Review details
⏰ Context from checks skipped due to timeout. (23)
- GitHub Check: report
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (12, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (11, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (7, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (6, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (1, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (9, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (5, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (10, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (2, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (3, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (4, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (8, 12)
- GitHub Check: internal / 🧪 Unit Tests: Internal
- GitHub Check: obsmap / 🧪 Unit Tests: Observability Map
- GitHub Check: e2e-webapp / 🧪 E2E Tests: Webapp
- GitHub Check: typecheck / typecheck
- GitHub Check: runops-guard / runops-guard
- GitHub Check: code-quality / code-quality
- GitHub Check: audit
- GitHub Check: 🛡️ E2E Auth Tests (full)
- GitHub Check: audit
- 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}: Prefer static imports over dynamicimport(); use dynamic imports only for unresolvable circular dependencies, genuine performance code splitting, or conditional runtime loading.
Import Trigger.dev tasks from@trigger.dev/sdk; never use@trigger.dev/sdk/v3or deprecatedclient.defineJob.
Add agentcrumbs while writing code using approved namespaces; mark lines with//@Crumbsor blocks with `// `#region` `@crumbs, and strip them before merging.
Files:
apps/webapp/app/v3/commonWorker.server.tsapps/webapp/app/services/supportSlackChannel.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/v3/commonWorker.server.tsapps/webapp/app/services/supportSlackChannel.server.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Files:
apps/webapp/app/v3/commonWorker.server.tsapps/webapp/app/services/supportSlackChannel.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/v3/commonWorker.server.tsapps/webapp/app/services/supportSlackChannel.server.ts
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: Access environment variables through theenvexport ofenv.server.tsinstead of directly accessingprocess.env
Use subpath exports from@trigger.dev/corepackage instead of importing from the root@trigger.dev/corepathDo not reintroduce the removed v1 execution path;
RunEngineVersion.V1branches may only reject or finalize gracefully so v3 clients receive a clean 4xx, never a 5xx.
Files:
apps/webapp/app/v3/commonWorker.server.tsapps/webapp/app/services/supportSlackChannel.server.ts
apps/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
For apps, use
typecheckfor verification and never usebuildas the correctness check.
Files:
apps/webapp/app/v3/commonWorker.server.tsapps/webapp/app/services/supportSlackChannel.server.ts
apps/webapp/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
apps/webapp/app/**/*.{ts,tsx}: For dashboard changes, visually verify the running Remix app with Chrome DevTools MCP, using snapshots, screenshots, interaction, and console-message checks as appropriate.
UseuseCallbackanduseMemoonly for context provider values, expensive derived data used as a dependency, or stable references required by dependency arrays; do not wrap ordinary event handlers or trivial computations.
Use named constants for sentinel or placeholder values instead of scattering raw string literals across comparisons.
Files:
apps/webapp/app/v3/commonWorker.server.tsapps/webapp/app/services/supportSlackChannel.server.ts
apps/webapp/app/**/*.ts
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
apps/webapp/app/**/*.ts: Never userequest.signalto detect client disconnects. UsegetRequestAbortSignal()fromapp/services/httpAsyncStorage.server.ts, which is wired to Express response close events.
Access environment variables through theenvexport fromapp/env.server.ts; never useprocess.envdirectly.
Always use PrismafindFirstinstead offindUnique.
Always use the$transactionhelper from~/db.server, never callprisma.$transactionor$replica.$transactiondirectly. Pass isolation levels as strings, useSerializablefor correctness-critical read-then-write invariants, and guard possibly undefined helper results when a definite value is required.
Files:
apps/webapp/app/v3/commonWorker.server.tsapps/webapp/app/services/supportSlackChannel.server.ts
apps/webapp/app/v3/**/*.ts
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
New code must target Run Engine V2 through the singleton in
app/v3/runEngine.server.ts; do not reintroduce V1 execution paths. V1 branches may only reject or finalize gracefully with a clean 4xx.
Files:
apps/webapp/app/v3/commonWorker.server.ts
apps/webapp/app/v3/{commonWorker,alertsWorker,batchTriggerWorker}.server.ts
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
Background job workers must use
@trigger.dev/redis-worker.
Files:
apps/webapp/app/v3/commonWorker.server.ts
🧠 Learnings (47)
📚 Learning: 2026-08-12T06:32:26.470Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 4577
File: apps/webapp/app/v3/services/createBackgroundWorker.server.ts:779-781
Timestamp: 2026-08-12T06:32:26.470Z
Learning: In `apps/webapp/app/v3/services/createBackgroundWorker.server.ts`, `syncDeclarativeSchedules` has a pre-existing non-serialized read/modify/delete reconciliation flow. Changes that reuse its initial schedule snapshot instead of a redundant re-fetch do not introduce this concurrency risk. Track reconciliation serialization separately from read-side performance changes unless a change modifies the synchronization boundary.
Applied to files:
apps/webapp/app/v3/commonWorker.server.ts
📚 Learning: 2026-07-15T18:37:08.044Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: apps/webapp/CLAUDE.md:0-0
Timestamp: 2026-07-15T18:37:08.044Z
Learning: Applies to apps/webapp/app/v3/{commonWorker,alertsWorker,batchTriggerWorker}.server.ts : Background job workers must use `trigger.dev/redis-worker`.
Applied to files:
apps/webapp/app/v3/commonWorker.server.ts
📚 Learning: 2026-08-08T08:33:56.975Z
Learnt from: kathiekiwi
Repo: triggerdotdev/trigger.dev PR: 4525
File: apps/webapp/app/v3/services/alerts/deliverDashboardAgentWatchAlert.server.ts:500-506
Timestamp: 2026-08-08T08:33:56.975Z
Learning: In `apps/webapp/app/v3/services/alerts/deliverDashboardAgentWatchAlert.server.ts` and `apps/webapp/app/v3/services/alerts/deliverErrorGroupAlert.server.ts`, the Slack error predicates are intentionally kept byte-for-byte identical. Any hardening change to these predicates must update both alert deliverers in a separate pull request, rather than diverging one feature pull request.
Applied to files:
apps/webapp/app/v3/commonWorker.server.tsapps/webapp/app/services/supportSlackChannel.server.ts
📚 Learning: 2026-07-13T14:51:32.938Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: packages/redis-worker/CLAUDE.md:0-0
Timestamp: 2026-07-13T14:51:32.938Z
Learning: Applies to packages/redis-worker/**/*.{ts,tsx} : Use `trigger.dev/redis-worker` for all background jobs in the webapp.
Applied to files:
apps/webapp/app/v3/commonWorker.server.ts
📚 Learning: 2026-07-15T18:37:08.044Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: apps/webapp/CLAUDE.md:0-0
Timestamp: 2026-07-15T18:37:08.044Z
Learning: Applies to apps/webapp/app/v3/services/{triggerTask,batchTriggerV3}.server.ts : Do not add database queries to `triggerTask.server.ts` or `batchTriggerV3.server.ts`; reuse the existing queue query and preserve the two-stage trigger-time/dequeue-time resolution pattern. Add task defaults to the existing `select` clause or resolve them at dequeue time.
Applied to files:
apps/webapp/app/v3/commonWorker.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/**/* : Bundle worker code using the build system in `src/build/` based on configuration from `trigger.config.ts`
Applied to files:
apps/webapp/app/v3/commonWorker.server.ts
📚 Learning: 2026-04-13T21:44:00.032Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3368
File: apps/webapp/app/services/taskIdentifierRegistry.server.ts:24-67
Timestamp: 2026-04-13T21:44:00.032Z
Learning: In `apps/webapp/app/services/taskIdentifierRegistry.server.ts`, the sequential upsert/updateMany/findMany writes in `syncTaskIdentifiers` are intentionally NOT wrapped in a Prisma transaction. This function runs only during deployment-change events (low-concurrency path), and any partial `isInLatestDeployment` state is acceptable because it self-corrects on the next deployment. Do not flag this as a missing-transaction/atomicity issue in future reviews.
Applied to files:
apps/webapp/app/v3/commonWorker.server.ts
📚 Learning: 2026-07-13T14:51:32.938Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: packages/redis-worker/CLAUDE.md:0-0
Timestamp: 2026-07-13T14:51:32.938Z
Learning: Applies to packages/redis-worker/src/worker.ts : Implement the Redis worker loop and job processing with concurrency control in `src/worker.ts`.
Applied to files:
apps/webapp/app/v3/commonWorker.server.ts
📚 Learning: 2026-05-15T08:05:57.683Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3625
File: apps/webapp/app/services/taskMetadataCache.server.ts:270-291
Timestamp: 2026-05-15T08:05:57.683Z
Learning: In the triggerdotdev/trigger.dev codebase, `populateByCurrentWorker()` in `apps/webapp/app/services/taskMetadataCache.server.ts` intentionally logs and swallows Redis errors rather than rethrowing. The design rationale: rethrowing would propagate into `ChangeCurrentDeploymentService.call` and break deploy promotion when Redis is briefly unavailable; the 24h `TASK_META_CACHE_CURRENT_ENV_TTL_SECONDS` TTL acts as the self-healing window for cache drift, and next-promotion overwrites the env key sooner in practice. A compensating DEL on failure is also not a win because if Redis is unreachable the DEL fails identically, and Lua scripts are atomic so a partial write is impossible. Do not flag this log+swallow pattern as a bug in future reviews.
Applied to files:
apps/webapp/app/v3/commonWorker.server.tsapps/webapp/app/services/supportSlackChannel.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/v3/commonWorker.server.ts
📚 Learning: 2026-05-01T15:44:50.087Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3499
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.roles/route.tsx:170-173
Timestamp: 2026-05-01T15:44:50.087Z
Learning: In `apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.roles/route.tsx`, the `CreateRoleUpsell` component is intentionally only rendered for non-enterprise plans (`!isEnterprise`). Enterprise plans intentionally have no "Create role" CTA because the create-role flow is not yet built. A real Create button will be added in a separate ticket alongside the create-role action wiring. Do not flag the missing enterprise create-role entry point as a bug until that ticket lands.
Applied to files:
apps/webapp/app/v3/commonWorker.server.ts
📚 Learning: 2026-08-11T12:52:37.785Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 4540
File: packages/redis-worker/src/fair-queue/index.ts:0-0
Timestamp: 2026-08-11T12:52:37.785Z
Learning: In `packages/redis-worker/src/fair-queue/index.ts`, the only current FairQueue consumer sets `deadLetterQueue: false` when retry is configured. Do not recommend a DLQ write failure path that leaves a terminally failed message in-flight for reclaim unless requeueing increments the stored message `attempt`; otherwise external FairQueue consumers can enter an uncapped retry loop.
Applied to files:
apps/webapp/app/v3/commonWorker.server.tsapps/webapp/app/services/supportSlackChannel.server.ts
📚 Learning: 2026-05-12T06:43:12.346Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3558
File: internal-packages/run-engine/src/run-queue/index.ts:420-424
Timestamp: 2026-05-12T06:43:12.346Z
Learning: In the triggerdotdev/trigger.dev codebase (`internal-packages/run-engine/src/run-queue/index.ts`), the established convention in `RunQueue` read-path methods (e.g., `lengthOfQueue`, `lengthOfQueues`, `currentConcurrencyOfQueues`) is to **fail open** on transient Redis pipeline errors: pipeline result errors (`baseErr`, `ctrErr`, etc.) are coerced to `0` rather than surfaced or re-thrown. This is intentional — the project treats Redis command errors the same as missing keys for these counter reads. Do not flag this pattern as a bug or suggest throwing/propagating these errors in future reviews.
Applied to files:
apps/webapp/app/v3/commonWorker.server.tsapps/webapp/app/services/supportSlackChannel.server.ts
📚 Learning: 2026-04-07T14:12:59.018Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3331
File: apps/webapp/app/runEngine/concerns/batchPayloads.server.ts:112-136
Timestamp: 2026-04-07T14:12:59.018Z
Learning: In `apps/webapp/app/runEngine/concerns/batchPayloads.server.ts`, the `pRetry` call wrapping `uploadPacketToObjectStore` intentionally retries **all** error types (no `shouldRetry` filter / `AbortError` guards). The maintainer explicitly prefers over-retrying to under-retrying because multiple heterogeneous object store backends are supported and it is impractical to enumerate all permanent error signatures. Do not flag this as an issue in future reviews.
Applied to files:
apps/webapp/app/v3/commonWorker.server.tsapps/webapp/app/services/supportSlackChannel.server.ts
📚 Learning: 2026-06-01T15:01:35.175Z
Learnt from: d-cs
Repo: triggerdotdev/trigger.dev PR: 3756
File: apps/webapp/app/v3/services/resetIdempotencyKey.server.ts:65-94
Timestamp: 2026-06-01T15:01:35.175Z
Learning: In `apps/webapp/app/v3/services/resetIdempotencyKey.server.ts` (triggerdotdev/trigger.dev), a transient `buffer.resetIdempotency()` failure when `pgCount > 0` does NOT warrant a 503 and should return success. The mollifier `ack` and `fail` Lua scripts always DEL the idempotency lookup key as part of the run's natural lifecycle (drain→ack or terminal→fail or cancel-bifurcation), so stale buffered idempotency lookups converge automatically without caller retries. Only when `pgCount === 0 && bufferResetFailed` is a 503 appropriate, because then the run's existence is genuinely unobservable (the buffer outage hides a potentially matching buffered run). The test "returns success when PG cleared >=1 run, even if the buffer reset throws" documents this contract explicitly.
Applied to files:
apps/webapp/app/v3/commonWorker.server.ts
📚 Learning: 2026-05-08T09:27:50.797Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3529
File: packages/cli-v3/src/executions/taskRunProcess.ts:216-220
Timestamp: 2026-05-08T09:27:50.797Z
Learning: In triggerdotdev/trigger.dev (`packages/cli-v3/src/executions/taskRunProcess.ts`), stale `_currentExecution` / `_isPreparedForNextAttempt` after an error-path rejection (e.g. `#rejectPendingAttempts`) is benign: `#gracefullyTerminate` immediately calls `kill()`, which synchronously sets `_isBeingKilled = true`, and the `isHealthy` getter returns `false` whenever `isBeingKilled` is true — preventing any caller from reusing the process. Both known callers (`dev-run-controller.ts` ~lines 516-567 and `execution.ts` ~lines 538-591) also handle the error and discard the process instance. Do not flag missing cleanup of these fields on error paths in this class.
Applied to files:
apps/webapp/app/v3/commonWorker.server.ts
📚 Learning: 2026-03-26T17:27:09.938Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 3264
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.private-connections._index/route.tsx:176-176
Timestamp: 2026-03-26T17:27:09.938Z
Learning: In `apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.private-connections._index/route.tsx`, the variable `hasPrivateNetworking` is intentionally hardcoded to `true` as a placeholder. Plan-level gating will be wired to actual billing data (e.g., `plan?.v3Subscription?.plan?.limits?.hasPrivateNetworking`) once the billing integration is complete. The route is already guarded at the feature-flag level via `hasPrivateConnections` in the loader. Do not flag this hardcoded value as dead code or a bug until the billing integration is in place.
Applied to files:
apps/webapp/app/v3/commonWorker.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/v3/commonWorker.server.ts
📚 Learning: 2026-06-17T17:13:49.929Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3948
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.bulk-actions.$bulkActionParam/route.tsx:48-62
Timestamp: 2026-06-17T17:13:49.929Z
Learning: In triggerdotdev/trigger.dev, within `dashboardLoader`/`dashboardAction` (or similar context resolver code) whenever you resolve an organization ID from an organization slug for RBAC/enterprise authorization scope, always read from the primary Prisma client (`prisma`), not `$replica`. Using `$replica` can hit replica-lag and cause the RBAC lookup/authorization to run without the correct org scope (bypassing intended role enforcement). Implement the slug→org lookup with `prisma.organization.findFirst(...)` (or equivalent primary-client query) and add an inline comment documenting why the primary client is required (replica lag could lead to unscoped RBAC checks).
Applied to files:
apps/webapp/app/v3/commonWorker.server.tsapps/webapp/app/services/supportSlackChannel.server.ts
📚 Learning: 2026-08-12T07:30:40.534Z
Learnt from: kathiekiwi
Repo: triggerdotdev/trigger.dev PR: 4525
File: internal-packages/dashboard-agent-db/src/watch-queries.ts:794-808
Timestamp: 2026-08-12T07:30:40.534Z
Learning: In triggerdotdev/trigger.dev dashboard-agent watches, `softDeleteChatsForOrganization()` does not serialize with `createWatch()` through the per-chat advisory lock. A create-versus-organization-delete race can temporarily leave an active watch on a deleted chat. This behavior existed before the Watch feature and is accepted as a bounded, rare administrative-path condition: the next watch tick calls `authorizeWatchEnvironment`, cancels the watch with `access_lost` after organization deletion, and chat hard deletion removes residual watch rows. Per-chat or organization-level locking and a concurrent regression test belong to the dedicated organization-deletion durability follow-up.
Applied to files:
apps/webapp/app/v3/commonWorker.server.tsapps/webapp/app/services/supportSlackChannel.server.ts
📚 Learning: 2026-05-04T19:14:41.044Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3499
File: apps/webapp/app/models/member.server.ts:230-245
Timestamp: 2026-05-04T19:14:41.044Z
Learning: In `apps/webapp/app/models/member.server.ts`, the `acceptInvite` function intentionally logs (but does not surface) `rbac.setUserRole` failures. This is safe in v1 because: (1) "Member" is the only plan-gated role and is hidden in v1, and (2) for the exposed roles (Owner/Admin/Developer), any `setUserRole` failure causes the runtime to fall through to the legacy `OrgMember.role → Developer` mapping. Compensating cleanup should be added when the Member role is re-enabled alongside env-tier route wiring (tracked in TRI-9018/9019/9020).
Applied to files:
apps/webapp/app/v3/commonWorker.server.tsapps/webapp/app/services/supportSlackChannel.server.ts
📚 Learning: 2026-08-11T10:53:25.039Z
Learnt from: kathiekiwi
Repo: triggerdotdev/trigger.dev PR: 4552
File: apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dashboard-agent.ts:158-165
Timestamp: 2026-08-11T10:53:25.039Z
Learning: For the dashboard agent message quota rollout, `apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dashboard-agent.ts` intentionally fails open when the cloud `agentMessages` plan limit is unavailable. During this transition, `apps/webapp/app/components/dashboard-agent/useAgentMessageQuota.ts` applies the client-side free-plan upgrade nudge because it can identify a cloud free plan. Server-provided quota-limit propagation is planned in TRI-12863.
Applied to files:
apps/webapp/app/v3/commonWorker.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/v3/commonWorker.server.tsapps/webapp/app/services/supportSlackChannel.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/v3/commonWorker.server.tsapps/webapp/app/services/supportSlackChannel.server.ts
📚 Learning: 2026-05-18T08:21:27.694Z
Learnt from: d-cs
Repo: triggerdotdev/trigger.dev PR: 3632
File: apps/webapp/sentry.server.ts:4-21
Timestamp: 2026-05-18T08:21:27.694Z
Learning: When handling Prisma error P1001 ("Can't reach database server") in TypeScript, don’t assume a single error shape. Prisma can surface P1001 via two different error classes/fields: `PrismaClientKnownRequestError` exposes it as `err.code === "P1001"` (common during mid-query connection drops), while `PrismaClientInitializationError` exposes it as `err.errorCode === "P1001"` (common on client startup failure). Therefore, predicates should use `err.code === "P1001" || err.errorCode === "P1001"`. Do not flag `err.code === "P1001"` as “unreachable/never matches,” as it is expected in production.
Applied to files:
apps/webapp/app/v3/commonWorker.server.tsapps/webapp/app/services/supportSlackChannel.server.ts
📚 Learning: 2026-05-18T08:21:27.694Z
Learnt from: d-cs
Repo: triggerdotdev/trigger.dev PR: 3632
File: apps/webapp/sentry.server.ts:4-21
Timestamp: 2026-05-18T08:21:27.694Z
Learning: When handling Prisma errors for P1001 ("Can't reach database server"), do not assume it only appears under a single property name. Prisma may surface P1001 via either `PrismaClientKnownRequestError` (`err.code === "P1001"`, e.g., mid-query connection drops) or `PrismaClientInitializationError` (`err.errorCode === "P1001"`, e.g., client startup connection failure). To reliably detect the condition, check `err.code === "P1001" || err.errorCode === "P1001"`, and avoid review rules that would incorrectly flag `err.code === "P1001"` as unreachable/never-matching.
Applied to files:
apps/webapp/app/v3/commonWorker.server.tsapps/webapp/app/services/supportSlackChannel.server.ts
📚 Learning: 2026-06-13T19:53:13.759Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3937
File: packages/trigger-sdk/skills/realtime-and-frontend/SKILL.md:258-260
Timestamp: 2026-06-13T19:53:13.759Z
Learning: When reviewing code that uses `trigger.dev/react-hooks`’s `useRealtimeRun`, preserve the call signature where the first argument is the full realtime handle object (not `handle.id`). This is intentional to maintain type-safety and is consistent with the official docs; do not suggest changing the first argument from the handle object to `handle.id`.
Applied to files:
apps/webapp/app/v3/commonWorker.server.tsapps/webapp/app/services/supportSlackChannel.server.ts
📚 Learning: 2026-06-23T13:04:21.413Z
Learnt from: carderne
Repo: triggerdotdev/trigger.dev PR: 4023
File: apps/webapp/app/services/upsertBranch.server.ts:14-18
Timestamp: 2026-06-23T13:04:21.413Z
Learning: In TypeScript, it’s valid to `import { type X }` and then use `typeof X` in a type-only position, e.g. `type Alias = z.infer<typeof X>`. The `type` modifier suppresses the runtime import, but the type checker still has the full exported type so `z.infer<typeof X>` can resolve correctly. In code reviews, don’t flag this as a TypeScript compile error as long as `typeof X` is used in a type context (e.g., with `z.infer`, `type` aliases, generics), not as a runtime value.
Applied to files:
apps/webapp/app/v3/commonWorker.server.tsapps/webapp/app/services/supportSlackChannel.server.ts
📚 Learning: 2026-03-29T19:16:28.864Z
Learnt from: nicktrn
Repo: triggerdotdev/trigger.dev PR: 3291
File: apps/webapp/app/v3/featureFlags.ts:53-65
Timestamp: 2026-03-29T19:16:28.864Z
Learning: When reviewing TypeScript code that uses Zod v3, treat `z.coerce.*()` schemas as their direct Zod type (e.g., `z.coerce.boolean()` returns a `ZodBoolean` with `_def.typeName === "ZodBoolean"`) rather than a `ZodEffects`. Only `.preprocess()`, `.refine()`/`.superRefine()`, and `.transform()` are expected to wrap schemas in `ZodEffects`. Therefore, in reviewers’ logic like `getFlagControlType`, do not flag/unblock failures that require unwrapping `ZodEffects` when the input schema is a `z.coerce.*` schema.
Applied to files:
apps/webapp/app/v3/commonWorker.server.ts
📚 Learning: 2026-06-09T16:27:26.195Z
Learnt from: myftija
Repo: triggerdotdev/trigger.dev PR: 3878
File: apps/webapp/app/v3/services/computeTemplateCreation.server.ts:0-0
Timestamp: 2026-06-09T16:27:26.195Z
Learning: When working in triggerdotdev/trigger.dev code related to worker-group/region default resolution (e.g., defaultWorkerInstanceGroupId handling used by getGlobalDefaultWorkerGroup, getDefaultWorkerGroupForProject, and RegionsPresenter), do NOT add org-level featureFlags overrides in only one resolution site. That can cause template creation routing/decisions to diverge from actual run routing. If org-level override of the default region/worker group is required, it must be centralized in getGlobalDefaultWorkerGroup so every resolution path remains aligned.
Applied to files:
apps/webapp/app/v3/commonWorker.server.ts
📚 Learning: 2026-05-05T09:38:02.512Z
Learnt from: d-cs
Repo: triggerdotdev/trigger.dev PR: 3523
File: apps/webapp/app/routes/api.v3.batches.ts:178-181
Timestamp: 2026-05-05T09:38:02.512Z
Learning: When reviewing code that catches `ServiceValidationError` in `*.server.ts` files, do not blindly forward `error.status` to HTTP responses, because SVEs may be thrown with non-default statuses (e.g., 400/500) and forwarding them can cause client-visible behavioral regressions (e.g., surfacing 500s to clients). Prefer a safe default response status of `error.status ?? 422`, but only after confirming via the reachable call graph that the caught `ServiceValidationError` instances are expected to carry those non-default statuses; otherwise, normalize to `422` to avoid unexpected client-visible 5xx behavior.
Applied to files:
apps/webapp/app/v3/commonWorker.server.tsapps/webapp/app/services/supportSlackChannel.server.ts
📚 Learning: 2026-05-12T21:04:05.815Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3542
File: apps/webapp/app/components/sessions/v1/SessionStatus.tsx:1-3
Timestamp: 2026-05-12T21:04:05.815Z
Learning: In this Remix + TypeScript codebase, do not flag a server/client boundary violation when a file imports only types from a module matching `*.server`.
Specifically, it’s safe to import types using `import type { Foo } from "*.server"` or `import { type Foo } from "*.server"` because TypeScript erases type-only imports at compile time and they emit no JavaScript, so they won’t cross the Remix server/client bundle boundary.
Only raise the boundary concern for value imports (e.g., `import { Foo }` without `type`, or `import Foo`), since those produce JavaScript output.
Applied to files:
apps/webapp/app/v3/commonWorker.server.tsapps/webapp/app/services/supportSlackChannel.server.ts
📚 Learning: 2026-06-25T18:21:51.905Z
Learnt from: carderne
Repo: triggerdotdev/trigger.dev PR: 4039
File: apps/webapp/app/routes/invite-revoke.tsx:0-0
Timestamp: 2026-06-25T18:21:51.905Z
Learning: During the Zod v4 migration in the triggerdotdev/trigger.dev webapp, ensure any imports from `conform-to/zod` use the Zod-4 subpath: `conform-to/zod/v4` (e.g., `import { parseWithZod } from "conform-to/zod/v4"`). Do not import from the package root `conform-to/zod`, because it is the Zod 3 implementation and may load Zod-3-only symbols (e.g., `ZodBranded`, `ZodEffects`), which can throw at module load (notably with `zod4.4.3`). This should be enforced across `apps/webapp/**/*` where helpers like `parseWithZod` and `conformZodMessage` are used.
Applied to files:
apps/webapp/app/v3/commonWorker.server.tsapps/webapp/app/services/supportSlackChannel.server.ts
📚 Learning: 2026-07-03T17:10:21.498Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 4148
File: apps/webapp/app/models/orgMember.server.ts:149-168
Timestamp: 2026-07-03T17:10:21.498Z
Learning: In triggerdotdev/trigger.dev, `User.email` (Prisma schema: `internal-packages/database/prisma/schema.prisma`) currently does NOT use `citext` and does NOT have a `lower(email)` functional unique index. Therefore, do not introduce Prisma queries like `where: { email: { equals: <value>, mode: "insensitive" } }` (or any case-insensitive lookup) against `User.email`, because it can force sequential scans of the `users` table under load. During review, ensure email is normalized (e.g., lowercased/trimmed) before both writes and subsequent lookups, and if true case-insensitive behavior/uniqueness is required, implement it via a separate app-wide migration (e.g., switch to `citext` and/or add a functional unique index with backfill) rather than bolting it onto individual feature PRs.
Applied to files:
apps/webapp/app/v3/commonWorker.server.tsapps/webapp/app/services/supportSlackChannel.server.ts
📚 Learning: 2026-05-14T08:21:07.614Z
Learnt from: d-cs
Repo: triggerdotdev/trigger.dev PR: 3614
File: apps/webapp/app/v3/mollifier/mollifierGate.server.ts:48-52
Timestamp: 2026-05-14T08:21:07.614Z
Learning: When using Trigger.dev v3 feature flags in the webapp, prefer the existing per-org gating mechanism supported by `flag()` via the `overrides` argument. Pass `Organization.featureFlags` (from `environment.organization.featureFlags`) as the `overrides` value; overrides must take precedence over the global `featureFlag` row. Do not require schema changes or add an `orgId` field to `FlagsOptions` for per-org gating—use the overrides pattern consistently (e.g., in gate flows like `resolveOrgFlag` and any server code that threads `environment.organization.featureFlags` into the gate call).
Applied to files:
apps/webapp/app/v3/commonWorker.server.ts
📚 Learning: 2026-06-04T18:16:35.386Z
Learnt from: nicktrn
Repo: triggerdotdev/trigger.dev PR: 3836
File: apps/supervisor/src/backpressure/backpressureMonitor.ts:3-5
Timestamp: 2026-06-04T18:16:35.386Z
Learning: When reviewing TypeScript in this repo, apply the rule “prefer type aliases over interfaces” only to data/object shapes and union/intersection type modeling. If an interface is being used as a behavioral contract for collaborators to implement (e.g., method-shape interfaces that define required behavior, such as `BackpressureLogger` / `BackpressureSignalSource` in `apps/supervisor/src/backpressure/backpressureMonitor.ts`), keep it as an `interface` and do not flag it as a type-alias-vs-interface violation.
Applied to files:
apps/webapp/app/v3/commonWorker.server.tsapps/webapp/app/services/supportSlackChannel.server.ts
📚 Learning: 2026-06-09T17:58:04.699Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 3879
File: apps/webapp/app/models/vercelIntegration.server.ts:619-630
Timestamp: 2026-06-09T17:58:04.699Z
Learning: In this codebase, outbound raw `fetch` calls should typically rely on Node/undici’s default request timeout (about ~300s) rather than adding a per-call `AbortController` + `setTimeout` wrapper inside individual functions (e.g. in files like `apps/webapp/app/models/vercelIntegration.server.ts`). During code review, do not flag the absence of a per-call timeout on a single `fetch` as an issue; if per-call timeouts are needed, they should be implemented via a codebase-wide convention (e.g., a shared fetch wrapper or documented pattern) rather than ad-hoc per-function changes.
Applied to files:
apps/webapp/app/v3/commonWorker.server.tsapps/webapp/app/services/supportSlackChannel.server.ts
📚 Learning: 2026-06-06T19:34:46.107Z
Learnt from: nicktrn
Repo: triggerdotdev/trigger.dev PR: 3855
File: internal-packages/testcontainers/src/minio.ts:125-136
Timestamp: 2026-06-06T19:34:46.107Z
Learning: In `internal-packages/testcontainers/src/minio.ts` (triggerdotdev/trigger.dev), the `resetBucket()` method on `StartedMinIOContainer` intentionally uses `throwOnError: false` for the `mc rm --recursive --force local/<bucket>` step. On the first reset the bucket may be empty (or absent), causing `mc rm` to exit non-zero; swallowing that error is correct because the subsequent `mc mb --ignore-existing` call (with `throwOnError: true`) provides the hard guarantee that the bucket exists and is empty. Do not flag this `throwOnError: false` pattern as a bug in future reviews.
Applied to files:
apps/webapp/app/services/supportSlackChannel.server.ts
📚 Learning: 2026-05-04T19:14:44.097Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3499
File: apps/webapp/test/auth-api.e2e.full.test.ts:205-227
Timestamp: 2026-05-04T19:14:44.097Z
Learning: In triggerdotdev/trigger.dev's e2e auth test suite (`apps/webapp/test/auth-api.e2e.full.test.ts` and related `*.e2e.full.test.ts` files), loose negative assertions like `expect(res.status).not.toBe(200)` are intentional. External infrastructure (e.g. ClickHouse) is unreachable in the e2e test environment, so a 5xx from the route handler after auth passes is an expected and acceptable outcome. Tightening these to a specific set like `[401, 403, 404]` would incorrectly exclude valid 5xx results. Do not flag these as issues during review.
Applied to files:
apps/webapp/app/services/supportSlackChannel.server.ts
📚 Learning: 2026-03-26T10:02:25.354Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 3254
File: apps/webapp/app/services/platformNotifications.server.ts:363-385
Timestamp: 2026-03-26T10:02:25.354Z
Learning: In `triggerdotdev/trigger.dev`, the `getNextCliNotification` fallback in `apps/webapp/app/services/platformNotifications.server.ts` intentionally uses `prisma.orgMember.findFirst` (single org) when no `projectRef` is provided. This is acceptable for v1 because the CLI (`dev` and `login` commands) always passes `projectRef` in normal usage, making the fallback a rare edge case. Do not flag the single-org fallback as a multi-org correctness bug in this file.
Applied to files:
apps/webapp/app/services/supportSlackChannel.server.ts
📚 Learning: 2026-05-12T21:04:06.206Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3542
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.sessions._index/route.tsx:40-42
Timestamp: 2026-05-12T21:04:06.206Z
Learning: In triggerdotdev/trigger.dev, the established codebase convention for handling a missing/unauthorized environment in Remix route loaders (e.g., when `findEnvironmentBySlug` returns falsy) is `throw new Error("Environment not found")`. This pattern is used consistently across routes such as `batches`, `runs._index`, and others. Do not flag it as a missing 404 Response; a 404-Response convention change would be a cross-cutting refactor and is intentionally out of scope for individual PRs.
Applied to files:
apps/webapp/app/services/supportSlackChannel.server.ts
📚 Learning: 2026-06-16T13:14:20.816Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3964
File: docs/ai-chat/reference.mdx:478-478
Timestamp: 2026-06-16T13:14:20.816Z
Learning: In the triggerdotdev/trigger.dev repo, the RC-gated `ai-chat` docs section (under `docs/ai-chat/`) is often updated in companion docs PRs that are meant to land alongside implementation PRs (e.g., PR `#3963` adds `handover: { isFinal: boolean } | null` to the exported `ChatTurn` type in `packages/trigger-sdk/src/v3/ai.ts`). When searching for SDK types referenced in these docs PRs, the field may not exist on the docs branch yet — check the companion implementation PR before flagging missing implementations.
Applied to files:
apps/webapp/app/services/supportSlackChannel.server.ts
📚 Learning: 2026-06-13T19:52:11.462Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3936
File: packages/trigger-sdk/src/v3/ai.ts:969-977
Timestamp: 2026-06-13T19:52:11.462Z
Learning: In `triggerdotdev/trigger.dev`, within `packages/trigger-sdk/src/v3/ai.ts`, `sessions.open(payload.chatId)` currently returns a `SessionHandle` whose `id` equals the external `payload.chatId` for chat agent/custom-agent paths. However, `ToolCallExecutionOptions.chatId` and `ai.chatContext()` should be treated as carrying the external chat ID contract, so code should prefer the boot payload value stored in `chatExternalIdKey` rather than relying on `SessionHandle.id` semantics.
Applied to files:
apps/webapp/app/services/supportSlackChannel.server.ts
📚 Learning: 2026-07-11T08:52:32.250Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 4234
File: apps/webapp/app/routes/realtime.v1.sessions.$session.$io.append.ts:159-164
Timestamp: 2026-07-11T08:52:32.250Z
Learning: In `apps/webapp/app/routes/realtime.v1.sessions.$session.$io.append.ts`, when the idempotency claim via `claimSessionStreamPart()` is lost (i.e., `wonClaim` is false, indicating a duplicate/retried append with the same client-supplied part id), the response omits `seq`, so the SDK's `appendInputChunk()` falls back to the legacy no-baseline behavior for turn-complete correlation in `packages/trigger-sdk/src/v3/chat.ts`. This is accepted as a known, narrow limitation (not a regression) rather than being fixed immediately: `claimSessionStreamPart()` currently only returns a boolean and cannot recover the already-committed seq. The tracked follow-up fix is to have the winner record the committed seq in the dedupe key so the loser can read it back on a lost claim.
Applied to files:
apps/webapp/app/services/supportSlackChannel.server.ts
📚 Learning: 2026-05-19T21:04:52.902Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3671
File: packages/trigger-sdk/src/v3/ai.ts:724-769
Timestamp: 2026-05-19T21:04:52.902Z
Learning: Repo: triggerdotdev/trigger.dev — In packages/trigger-sdk/src/v3/ai.ts, recovery now preserves per-message clientData: replaySessionInTail returns { message, metadata, seqNum } for each session.in record, and the recovery boot queue maps metadata by message.id to reconstruct recovered turns. For messages without a matching record (e.g., hook-synthesized), it intentionally falls back to the current boot payload’s metadata. Do not flag “lost metadata on recovered turns” in future reviews when this path is in use.
Applied to files:
apps/webapp/app/services/supportSlackChannel.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/supportSlackChannel.server.ts
📚 Learning: 2026-03-26T09:02:07.973Z
Learnt from: myftija
Repo: triggerdotdev/trigger.dev PR: 3274
File: apps/webapp/app/services/runsReplicationService.server.ts:922-924
Timestamp: 2026-03-26T09:02:07.973Z
Learning: When parsing Trigger.dev task run annotations in server-side services, keep `TaskRun.annotations` strictly conforming to the `RunAnnotations` schema from `trigger.dev/core/v3`. If the code already uses `RunAnnotations.safeParse` (e.g., in a `#parseAnnotations` helper), treat that as intentional/necessary for atomic, schema-accurate annotation handling. Do not recommend relaxing the annotation payload schema or using a permissive “passthrough” parse path, since the annotations are expected to be written atomically in one operation and should not contain partial/legacy payloads that would require a looser parser.
Applied to files:
apps/webapp/app/services/supportSlackChannel.server.ts
🔇 Additional comments (1)
apps/webapp/app/v3/commonWorker.server.ts (1)
202-208: LGTM!Also applies to: 337-380
Comment on lines
+347
to
+356
| // Stay ARCHIVED rather than dropping to FAILED: the channel is still | ||
| // archived in Slack, and only this branch unarchives it. Marking it FAILED | ||
| // would send the next attempt down the reuse path, which invites into an | ||
| // archived channel and fails forever. | ||
| await setStatus(prisma, organizationId, "ARCHIVED", { | ||
| slackChannelId: channelId, | ||
| slackChannelName: channelName, | ||
| lastError: error instanceof Error ? error.message : String(error), | ||
| }); | ||
| return { status: "failed", retryable: true }; |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
target="apps/webapp/app/services/supportSlackChannel.server.ts"
printf '%s\n' '--- file outline ---'
ast-grep outline "$target" || true
printf '%s\n' '--- relevant symbols ---'
rg -n -C 8 \
'unarchiveChannel|inviteSharedByEmail|setStatus|createPrivateChannel|ARCHIVED|FAILED|retryable' \
"$target"
printf '%s\n' '--- related usages ---'
rg -n -C 5 \
'unarchiveChannel|inviteSharedByEmail|setStatus|createPrivateChannel|supportSlackChannel' \
apps/webapp packages 2>/dev/null | head -n 500Repository: triggerdotdev/trigger.dev
Length of output: 50381
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- status and provisioning implementation ---'
sed -n '260,430p' apps/webapp/app/services/supportSlackChannel.server.ts
printf '%s\n' '--- support channel tests: provisioning and failure cases ---'
rg -n -C 12 \
'ARCHIVED|FAILED|failInvite|failUnarchive|provisionOrganizationSupportChannel|createPrivateChannel must not' \
apps/webapp/test/supportSlackChannel.test.ts
printf '%s\n' '--- worker handling for provisioning result ---'
rg -n -C 20 \
'provisionOrganizationSupportChannel|retryable|failSupportChannelProvisioning|OrganizationSupportChannelSchema' \
apps/webapp/app/v3/commonWorker.server.tsRepository: triggerdotdev/trigger.dev
Length of output: 26755
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
from dataclasses import dataclass
`@dataclass`
class Row:
status: str
channel_id: str
def current_archived_branch(row: Row, *, unarchive_ok: bool, invite_ok: bool) -> Row:
# Mirrors the branch at lines 323-356:
if not unarchive_ok or not invite_ok:
return Row("ARCHIVED", row.channel_id)
return Row("INVITED", row.channel_id)
def corrected_archived_branch(row: Row, *, unarchive_ok: bool, invite_ok: bool) -> Row:
# Expected split: unarchive failure keeps ARCHIVED; invite failure follows
# the normal retryable FAILED path and retains the channel identity.
if not unarchive_ok:
return Row("ARCHIVED", row.channel_id)
if not invite_ok:
return Row("FAILED", row.channel_id)
return Row("INVITED", row.channel_id)
def retry_path(row: Row) -> str:
# Mirrors the branch selection on the next attempt.
if row.status == "ARCHIVED" and row.channel_id:
return "unarchive_then_invite"
if row.channel_id:
return "invite_existing_channel"
return "create_then_invite"
archived = Row("ARCHIVED", "C123")
assert current_archived_branch(archived, unarchive_ok=False, invite_ok=True) == Row("ARCHIVED", "C123")
assert current_archived_branch(archived, unarchive_ok=True, invite_ok=False) == Row("ARCHIVED", "C123")
assert corrected_archived_branch(archived, unarchive_ok=False, invite_ok=True) == Row("ARCHIVED", "C123")
assert corrected_archived_branch(archived, unarchive_ok=True, invite_ok=False) == Row("FAILED", "C123")
assert retry_path(corrected_archived_branch(archived, unarchive_ok=True, invite_ok=False)) == "invite_existing_channel"
print("successful unarchive + failed invite: current=ARCHIVED, corrected=FAILED, retry=invite_existing_channel")
print("failed unarchive: current=ARCHIVED, corrected=ARCHIVED, retry=unarchive_then_invite")
PYRepository: triggerdotdev/trigger.dev
Length of output: 351
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
from dataclasses import dataclass
`@dataclass`
class Row:
status: str
channel_id: str
def current_archived_branch(row: Row, *, unarchive_ok: bool, invite_ok: bool) -> Row:
if not unarchive_ok or not invite_ok:
return Row("ARCHIVED", row.channel_id)
return Row("INVITED", row.channel_id)
def corrected_archived_branch(row: Row, *, unarchive_ok: bool, invite_ok: bool) -> Row:
if not unarchive_ok:
return Row("ARCHIVED", row.channel_id)
if not invite_ok:
return Row("FAILED", row.channel_id)
return Row("INVITED", row.channel_id)
def retry_path(row: Row) -> str:
if row.status == "ARCHIVED" and row.channel_id:
return "unarchive_then_invite"
if row.channel_id:
return "invite_existing_channel"
return "create_then_invite"
archived = Row("ARCHIVED", "C123")
assert current_archived_branch(archived, unarchive_ok=False, invite_ok=True) == Row("ARCHIVED", "C123")
assert current_archived_branch(archived, unarchive_ok=True, invite_ok=False) == Row("ARCHIVED", "C123")
assert corrected_archived_branch(archived, unarchive_ok=False, invite_ok=True) == Row("ARCHIVED", "C123")
assert corrected_archived_branch(archived, unarchive_ok=True, invite_ok=False) == Row("FAILED", "C123")
assert retry_path(corrected_archived_branch(archived, unarchive_ok=True, invite_ok=False)) == "invite_existing_channel"
print("successful unarchive + failed invite: current=ARCHIVED, corrected=FAILED, retry=invite_existing_channel")
print("failed unarchive: current=ARCHIVED, corrected=ARCHIVED, retry=unarchive_then_invite")
PYRepository: triggerdotdev/trigger.dev
Length of output: 351
Split unarchive and invitation failure handling.
If unarchiveChannel() succeeds but inviteSharedByEmail() fails, Slack leaves the channel active while this catch persists ARCHIVED. Keep ARCHIVED only when unarchiving fails. After a successful unarchive and failed invite, persist FAILED with the channel identity so the next retry uses the existing active channel.
Comment on lines
+124
to
+131
| // Persist before enqueueing. The worker can finish between the two, and if | ||
| // the write came second it would clobber INVITED back to PROVISIONING — | ||
| // leaving the page stuck, with the job already deduped so nothing retries. | ||
| await prisma.organizationSupportChannel.upsert({ | ||
| where: { organizationId }, | ||
| create: { organizationId, status: "PROVISIONING" }, | ||
| update: { status: "PROVISIONING", lastError: null }, | ||
| }); |
Contributor
There was a problem hiding this comment.
🔴 Rebuilding a support channel after it was taken away never works
The organization's support channel record is reset to "setting up" (upsert at apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.support.tsx:127-131) before the background job runs, so the job no longer knows the channel was previously shut down and every attempt to bring it back fails.
Impact: A customer whose Slack support channel was unlinked can click Connect forever and only ever gets an error; the channel is never restored.
ARCHIVED status is overwritten before the worker reads it, making the unarchive branch unreachable
unlinkSupportChannel (apps/webapp/app/services/supportSlackChannel.server.ts:440-463) archives the Slack channel and leaves the row at status ARCHIVED with slackChannelId retained. The support page then renders the "Connect to Slack" form for that state.
When the user submits, the action unconditionally writes status: "PROVISIONING" before enqueuing (...settings.support.tsx:127-131). By the time the worker calls provisionOrganizationSupportChannel, existing.status is PROVISIONING, so the dedicated re-upgrade branch at apps/webapp/app/services/supportSlackChannel.server.ts:323 (which calls unarchiveChannel first) is skipped. Execution falls through to the "reuse the persisted channel" path at apps/webapp/app/services/supportSlackChannel.server.ts:363-405, which calls inviteSharedByEmail directly on a channel that is still archived in Slack — exactly the failure mode the comment at apps/webapp/app/services/supportSlackChannel.server.ts:347-350 warns about. The row lands on FAILED, and every retry repeats the same path.
The only enqueue site for supportChannel.provision is this action (enqueueProvisionSupportChannel, apps/webapp/app/services/supportSlackChannel.server.ts:648), so in practice the unarchive branch is dead code and the archived → reconnect flow is permanently broken.
Prompt for agents
In apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.support.tsx the connect action always upserts the OrganizationSupportChannel row to status PROVISIONING before enqueuing the worker job. When the row is currently ARCHIVED (set by unlinkSupportChannel after an admin unlink, which archives the Slack channel but keeps slackChannelId), this overwrite destroys the only signal provisionOrganizationSupportChannel uses to take its unarchive-and-reuse branch (apps/webapp/app/services/supportSlackChannel.server.ts around line 323). The worker then takes the plain reuse path and invites into a channel that is still archived in Slack, which fails on every attempt — the exact scenario the comment in that branch describes.
Possible approaches: leave the row at ARCHIVED (only clearing lastError) when re-connecting so the worker can detect it, or make the worker decide based on whether the channel is archived in Slack rather than on the row's status (e.g. always unarchive when a persisted slackChannelId exists, treating the Slack 'not_archived' error as a no-op — unarchiveChannel already handles that). Also note the error path in the same action that writes status FAILED would similarly lose the ARCHIVED signal, so it needs the same treatment.
Was this helpful? React with 👍 or 👎 to provide feedback.
Comment on lines
+83
to
+99
| const organizations = await prisma.organization.findMany({ | ||
| where: { deletedAt: null }, | ||
| select: { | ||
| id: true, | ||
| slug: true, | ||
| title: true, | ||
| supportChannel: { | ||
| select: { slackChannelId: true, slackChannelName: true, status: true }, | ||
| }, | ||
| members: { | ||
| where: { role: "ADMIN" }, | ||
| take: 1, | ||
| orderBy: { createdAt: "asc" }, | ||
| select: { user: { select: { email: true } } }, | ||
| }, | ||
| }, | ||
| }); |
Contributor
There was a problem hiding this comment.
🟡 Admin Slack channels page loads every organization in the system into each dropdown
The admin page fetches all organizations with no limit or search (prisma.organization.findMany at apps/webapp/app/routes/admin.slack-channels.tsx:83-99) and renders the full list inside a dropdown for every discovered channel, so on an instance with many organizations the page becomes very heavy to load and render.
Impact: The admin Slack channels page can take a long time to load or hang the browser on installs with a large number of organizations.
Unbounded org query fanned out across every channel row
The loader selects id/slug/title plus a support channel and an admin member for every non-deleted organization. The resulting orgs array is serialized into the loader payload and then mapped into <option> elements once per channel row (apps/webapp/app/routes/admin.slack-channels.tsx:330-334), giving O(channels × organizations) DOM nodes. Other admin pages in this app paginate/search organizations rather than listing them all. A searchable/paginated org picker, or reusing the proposal list plus an on-demand lookup, would avoid this.
Was this helpful? React with 👍 or 👎 to provide feedback.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
A private Slack support channel for paid organizations, plus a super-admin page to link the channels that already exist.
/orgs/:org/settings/support— Owners on an entitled plan can create a private Slack Connect channel and get invited to it. Everyone else sees an upgrade option./admin/slack-channels— discoverscus-*Slack Connect channels the bot is in, proposes an organization for each, and writes the link once a human approves.Provisioning runs on the common worker and is retry-safe: the channel id is persisted before the invite is attempted, so a retry never creates a second channel.
Dormant until the plan entitlement ships
The page gates on a
supportChannelplan entitlement and is fail-closed — until that entitlement is granted, every organization sees the upgrade option and nothing provisions. Safe to merge; the feature is switched on separately (TRI-12095 for the entitlement, TRI-12049 for the Slack app scopes).Notes
OrganizationSupportChannel.SLACK_BOT_TOKENis optional — with no token the worker job is a no-op.manage:billing, enforced on the action and mirrored as a disabled button.TRI-11240