feat(webapp): Vercel / Slack integrations improvements#3108
feat(webapp): Vercel / Slack integrations improvements#3108
Conversation
Redirect project settings to the general settings route - Replace v3ProjectSettingsPath with v3ProjectSettingsGeneralPath in org project settings loader so users land on the correct general settings page after selecting an environment. Add integrations settings route for Vercel onboarding flows - Swap v3ProjectSettingsPath for v3ProjectSettingsIntegrationsPath in VercelOnboardingModal imports to point onboarding actions at the integrations-specific settings route. Improve VercelOnboardingModal behavior and telemetry - Introduce origin variable and use it to derive fromMarketplaceContext for clearer intent. - Add optional vercelManageAccessUrl prop to allow rendering a manage access link when present. - Track a "vercel onboarding github step viewed" event with extra context (origin, step, org/project slugs, GH app installation state) and include capture and identifiers in the effect dependencies. - Prevent dialog from closing when interacting outside by handling onInteractOutside to stop accidental dismissals. - Start introducing rendering for a "Manage access" link next to the Connect Project button when vercelManageAccessUrl exists and no origin query param is present. Misc - Update effect dependency array to include newly referenced values (capture, organizationSlug, projectSlug, gitHubAppInstallations.length) to satisfy hooks correctness.
Add resolution and propagation of deployment integration metadata (git links and Vercel deployment URL) to alert delivery flow so emails, Slack messages and webhooks can include richer context for deployment alerts. - Resolve Git metadata and Vercel integration info via BranchesPresenter and Vercel integration schema in a new #resolveDeploymentMetadata flow; represent result as DeploymentIntegrationMetadata. - Thread DeploymentIntegrationMetadata through DeliverAlertService and pass it into #sendEmail, #sendSlack and #sendWebhook calls. - Populate email payload with git details (branch, short SHA, commit message and PR info) and vercelDeploymentUrl when present. - Use neverthrow.fromPromise to safely handle metadata resolution and fall back to empty metadata on errors. Motivation: provide recipients immediate links and SCM context for deployment success/failure alerts to speed debugging and navigation.
Replace the generic "Integrations" item with a dedicated Vercel entry in the Organization Settings side menu. Import the VercelLogo component and adjust the menu structure to include an "Integrations" header and a separate Vercel menu item. Update icon props and active color to match the Vercel logo styling. This clarifies the integrations section and provides a direct link to the organization's Vercel integration page.
Add a fire-and-forget call to loopsClient.vercelIntegrationStarted from the Vercel installation route so we emit a "vercel-integration" event when a user begins the Vercel integration flow. Implement vercelIntegrationStarted in services/loops.server to log and dispatch the event payload (email, userId, firstName, eventName). This provides visibility into integration adoption without blocking the redirect flow. Error handling is intentionally silent to avoid affecting user experience.
Introduce a new route for organization Slack integration settings, including loader and action handlers. The loader fetches the organization's Slack integration, its team name, and related alert channels for display. The action supports an "uninstall" intent that disables related Slack alert channels and marks the integration as deleted within a single transaction. Also add UI imports and helper utilities used by the page (formatDate, components, and validation). Include error handling and logging for missing integrations and transaction failures. Reasons: - Provide a dedicated settings UI for viewing Slack integration details and managing/uninstalling the integration. - Ensure uninstall cleans up related alert channels atomically to avoid inconsistent state.
|
WalkthroughThis PR restructures project settings navigation by splitting it into distinct "General" and "Integrations" pages, each with corresponding server-side implementations. It introduces a reusable Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx (1)
52-57:⚠️ Potential issue | 🟡 Minor
project.idis rendered twice in the same property item.
Property.Valueon line 54 and theParagraphon line 56 both displayproject.id. The "Org ID" item below only usesProperty.Value, suggesting the extradiv+Paragraphwrapper here is a leftover. Consider removing the duplicate:Proposed fix
<Property.Item> <Property.Label>ID</Property.Label> - <Property.Value>{project.id}</Property.Value> - <div className="flex items-center gap-2"> - <Paragraph variant="extra-small/bright/mono">{project.id}</Paragraph> - </div> + <Property.Value>{project.id}</Property.Value> </Property.Item>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/webapp/app/routes/_app.orgs`.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx around lines 52 - 57, The Property.Item for the project ID currently renders project.id twice (once in Property.Value and again inside the extra div/Paragraph); remove the redundant div and Paragraph so only Property.Value displays project.id. Locate the Property.Item that uses Property.Label "ID" and drop the surrounding <div className="flex items-center gap-2"> and the Paragraph component that references project.id to eliminate the duplicate rendering.internal-packages/emails/emails/deployment-failure.tsx (1)
47-69:⚠️ Potential issue | 🟡 MinorPre-existing:
error.messagemissing frompreviewDefaults.The
errorobject inpreviewDefaults(Line 53-56) hasnameandstackbut omitsmessage, which is required by the schema (Line 28). This causes the email preview to render"Error undefined"on Line 97. Not introduced by this PR, but worth fixing while you're updating this block.🐛 Suggested fix
error: { name: "Error", + message: "Something went wrong", stack: "Error: Something went wrong\n at main.ts:12:34", },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal-packages/emails/emails/deployment-failure.tsx` around lines 47 - 69, The previewDefaults object’s error property is missing the required message field which causes the preview to render "Error undefined"; update the previewDefaults constant (the error object inside previewDefaults) to include an error.message (e.g., "Something went wrong" or the same text as in stack) so it satisfies the schema and the email preview renders correctly.apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.integrations.vercel.tsx (1)
145-195:⚠️ Potential issue | 🟡 MinorNon-atomic external + local operation: Vercel-side uninstall may succeed while local soft-delete fails.
The Vercel API call (Line 146) happens before the local DB transaction (Line 164). If the transaction fails, Vercel is already uninstalled but local records remain active. Retrying would attempt another Vercel API uninstall on an already-removed integration, which may fail or behave unexpectedly.
Consider reordering to soft-delete locally first (recoverable), then call the Vercel API, or wrapping both in a compensating flow.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/webapp/app/routes/_app.orgs`.$organizationSlug.settings.integrations.vercel.tsx around lines 145 - 195, Reorder and make the uninstall atomic by performing the local soft-delete inside a transaction first (using the existing $transaction / tx.organizationProjectIntegration.updateMany and tx.organizationIntegration.update calls) to mark the integration as "uninstalling" or set deletedAt, then call VercelIntegrationRepository.uninstallVercelIntegration; if the external uninstall fails, run a compensating transaction to revert the local changes (clear deletedAt or reset the "uninstalling" flag) and return the error, otherwise commit and keep the soft-delete. Use the same fromPromise/txResult error handling and the existing logger/identifiers (organization.id, organizationSlug, userId, vercelIntegration.id) when logging failures for both the external uninstall and the compensating rollback.apps/webapp/app/v3/services/alerts/deliverAlert.server.ts (1)
614-614:⚠️ Potential issue | 🟡 MinorRedundant non-null assertion before nullish coalescing.
alert.workerDeployment.deployedAt!asserts non-null, but?? new Date()already handles the null case. The!is misleading — remove it.Proposed fix
- deployedAt: alert.workerDeployment.deployedAt! ?? new Date(), + deployedAt: alert.workerDeployment.deployedAt ?? new Date(),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/webapp/app/v3/services/alerts/deliverAlert.server.ts` at line 614, The expression uses a redundant non-null assertion on alert.workerDeployment.deployedAt before a nullish coalescing fallback; remove the exclamation mark so deployedAt is set with alert.workerDeployment.deployedAt ?? new Date(), leaving the nullish coalescing to handle null/undefined instead of forcing non-null via the ! operator.
🧹 Nitpick comments (17)
apps/webapp/app/models/vercelIntegration.server.ts (2)
978-990: Redundant Vercel API call after removal.
removeAllVercelEnvVarsByKeyfetches all env vars viafilterProjectEnvs, thenbatchUpsertVercelEnvVars(line 985) fetches them again. After the removal step, the upsert will always find zero matching vars and create fresh ones, making the upsert's lookup unnecessary. Consider passing a flag to skip the lookup in the upsert, or switching to a simple batch-create after removal.This is a minor inefficiency (one extra API call per sync), not a correctness issue.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/webapp/app/models/vercelIntegration.server.ts` around lines 978 - 990, The code performs an unnecessary second fetch of Vercel envs: removeAllVercelEnvVarsByKey already calls filterProjectEnvs to delete matching keys, then batchUpsertVercelEnvVars calls filterProjectEnvs again only to find none; remove that redundant call by either (A) adding a skipLookup boolean parameter to batchUpsertVercelEnvVars (e.g., skipLookup=true when called after removeAllVercelEnvVarsByKey) and short-circuiting the filterProjectEnvs call when skipLookup is true, or (B) introduce a dedicated batchCreateVercelEnvVars helper and call it here instead of batchUpsertVercelEnvVars; update the call site shown (the call to batchUpsertVercelEnvVars after removeAllVercelEnvVarsByKey) to use the new flag or the new batch-create helper and ensure related callers retain default behavior.
1536-1563: NewremoveAllVercelEnvVarsByKeylooks correct; note unhandled API errors.The method has no try/catch or
ResultAsyncwrapping — both thefilterProjectEnvsandbatchRemoveProjectEnvcalls will throw on failure. This is fine because the caller (syncApiKeysToVercel) wraps the entire flow inResultAsync.fromPromise, but the error message the user gets will be generic ("Failed to sync API keys to Vercel") rather than pinpointing the removal step. If you later need finer diagnostics, consider wrapping this inwrapVercelCalltoo.Also, on line 1552, the non-null assertion
env.id!is safe because the filter on line 1551 checksenv.idis truthy, but using a type guard (e.g.,.filter((env): env is typeof env & { id: string } => ...)) would be more idiomatic.Suggested type-safe alternative for the filter+map
const envs = extractVercelEnvs(existingEnvs); - const idsToRemove = envs - .filter((env) => env.key === key && env.id) - .map((env) => env.id!); + const idsToRemove = envs + .filter((env): env is typeof env & { id: string } => env.key === key && typeof env.id === "string") + .map((env) => env.id);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/webapp/app/models/vercelIntegration.server.ts` around lines 1536 - 1563, removeAllVercelEnvVarsByKey currently leaves API errors unannotated and uses a non-null assertion for env.id; wrap the Vercel calls (client.projects.filterProjectEnvs and client.projects.batchRemoveProjectEnv) in the existing wrapVercelCall (or a try/catch that rethrows a descriptive error) so failures surface as "failed removing Vercel envs for key X" instead of a generic upstream message, and replace .filter((env) => env.key === key && env.id) .map(env => env.id!) with a type guard filter such as .filter((env): env is typeof env & { id: string } => env.key === key && Boolean(env.id)) followed by .map(env => env.id) to remove the non-null assertion and make the code type-safe; reference removeAllVercelEnvVarsByKey, client.projects.filterProjectEnvs, client.projects.batchRemoveProjectEnv, syncApiKeysToVercel, and wrapVercelCall.apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings.general/route.tsx (2)
65-70: Parameter validation uses manual destructuring instead of Zod schema parsing.Other routes in the codebase use
ProjectParamSchema.parse(params)orEnvironmentParamSchema.parse(params)for parameter validation. Additionally, the error message on line 69 only mentionsorganizationSlugbut the condition also covers a missingprojectParam.Suggested fix using existing schema
export const action: ActionFunction = async ({ request, params }) => { const userId = await requireUserId(request); - const { organizationSlug, projectParam } = params; - if (!organizationSlug || !projectParam) { - return json({ errors: { body: "organizationSlug is required" } }, { status: 400 }); - } + const { organizationSlug, projectParam } = ProjectParamSchema.parse(params);This requires importing
ProjectParamSchemafrom~/utils/pathBuilder(already imported in other routes) and gives proper Zod validation with automatic error messages.As per coding guidelines: "Use zod for validation in packages/core and apps/webapp".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/webapp/app/routes/_app.orgs`.$organizationSlug.projects.$projectParam.env.$envParam.settings.general/route.tsx around lines 65 - 70, The action function is manually destructuring params and checking organizationSlug/projectParam instead of using the project's Zod schemas; replace the manual check by parsing params with the existing ProjectParamSchema (or EnvironmentParamSchema if appropriate) to validate and extract organizationSlug and projectParam, update the function to import ProjectParamSchema from ~/utils/pathBuilder, use ProjectParamSchema.parse(params) (or .safeParse and return json errors) to validate and remove the ad-hoc if block, and ensure any error response reports the schema validation errors rather than the single "organizationSlug is required" message.
219-228:autoFocuson the rename input may be disruptive.The rename field auto-focuses whenever the General settings page loads. If users navigate here just to view settings or copy the project ref, the auto-focus on the rename input could be unexpected. Consider removing it so the page loads neutrally.
Suggested diff
<Input {...conform.input(projectName, { type: "text" })} defaultValue={project.name} placeholder="Project name" icon={FolderIcon} - autoFocus onChange={(e) => { setHasRenameFormChanges(e.target.value !== project.name); }} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/webapp/app/routes/_app.orgs`.$organizationSlug.projects.$projectParam.env.$envParam.settings.general/route.tsx around lines 219 - 228, The Input field for renaming uses the autoFocus prop causing focus every time the General settings page loads; remove the autoFocus attribute from the Input component that uses conform.input(projectName, { type: "text" }) (the rename input that sets setHasRenameFormChanges based on e.target.value !== project.name) so the page loads without automatically focusing the rename field.apps/webapp/app/components/navigation/OrganizationSettingsSideMenu.tsx (1)
125-138: Both integration items share the samedata-action="integrations"value.If
data-actionis used for testing or analytics, having both Vercel and Slack use the identical value makes them indistinguishable. Consider using distinct values like"integrations-vercel"and"integrations-slack".Proposed diff
<SideMenuItem name="Vercel" icon={VercelLogo} activeIconColor="text-white" to={organizationVercelIntegrationPath(organization)} - data-action="integrations" + data-action="integrations-vercel" /> <SideMenuItem name="Slack" icon={SlackIcon} activeIconColor="text-white" to={organizationSlackIntegrationPath(organization)} - data-action="integrations" + data-action="integrations-slack" />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/webapp/app/components/navigation/OrganizationSettingsSideMenu.tsx` around lines 125 - 138, Two SideMenuItem components (the Vercel and Slack entries) currently share data-action="integrations", making them indistinguishable for tests/analytics; update the Vercel SideMenuItem (the one using organizationVercelIntegrationPath and VercelLogo) to use a unique data-action such as "integrations-vercel" and update the Slack SideMenuItem (the one using organizationSlackIntegrationPath and SlackIcon) to use "integrations-slack" so each integration has a distinct data-action value.apps/webapp/app/components/integrations/VercelOnboardingModal.tsx (1)
549-555: Inconsistent event emission:captureused directly instead oftrackOnboarding.All other tracking calls in this component use
trackOnboarding(...), which automatically attachesorigin,step,organization_slug, andproject_slug. This call usescapturedirectly with a manually constructed payload. Consider usingtrackOnboardingfor consistency and to avoid drift in event schemas.Suggested change
- capture("vercel onboarding github step viewed", { - origin: fromMarketplaceContext ? "marketplace" : "dashboard", - step: "github-connection", - organization_slug: organizationSlug, - project_slug: projectSlug, - github_app_installed: gitHubAppInstallations.length > 0, - }); + trackOnboarding("vercel onboarding github step viewed", { + github_app_installed: gitHubAppInstallations.length > 0, + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/webapp/app/components/integrations/VercelOnboardingModal.tsx` around lines 549 - 555, Replace the direct capture call with the component's tracking helper to keep schema and defaults consistent: in VercelOnboardingModal.tsx, swap the capture("vercel onboarding github step viewed", ...) usage for trackOnboarding("vercel onboarding github step viewed", ...) and remove the manually supplied origin/step/organization_slug/project_slug (trackOnboarding already injects origin based on fromMarketplaceContext, step, organizationSlug, projectSlug); still pass any extra property like github_app_installed: gitHubAppInstallations.length > 0 as the event payload. Ensure you use the same trackOnboarding import and signature as other calls in the file.apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.integrations.vercel.tsx (2)
82-84: Unsafeas anycast onintegrationData— prefer schema validation.Other presenters in this PR (e.g.,
DeploymentListPresenter,DeploymentPresenter) useVercelProjectIntegrationDataSchema.safeParse()to safely extractteamId,installationId, etc. Usingas anyhere bypasses validation and is inconsistent.♻️ Suggested fix
+import { VercelProjectIntegrationDataSchema } from "~/v3/vercel/vercelProjectIntegrationSchema"; + // Get team ID from integrationData -const integrationData = vercelIntegration.integrationData as any; -const teamId = integrationData?.teamId ?? null; -const installationId = integrationData?.installationId ?? null; +const parsed = VercelProjectIntegrationDataSchema.safeParse(vercelIntegration.integrationData); +const teamId = parsed.success ? (parsed.data.vercelTeamId ?? null) : null; +const installationId = parsed.success ? (parsed.data.vercelProjectId ?? null) : null;Note: Adjust field names to match the actual schema fields (
vercelTeamId, etc.) — theintegrationDatastored for org-level integrations may differ from the project-level schema. Verify which fields are stored at this level.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/webapp/app/routes/_app.orgs`.$organizationSlug.settings.integrations.vercel.tsx around lines 82 - 84, Replace the unsafe cast of integrationData in the vercelIntegration code path by validating it with the appropriate Zod schema (e.g., VercelProjectIntegrationDataSchema.safeParse or the org-level schema if different) instead of "as any": call safeParse(vercelIntegration.integrationData), check parsed.success, and extract teamId and installationId from parsed.data (or set them to null on failure); update references to integrationData, teamId, and installationId so they come from the validated parsed.data and handle the parse failure path (log or default to null) to maintain type safety and consistency with DeploymentListPresenter/DeploymentPresenter.
355-365: Hardcoded{ slug: "prod" }environment in the settings link.The environment is hardcoded to
"prod"for the "Configure" link. If a project's production environment slug ever differs, this link will be incorrect. Consider whether the actual environment slug for each project could be resolved from the query data.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/webapp/app/routes/_app.orgs`.$organizationSlug.settings.integrations.vercel.tsx around lines 355 - 365, The Configure link currently hardcodes the environment slug to { slug: "prod" } causing incorrect URLs when a project's production environment differs; change LinkButton's to prop to compute the slug from the project data (use the projectIntegration.project object), e.g. look up a production environment slug via projectIntegration.project.environments (find the one marked production or the defaultEnvironmentSlug property if present) and pass that slug into v3ProjectSettingsIntegrationsPath, falling back to an appropriate first/primary environment or a safe default only if no slug can be resolved.apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.github.tsx (1)
570-570: Long line — consider extracting for readability.Line 570 is quite long. Consider extracting the default path construction to a separate
constfor readability.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/webapp/app/routes/resources.orgs`.$organizationSlug.projects.$projectParam.env.$envParam.github.tsx at line 570, The expression assigning githubInstallationRedirect is too long; extract the default path into a named const (e.g., defaultIntegrationsPath) and then set githubInstallationRedirect = redirectUrl || defaultIntegrationsPath. Locate the assignment to githubInstallationRedirect and create the const using v3ProjectSettingsIntegrationsPath with organizationSlug, projectSlug, and environmentSlug so the main line reads clearly and improves readability.apps/webapp/app/v3/services/alerts/deliverAlert.server.ts (3)
36-39: Service depends on a presenter — unusual dependency direction.
DeliverAlertServiceimportsprocessGitMetadataandGitMetaLinksfromBranchesPresenter.server.ts. Services typically sit below presenters in the dependency hierarchy. Consider extractingprocessGitMetadataandGitMetaLinksinto a shared utility module (e.g.,~/v3/gitMeta.server.ts) that both the presenter and service can import.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/webapp/app/v3/services/alerts/deliverAlert.server.ts` around lines 36 - 39, DeliverAlertService currently imports processGitMetadata and GitMetaLinks from BranchesPresenter.server — flip the dependency by extracting those symbols into a shared utility module (e.g., create ~/v3/gitMeta.server.ts) and move the implementation/type there; then update DeliverAlertService (deliverAlert.server.ts) and BranchesPresenter.server to import processGitMetadata and GitMetaLinks from the new shared module instead of from the presenter. Ensure exported names match exactly (processGitMetadata, GitMetaLinks) and update any relative import paths and tests/imports that referenced the old presenter location.
1119-1119: Non-null assertion onalert.workerDeployment!.shortCode.This is only called from
DEPLOYMENT_SUCCESSandDEPLOYMENT_FAILUREbranches whereworkerDeploymentis already null-checked, so it's safe at runtime. However, it would be cleaner to passshortCodeas a parameter (likeversionandenvironment) instead of re-accessing the alert inside the helper.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/webapp/app/v3/services/alerts/deliverAlert.server.ts` at line 1119, Replace the non-null assertion access alert.workerDeployment!.shortCode by passing shortCode into the helper like the already-passed version and environment: update the helper's signature to accept a shortCode parameter (e.g., add shortCode: string to the function used in the DEPLOYMENT_SUCCESS/DEPLOYMENT_FAILURE branches), update the call sites in those branches to extract const shortCode = alert.workerDeployment.shortCode (after the existing null-check) and pass it in, and remove the alert.workerDeployment! usage inside the helper.
266-278: Duplicated git object construction in email payloads — extract a helper.The same git metadata mapping (Lines 266–277 and 300–311) is repeated for
DEPLOYMENT_FAILUREandDEPLOYMENT_SUCCESSemail payloads. You already have#buildWebhookGitObjectfor webhook payloads — consider a similar#buildEmailGitObjecthelper (or reuse the webhook one if the shape matches).Proposed refactor sketch
+ `#buildEmailGitObject`(git: GitMetaLinks | null) { + if (!git) return undefined; + return { + branchName: git.branchName, + shortSha: git.shortSha, + commitMessage: git.commitMessage, + commitUrl: git.commitUrl, + branchUrl: git.branchUrl, + pullRequestNumber: git.pullRequestNumber, + pullRequestTitle: git.pullRequestTitle, + pullRequestUrl: git.pullRequestUrl, + }; + }Then use
git: this.#buildEmailGitObject(deploymentMeta.git)in both email cases.Also applies to: 300-312
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/webapp/app/v3/services/alerts/deliverAlert.server.ts` around lines 266 - 278, The email payloads for DEPLOYMENT_FAILURE and DEPLOYMENT_SUCCESS duplicate the same git metadata mapping; extract that mapping into a helper (e.g. a private method named `#buildEmailGitObject`(gitMeta)) or reuse the existing `#buildWebhookGitObject` if the returned shape matches, and replace the inline object in both payload constructions with git: this.#buildEmailGitObject(deploymentMeta.git) (or git: this.#buildWebhookGitObject(deploymentMeta.git)); update where vercelDeploymentUrl is still set separately and ensure the helper returns undefined when gitMeta is falsy to preserve existing behavior.apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.integrations.slack.tsx (2)
101-106: Consider limiting the data returned from the loader.The loader returns the full
slackIntegrationobject (Line 103) andorganizationobject (Line 102), which may include fields not needed by the UI (e.g., tokens, internal IDs). Returning only the fields used by the component (e.g.,createdAt,id) would reduce the serialized payload and avoid accidental leakage of internal data.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/webapp/app/routes/_app.orgs`.$organizationSlug.settings.integrations.slack.tsx around lines 101 - 106, The loader is returning full objects (organization, slackIntegration) via typedjson which may leak sensitive/internal fields; instead, restrict the returned payload to only the fields the UI consumes (e.g., for slackIntegration return id, createdAt, name/metadata and omit tokens/secret fields; for organization return only id, slug, name or whatever the component reads), update the loader's return object to include these sanitized shapes instead of the raw objects, and adjust any related type definitions or the consuming component (e.g., references to slackIntegration and organization in the loader and consuming code) to use the narrower fields.
70-71: Unsafeas anycast onintegrationData— prefer Zod validation.
integrationDatais aPrisma.JsonValuewhich could be anything. Casting toanyand accessing?.team?.namesilently swallows malformed data. As per coding guidelines, use Zod for validation in the webapp.Proposed fix sketch
- const integrationData = slackIntegration.integrationData as any; - const teamName = integrationData?.team?.name ?? null; + const SlackIntegrationData = z.object({ + team: z.object({ name: z.string() }).optional(), + }).passthrough(); + const parsed = SlackIntegrationData.safeParse(slackIntegration.integrationData); + const teamName = parsed.success ? (parsed.data.team?.name ?? null) : null;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/webapp/app/routes/_app.orgs`.$organizationSlug.settings.integrations.slack.tsx around lines 70 - 71, integrationData is being unsafely cast to any (slackIntegration.integrationData) and then accessed (teamName), which can hide malformed data; replace the cast with a Zod schema (e.g. SlackIntegrationDataSchema) that models the expected shape ({ team?: { name?: string | null } }) and use SlackIntegrationDataSchema.safeParse(slackIntegration.integrationData) (or parse and catch) to validate before reading, then set teamName from the validated result or fallback to null; update any types/usages to use the validated type instead of any.apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings.integrations/route.tsx (3)
360-362:useActionData() as anydiscards type safety.This is a known trade-off with
@conform-to/reactv0, but consider at least narrowing the cast to the submission type or adding a brief comment explaining whyanyis needed here.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/webapp/app/routes/_app.orgs`.$organizationSlug.projects.$projectParam.env.$envParam.settings.integrations/route.tsx around lines 360 - 362, The useActionData() call in BuildSettingsForm currently uses an unsafe cast to any; narrow that to the actual submission type (e.g., Submission | null or the specific ReturnType/ActionData used by the form) instead of any, or add a one-line comment explaining why any is unavoidable with `@conform-to/react` v0; update the declaration of lastSubmission in BuildSettingsForm (near useActionData/useNavigation) to use the precise type or include the explanatory comment so type-safety is preserved or the exception is documented.
259-278: UnstablevercelFetcherreference inuseCallbackdependency array.
vercelFetcher(fromuseTypedFetcher) returns a new object reference on each state change, so including the whole object in the dependency array ofhandleOpenVercelModalcauses it to be recreated on every fetcher state transition. This cascades into the effect at Lines 281–287 (which depends onopenVercelOnboarding→ indirectly onhandleOpenVercelModal). Prefer destructuring the specific properties you need (vercelFetcher.data,vercelFetcher.load,vercelFetcher.state).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/webapp/app/routes/_app.orgs`.$organizationSlug.projects.$projectParam.env.$envParam.settings.integrations/route.tsx around lines 259 - 278, The handler handleOpenVercelModal should stop depending on the unstable vercelFetcher object; destructure the specific pieces you need from useTypedFetcher (e.g. const { data: vercelData, load: vercelLoad, state: vercelState } = vercelFetcher) and use vercelData and vercelLoad (and vercelState if used) inside the function, then update the useCallback dependency array to include vercelData, vercelLoad (and vercelState) instead of vercelFetcher; keep other deps like organization.slug, project.slug, environment.slug, setSearchParams, hasQueryParam, openVercelOnboarding, and ensure waitingForButtonClickRef.current is set before calling vercelLoad.
213-253: RedundantuseEffecthooks risk infinite re-render loops and make the logic hard to follow.There are three
useEffecthooks (Lines 215–233, 237–243, 246–253) that overlap significantly:
- Effect 2 (Lines 237–243) calls
openVercelOnboarding()wheneverhasQueryParam && !isModalOpen, without waiting for data to load. This can open the modal prematurely (before onboarding data is available).- Effect 3 (Lines 246–253) is identical to the first branch of Effect 1 (Lines 218–222).
- All three mutate
isModalOpenand callopenVercelOnboarding(which also sets search params), which are in each other's dependency arrays, creating a chain of cascading re-renders.Consider consolidating into a single effect that handles all three states (no data → load, data loaded → open, query param removed → close). This would be easier to reason about and eliminate the risk of render loops.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/webapp/app/routes/_app.orgs`.$organizationSlug.projects.$projectParam.env.$envParam.settings.integrations/route.tsx around lines 213 - 253, Consolidate the three overlapping useEffect hooks into a single useEffect that handles all cases: when hasQueryParam && vercelIntegrationEnabled, if vercelFetcher.data?.onboardingData && vercelFetcher.state === "idle" then call openVercelOnboarding() only if !isModalOpen; else if vercelFetcher.state === "idle" && vercelFetcher.data === undefined then call vercelFetcher.load(vercelResourcePath(organization.slug, project.slug, environment.slug) + '?vercelOnboarding=true'); and when !hasQueryParam and isModalOpen call setIsModalOpen(false). Remove the other two effects and ensure the dependency array includes hasQueryParam, vercelIntegrationEnabled, organization.slug, project.slug, environment.slug, vercelFetcher.data, vercelFetcher.state, isModalOpen, openVercelOnboarding, setIsModalOpen and vercelFetcher (or its stable ref) to avoid stale closures and prevent cascading re-renders.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/webapp/app/presenters/v3/DeploymentPresenter.server.ts`:
- Around line 203-206: Replace the inline Vercel URL construction in
DeploymentPresenter.server.ts with the shared helper: instead of manipulating
integrationDeployment.integrationDeploymentId with replace(/^dpl_/, "") and
templating the URL directly, call
buildVercelDeploymentUrl(integrationDeployment.integrationDeploymentId,
parsed.data.vercelTeamSlug, parsed.data.vercelProjectName) (the shared helper
handles the dpl_ prefix/new formats); update the vercelDeploymentUrl assignment
to use that helper and remove the inline replace/template logic so the logic is
centralized and consistent with DeploymentListPresenter.
In
`@apps/webapp/app/routes/_app.orgs`.$organizationSlug.projects.$projectParam.env.$envParam.settings.general/route.tsx:
- Line 96: The code extracts projectId from membershipResultOrFail.value but
calls projectSettingsService.deleteProject with projectParam (the slug); make
this consistent by passing the verified projectId to deleteProject instead of
projectParam: update the call site where deleteProject is invoked (alongside the
existing renameProject usage) to use projectId and userId (e.g.,
projectSettingsService.deleteProject(projectId, userId)), or alternatively
change ProjectSettingsService.deleteProject signature to clearly accept a
projectId type—ensure both renameProject and deleteProject use the same
projectId symbol to remove ambiguity.
In
`@apps/webapp/app/routes/_app.orgs`.$organizationSlug.projects.$projectParam.env.$envParam.settings.integrations/route.tsx:
- Around line 124-126: The guard currently checks both organizationSlug and
projectParam but returns an error that only mentions organizationSlug; update
the validation to return an accurate message (e.g., "organizationSlug and
projectParam are required" or return specific messages per missing param).
Locate the conditional that uses organizationSlug and projectParam and the
json(...) call and change the error body to include both parameter names (or
split into two checks) so the response accurately reflects the validation
failure.
In
`@apps/webapp/app/routes/_app.orgs`.$organizationSlug.settings.integrations.slack.tsx:
- Line 186: The redirect uses a hardcoded string
(`redirect(`/orgs/${organizationSlug}/settings`)`) which is fragile; replace it
to use the shared path builder `organizationSettingsPath()` so the route stays
consistent—call `organizationSettingsPath(organizationSlug)` (or the correct arg
shape) in place of the hardcoded path in the function where the redirect occurs
to return `redirect(organizationSettingsPath(...))`.
In `@apps/webapp/app/v3/vercel/vercelProjectIntegrationSchema.ts`:
- Around line 205-212: The buildVercelDeploymentUrl function currently accepts
vercelTeamSlug: string | undefined but produces a broken URL when undefined;
change the parameter type to vercelTeamSlug: string so callers must
validate/preserve a team slug before calling buildVercelDeploymentUrl (or
alternatively make the function return undefined when vercelTeamSlug is
missing), and update any callers that pass undefined to either perform the guard
or handle the undefined return. Ensure the change targets the
buildVercelDeploymentUrl signature and all call sites that rely on passing
undefined.
---
Outside diff comments:
In
`@apps/webapp/app/routes/_app.orgs`.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx:
- Around line 52-57: The Property.Item for the project ID currently renders
project.id twice (once in Property.Value and again inside the extra
div/Paragraph); remove the redundant div and Paragraph so only Property.Value
displays project.id. Locate the Property.Item that uses Property.Label "ID" and
drop the surrounding <div className="flex items-center gap-2"> and the Paragraph
component that references project.id to eliminate the duplicate rendering.
In
`@apps/webapp/app/routes/_app.orgs`.$organizationSlug.settings.integrations.vercel.tsx:
- Around line 145-195: Reorder and make the uninstall atomic by performing the
local soft-delete inside a transaction first (using the existing $transaction /
tx.organizationProjectIntegration.updateMany and
tx.organizationIntegration.update calls) to mark the integration as
"uninstalling" or set deletedAt, then call
VercelIntegrationRepository.uninstallVercelIntegration; if the external
uninstall fails, run a compensating transaction to revert the local changes
(clear deletedAt or reset the "uninstalling" flag) and return the error,
otherwise commit and keep the soft-delete. Use the same fromPromise/txResult
error handling and the existing logger/identifiers (organization.id,
organizationSlug, userId, vercelIntegration.id) when logging failures for both
the external uninstall and the compensating rollback.
In `@apps/webapp/app/v3/services/alerts/deliverAlert.server.ts`:
- Line 614: The expression uses a redundant non-null assertion on
alert.workerDeployment.deployedAt before a nullish coalescing fallback; remove
the exclamation mark so deployedAt is set with alert.workerDeployment.deployedAt
?? new Date(), leaving the nullish coalescing to handle null/undefined instead
of forcing non-null via the ! operator.
In `@internal-packages/emails/emails/deployment-failure.tsx`:
- Around line 47-69: The previewDefaults object’s error property is missing the
required message field which causes the preview to render "Error undefined";
update the previewDefaults constant (the error object inside previewDefaults) to
include an error.message (e.g., "Something went wrong" or the same text as in
stack) so it satisfies the schema and the email preview renders correctly.
---
Nitpick comments:
In `@apps/webapp/app/components/integrations/VercelOnboardingModal.tsx`:
- Around line 549-555: Replace the direct capture call with the component's
tracking helper to keep schema and defaults consistent: in
VercelOnboardingModal.tsx, swap the capture("vercel onboarding github step
viewed", ...) usage for trackOnboarding("vercel onboarding github step viewed",
...) and remove the manually supplied origin/step/organization_slug/project_slug
(trackOnboarding already injects origin based on fromMarketplaceContext, step,
organizationSlug, projectSlug); still pass any extra property like
github_app_installed: gitHubAppInstallations.length > 0 as the event payload.
Ensure you use the same trackOnboarding import and signature as other calls in
the file.
In `@apps/webapp/app/components/navigation/OrganizationSettingsSideMenu.tsx`:
- Around line 125-138: Two SideMenuItem components (the Vercel and Slack
entries) currently share data-action="integrations", making them
indistinguishable for tests/analytics; update the Vercel SideMenuItem (the one
using organizationVercelIntegrationPath and VercelLogo) to use a unique
data-action such as "integrations-vercel" and update the Slack SideMenuItem (the
one using organizationSlackIntegrationPath and SlackIcon) to use
"integrations-slack" so each integration has a distinct data-action value.
In `@apps/webapp/app/models/vercelIntegration.server.ts`:
- Around line 978-990: The code performs an unnecessary second fetch of Vercel
envs: removeAllVercelEnvVarsByKey already calls filterProjectEnvs to delete
matching keys, then batchUpsertVercelEnvVars calls filterProjectEnvs again only
to find none; remove that redundant call by either (A) adding a skipLookup
boolean parameter to batchUpsertVercelEnvVars (e.g., skipLookup=true when called
after removeAllVercelEnvVarsByKey) and short-circuiting the filterProjectEnvs
call when skipLookup is true, or (B) introduce a dedicated
batchCreateVercelEnvVars helper and call it here instead of
batchUpsertVercelEnvVars; update the call site shown (the call to
batchUpsertVercelEnvVars after removeAllVercelEnvVarsByKey) to use the new flag
or the new batch-create helper and ensure related callers retain default
behavior.
- Around line 1536-1563: removeAllVercelEnvVarsByKey currently leaves API errors
unannotated and uses a non-null assertion for env.id; wrap the Vercel calls
(client.projects.filterProjectEnvs and client.projects.batchRemoveProjectEnv) in
the existing wrapVercelCall (or a try/catch that rethrows a descriptive error)
so failures surface as "failed removing Vercel envs for key X" instead of a
generic upstream message, and replace .filter((env) => env.key === key &&
env.id) .map(env => env.id!) with a type guard filter such as .filter((env): env
is typeof env & { id: string } => env.key === key && Boolean(env.id)) followed
by .map(env => env.id) to remove the non-null assertion and make the code
type-safe; reference removeAllVercelEnvVarsByKey,
client.projects.filterProjectEnvs, client.projects.batchRemoveProjectEnv,
syncApiKeysToVercel, and wrapVercelCall.
In
`@apps/webapp/app/routes/_app.orgs`.$organizationSlug.projects.$projectParam.env.$envParam.settings.general/route.tsx:
- Around line 65-70: The action function is manually destructuring params and
checking organizationSlug/projectParam instead of using the project's Zod
schemas; replace the manual check by parsing params with the existing
ProjectParamSchema (or EnvironmentParamSchema if appropriate) to validate and
extract organizationSlug and projectParam, update the function to import
ProjectParamSchema from ~/utils/pathBuilder, use
ProjectParamSchema.parse(params) (or .safeParse and return json errors) to
validate and remove the ad-hoc if block, and ensure any error response reports
the schema validation errors rather than the single "organizationSlug is
required" message.
- Around line 219-228: The Input field for renaming uses the autoFocus prop
causing focus every time the General settings page loads; remove the autoFocus
attribute from the Input component that uses conform.input(projectName, { type:
"text" }) (the rename input that sets setHasRenameFormChanges based on
e.target.value !== project.name) so the page loads without automatically
focusing the rename field.
In
`@apps/webapp/app/routes/_app.orgs`.$organizationSlug.projects.$projectParam.env.$envParam.settings.integrations/route.tsx:
- Around line 360-362: The useActionData() call in BuildSettingsForm currently
uses an unsafe cast to any; narrow that to the actual submission type (e.g.,
Submission | null or the specific ReturnType/ActionData used by the form)
instead of any, or add a one-line comment explaining why any is unavoidable with
`@conform-to/react` v0; update the declaration of lastSubmission in
BuildSettingsForm (near useActionData/useNavigation) to use the precise type or
include the explanatory comment so type-safety is preserved or the exception is
documented.
- Around line 259-278: The handler handleOpenVercelModal should stop depending
on the unstable vercelFetcher object; destructure the specific pieces you need
from useTypedFetcher (e.g. const { data: vercelData, load: vercelLoad, state:
vercelState } = vercelFetcher) and use vercelData and vercelLoad (and
vercelState if used) inside the function, then update the useCallback dependency
array to include vercelData, vercelLoad (and vercelState) instead of
vercelFetcher; keep other deps like organization.slug, project.slug,
environment.slug, setSearchParams, hasQueryParam, openVercelOnboarding, and
ensure waitingForButtonClickRef.current is set before calling vercelLoad.
- Around line 213-253: Consolidate the three overlapping useEffect hooks into a
single useEffect that handles all cases: when hasQueryParam &&
vercelIntegrationEnabled, if vercelFetcher.data?.onboardingData &&
vercelFetcher.state === "idle" then call openVercelOnboarding() only if
!isModalOpen; else if vercelFetcher.state === "idle" && vercelFetcher.data ===
undefined then call vercelFetcher.load(vercelResourcePath(organization.slug,
project.slug, environment.slug) + '?vercelOnboarding=true'); and when
!hasQueryParam and isModalOpen call setIsModalOpen(false). Remove the other two
effects and ensure the dependency array includes hasQueryParam,
vercelIntegrationEnabled, organization.slug, project.slug, environment.slug,
vercelFetcher.data, vercelFetcher.state, isModalOpen, openVercelOnboarding,
setIsModalOpen and vercelFetcher (or its stable ref) to avoid stale closures and
prevent cascading re-renders.
In
`@apps/webapp/app/routes/_app.orgs`.$organizationSlug.settings.integrations.slack.tsx:
- Around line 101-106: The loader is returning full objects (organization,
slackIntegration) via typedjson which may leak sensitive/internal fields;
instead, restrict the returned payload to only the fields the UI consumes (e.g.,
for slackIntegration return id, createdAt, name/metadata and omit tokens/secret
fields; for organization return only id, slug, name or whatever the component
reads), update the loader's return object to include these sanitized shapes
instead of the raw objects, and adjust any related type definitions or the
consuming component (e.g., references to slackIntegration and organization in
the loader and consuming code) to use the narrower fields.
- Around line 70-71: integrationData is being unsafely cast to any
(slackIntegration.integrationData) and then accessed (teamName), which can hide
malformed data; replace the cast with a Zod schema (e.g.
SlackIntegrationDataSchema) that models the expected shape ({ team?: { name?:
string | null } }) and use
SlackIntegrationDataSchema.safeParse(slackIntegration.integrationData) (or parse
and catch) to validate before reading, then set teamName from the validated
result or fallback to null; update any types/usages to use the validated type
instead of any.
In
`@apps/webapp/app/routes/_app.orgs`.$organizationSlug.settings.integrations.vercel.tsx:
- Around line 82-84: Replace the unsafe cast of integrationData in the
vercelIntegration code path by validating it with the appropriate Zod schema
(e.g., VercelProjectIntegrationDataSchema.safeParse or the org-level schema if
different) instead of "as any": call
safeParse(vercelIntegration.integrationData), check parsed.success, and extract
teamId and installationId from parsed.data (or set them to null on failure);
update references to integrationData, teamId, and installationId so they come
from the validated parsed.data and handle the parse failure path (log or default
to null) to maintain type safety and consistency with
DeploymentListPresenter/DeploymentPresenter.
- Around line 355-365: The Configure link currently hardcodes the environment
slug to { slug: "prod" } causing incorrect URLs when a project's production
environment differs; change LinkButton's to prop to compute the slug from the
project data (use the projectIntegration.project object), e.g. look up a
production environment slug via projectIntegration.project.environments (find
the one marked production or the defaultEnvironmentSlug property if present) and
pass that slug into v3ProjectSettingsIntegrationsPath, falling back to an
appropriate first/primary environment or a safe default only if no slug can be
resolved.
In
`@apps/webapp/app/routes/resources.orgs`.$organizationSlug.projects.$projectParam.env.$envParam.github.tsx:
- Line 570: The expression assigning githubInstallationRedirect is too long;
extract the default path into a named const (e.g., defaultIntegrationsPath) and
then set githubInstallationRedirect = redirectUrl || defaultIntegrationsPath.
Locate the assignment to githubInstallationRedirect and create the const using
v3ProjectSettingsIntegrationsPath with organizationSlug, projectSlug, and
environmentSlug so the main line reads clearly and improves readability.
In `@apps/webapp/app/v3/services/alerts/deliverAlert.server.ts`:
- Around line 36-39: DeliverAlertService currently imports processGitMetadata
and GitMetaLinks from BranchesPresenter.server — flip the dependency by
extracting those symbols into a shared utility module (e.g., create
~/v3/gitMeta.server.ts) and move the implementation/type there; then update
DeliverAlertService (deliverAlert.server.ts) and BranchesPresenter.server to
import processGitMetadata and GitMetaLinks from the new shared module instead of
from the presenter. Ensure exported names match exactly (processGitMetadata,
GitMetaLinks) and update any relative import paths and tests/imports that
referenced the old presenter location.
- Line 1119: Replace the non-null assertion access
alert.workerDeployment!.shortCode by passing shortCode into the helper like the
already-passed version and environment: update the helper's signature to accept
a shortCode parameter (e.g., add shortCode: string to the function used in the
DEPLOYMENT_SUCCESS/DEPLOYMENT_FAILURE branches), update the call sites in those
branches to extract const shortCode = alert.workerDeployment.shortCode (after
the existing null-check) and pass it in, and remove the alert.workerDeployment!
usage inside the helper.
- Around line 266-278: The email payloads for DEPLOYMENT_FAILURE and
DEPLOYMENT_SUCCESS duplicate the same git metadata mapping; extract that mapping
into a helper (e.g. a private method named `#buildEmailGitObject`(gitMeta)) or
reuse the existing `#buildWebhookGitObject` if the returned shape matches, and
replace the inline object in both payload constructions with git:
this.#buildEmailGitObject(deploymentMeta.git) (or git:
this.#buildWebhookGitObject(deploymentMeta.git)); update where
vercelDeploymentUrl is still set separately and ensure the helper returns
undefined when gitMeta is falsy to preserve existing behavior.
| if (integrationDeployment) { | ||
| const vercelId = integrationDeployment.integrationDeploymentId.replace(/^dpl_/, ""); | ||
| vercelDeploymentUrl = `https://vercel.com/${parsed.data.vercelTeamSlug}/${parsed.data.vercelProjectName}/${vercelId}`; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Inline Vercel URL construction — should use buildVercelDeploymentUrl helper.
DeploymentListPresenter was refactored to use the shared buildVercelDeploymentUrl helper, but this presenter still constructs the URL inline with the same replace(/^dpl_/, "") + template logic. This creates duplicated logic that can drift.
♻️ Proposed fix
+import {
+ VercelProjectIntegrationDataSchema,
+ buildVercelDeploymentUrl,
+} from "~/v3/vercel/vercelProjectIntegrationSchema";
-import { VercelProjectIntegrationDataSchema } from "~/v3/vercel/vercelProjectIntegrationSchema"; if (integrationDeployment) {
- const vercelId = integrationDeployment.integrationDeploymentId.replace(/^dpl_/, "");
- vercelDeploymentUrl = `https://vercel.com/${parsed.data.vercelTeamSlug}/${parsed.data.vercelProjectName}/${vercelId}`;
+ vercelDeploymentUrl = buildVercelDeploymentUrl(
+ parsed.data.vercelTeamSlug!,
+ parsed.data.vercelProjectName,
+ integrationDeployment.integrationDeploymentId
+ );
}Based on learnings: "Consider centralizing this logic in a small helper (e.g., getVercelDeploymentId(id) or a URL builder) and add tests to verify both prefixed and non-prefixed inputs."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/webapp/app/presenters/v3/DeploymentPresenter.server.ts` around lines 203
- 206, Replace the inline Vercel URL construction in
DeploymentPresenter.server.ts with the shared helper: instead of manipulating
integrationDeployment.integrationDeploymentId with replace(/^dpl_/, "") and
templating the URL directly, call
buildVercelDeploymentUrl(integrationDeployment.integrationDeploymentId,
parsed.data.vercelTeamSlug, parsed.data.vercelProjectName) (the shared helper
handles the dpl_ prefix/new formats); update the vercelDeploymentUrl assignment
to use that helper and remove the inline replace/template logic so the logic is
centralized and consistent with DeploymentListPresenter.
| return json({ errors: { body: membershipResultOrFail.error.type } }, { status: 404 }); | ||
| } | ||
|
|
||
| const { projectId } = membershipResultOrFail.value; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check the file structure and find the relevant routes file
find . -type f -name "route.tsx" | grep -E "settings.general" | head -20Repository: triggerdotdev/trigger.dev
Length of output: 185
🏁 Script executed:
# List the file to understand its structure
ls -la "apps/webapp/app/routes/_app.orgs.\$organizationSlug.projects.\$projectParam.env.\$envParam.settings.general/" 2>/dev/null || echo "Trying alternative path..." && find apps/webapp/app/routes -type d -name "*settings.general*"Repository: triggerdotdev/trigger.dev
Length of output: 353
🏁 Script executed:
# Search for the relevant route file more broadly
rg -l "deleteProject|renameProject" --type=ts --type=tsx apps/webapp/app/routes | grep -i "settings"Repository: triggerdotdev/trigger.dev
Length of output: 96
🏁 Script executed:
# Read the route file to see the relevant code sections
cat -n "apps/webapp/app/routes/_app.orgs.\$organizationSlug.projects.\$projectParam.env.\$envParam.settings.general/route.tsx" | head -150Repository: triggerdotdev/trigger.dev
Length of output: 6350
🏁 Script executed:
# Search for deleteProject and renameProject method definitions
rg "deleteProject|renameProject" --type=ts -C5 apps/webappRepository: triggerdotdev/trigger.dev
Length of output: 6904
🏁 Script executed:
# Read the full ProjectSettingsService file
cat -n "apps/webapp/app/services/projectSettings.server.ts"Repository: triggerdotdev/trigger.dev
Length of output: 11942
🏁 Script executed:
# Check the DeleteProjectService implementation
cat -n "apps/webapp/app/services/deleteProject.server.ts" | head -100Repository: triggerdotdev/trigger.dev
Length of output: 2757
Pass projectId (which is already verified) to deleteProject instead of projectParam.
Line 100 calls renameProject with projectId, but line 126 calls deleteProject with projectParam (the URL slug). While DeleteProjectService is designed to accept either parameter type, this inconsistency makes the code harder to follow. Since projectId is already extracted and verified from the membership check on line 96, use it consistently: projectSettingsService.deleteProject(projectId, userId) or update ProjectSettingsService.deleteProject to accept projectId instead of projectSlug.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/webapp/app/routes/_app.orgs`.$organizationSlug.projects.$projectParam.env.$envParam.settings.general/route.tsx
at line 96, The code extracts projectId from membershipResultOrFail.value but
calls projectSettingsService.deleteProject with projectParam (the slug); make
this consistent by passing the verified projectId to deleteProject instead of
projectParam: update the call site where deleteProject is invoked (alongside the
existing renameProject usage) to use projectId and userId (e.g.,
projectSettingsService.deleteProject(projectId, userId)), or alternatively
change ProjectSettingsService.deleteProject signature to clearly accept a
projectId type—ensure both renameProject and deleteProject use the same
projectId symbol to remove ambiguity.
| if (!organizationSlug || !projectParam) { | ||
| return json({ errors: { body: "organizationSlug is required" } }, { status: 400 }); | ||
| } |
There was a problem hiding this comment.
Error message doesn't match the validation check.
The guard checks both organizationSlug and projectParam, but the error message only mentions organizationSlug.
Proposed fix
if (!organizationSlug || !projectParam) {
- return json({ errors: { body: "organizationSlug is required" } }, { status: 400 });
+ return json({ errors: { body: "organizationSlug and projectParam are required" } }, { status: 400 });
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (!organizationSlug || !projectParam) { | |
| return json({ errors: { body: "organizationSlug is required" } }, { status: 400 }); | |
| } | |
| if (!organizationSlug || !projectParam) { | |
| return json({ errors: { body: "organizationSlug and projectParam are required" } }, { status: 400 }); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/webapp/app/routes/_app.orgs`.$organizationSlug.projects.$projectParam.env.$envParam.settings.integrations/route.tsx
around lines 124 - 126, The guard currently checks both organizationSlug and
projectParam but returns an error that only mentions organizationSlug; update
the validation to return an accurate message (e.g., "organizationSlug and
projectParam are required" or return specific messages per missing param).
Locate the conditional that uses organizationSlug and projectParam and the
json(...) call and change the error body to include both parameter names (or
split into two checks) so the response accurately reflects the validation
failure.
| return ( | ||
| <> | ||
| <MainHorizontallyCenteredContainer className="md:mt-6"> | ||
| <div className="flex flex-col gap-6"> | ||
| {githubAppEnabled && ( | ||
| <React.Fragment> | ||
| <div> | ||
| <Header2 spacing>Git settings</Header2> | ||
| <div className="w-full rounded-sm border border-grid-dimmed p-4"> | ||
| <GitHubSettingsPanel | ||
| organizationSlug={organization.slug} | ||
| projectSlug={project.slug} | ||
| environmentSlug={environment.slug} | ||
| billingPath={v3BillingPath({ slug: organization.slug })} | ||
| /> | ||
| </div> | ||
| </div> | ||
|
|
||
| {vercelIntegrationEnabled && ( | ||
| <div> | ||
| <Header2 spacing>Vercel integration</Header2> | ||
| <div className="w-full rounded-sm border border-grid-dimmed p-4"> | ||
| <VercelSettingsPanel | ||
| organizationSlug={organization.slug} | ||
| projectSlug={project.slug} | ||
| environmentSlug={environment.slug} | ||
| onOpenVercelModal={handleOpenVercelModal} | ||
| isLoadingVercelData={vercelFetcher.state === "loading" || vercelFetcher.state === "submitting"} | ||
| /> | ||
| </div> | ||
| </div> | ||
| )} | ||
|
|
||
| <div> | ||
| <Header2 spacing>Build settings</Header2> | ||
| <div className="w-full rounded-sm border border-grid-dimmed p-4"> | ||
| <BuildSettingsForm buildSettings={buildSettings ?? {}} /> | ||
| </div> | ||
| </div> | ||
| </React.Fragment> | ||
| )} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for githubAppEnabled usage to understand the pattern
rg 'githubAppEnabled' --type=tsx --type=ts -B 2 -A 2Repository: triggerdotdev/trigger.dev
Length of output: 96
🏁 Script executed:
#!/bin/bash
# Check the specific route file to see full context and if there's an empty state
fd 'route.tsx' -p '*integrations*' | head -20Repository: triggerdotdev/trigger.dev
Length of output: 170
🏁 Script executed:
#!/bin/bash
# Look for where githubAppEnabled is set/derived from
rg 'githubAppEnabled\s*=' --type=tsx --type=ts -B 5 -A 2Repository: triggerdotdev/trigger.dev
Length of output: 96
🏁 Script executed:
#!/bin/bash
# Check if there are other settings routes that handle this differently
fd 'route.tsx' -p '*settings*' | head -20Repository: triggerdotdev/trigger.dev
Length of output: 166
🏁 Script executed:
#!/bin/bash
# Read the actual route file mentioned in the review
cat -n apps/webapp/app/routes/_app.orgs.\$organizationSlug.projects.\$projectParam.env.\$envParam.settings.integrations/route.tsxRepository: triggerdotdev/trigger.dev
Length of output: 23396
🏁 Script executed:
#!/bin/bash
# Search for githubAppEnabled usage with correct syntax
rg 'githubAppEnabled' -g '*.ts' -g '*.tsx' -B 2 -A 2Repository: triggerdotdev/trigger.dev
Length of output: 4037
🏁 Script executed:
#!/bin/bash
# Find settings routes that might have similar patterns
find apps/webapp/app/routes -name 'route.tsx' -path '*settings*' | head -10Repository: triggerdotdev/trigger.dev
Length of output: 871
🏁 Script executed:
#!/bin/bash
# Look for BuildSettingsForm to understand its dependencies
rg 'BuildSettingsForm' -g '*.ts' -g '*.tsx' -B 3 -A 3Repository: triggerdotdev/trigger.dev
Length of output: 5302
🏁 Script executed:
#!/bin/bash
# Check the ProjectSettingsPresenter to see what it returns when githubAppEnabled is false
cat -n apps/webapp/app/services/projectSettingsPresenter.server.ts | head -100Repository: triggerdotdev/trigger.dev
Length of output: 3863
🏁 Script executed:
#!/bin/bash
# Look for any comments or documentation about why Build settings need GitHub app
rg 'Build settings' -g '*.ts' -g '*.tsx' -B 2 -A 2Repository: triggerdotdev/trigger.dev
Length of output: 1606
🏁 Script executed:
#!/bin/bash
# Check if there are any tests or comments explaining this gating behavior
rg 'githubAppEnabled.*buildSettings' -g '*.ts' -g '*.tsx' -B 2 -A 2Repository: triggerdotdev/trigger.dev
Length of output: 867
🏁 Script executed:
#!/bin/bash
# Check the general settings route to see if it has similar gating
cat -n apps/webapp/app/routes/_app.orgs.\$organizationSlug.projects.\$projectParam.env.\$envParam.settings.general/route.tsx | grep -A 30 "return ("Repository: triggerdotdev/trigger.dev
Length of output: 1676
🏁 Script executed:
#!/bin/bash
# Search for any TODOs or FIXMEs related to this gating
rg 'TODO|FIXME' -g '*.ts' -g '*.tsx' -B 2 -A 2 | grep -i -C 3 'github.*build\|build.*github\|integrations'Repository: triggerdotdev/trigger.dev
Length of output: 1382
🏁 Script executed:
#!/bin/bash
# Check if buildSettings are used anywhere else when githubAppEnabled is false
rg 'buildSettings' -g '*.ts' -g '*.tsx' -B 3 -A 3 | grep -C 5 'githubAppEnabled.*false'Repository: triggerdotdev/trigger.dev
Length of output: 51
🏁 Script executed:
#!/bin/bash
# Check if the action handler validates githubAppEnabled before processing build settings
sed -n '121,173p' apps/webapp/app/routes/_app.orgs.\$organizationSlug.projects.\$projectParam.env.\$envParam.settings.integrations/route.tsxRepository: triggerdotdev/trigger.dev
Length of output: 1878
🏁 Script executed:
#!/bin/bash
# Look for any architectural docs or comments about why Build settings require GitHub app
find . -name '*.md' -o -name '*.txt' | xargs grep -l 'build.*settings\|github.*build' 2>/dev/null | head -5Repository: triggerdotdev/trigger.dev
Length of output: 128
🏁 Script executed:
#!/bin/bash
# Check BranchesPresenter to understand the learning context better
rg 'BranchesPresenter' -g '*.ts' -g '*.tsx' -l | head -3Repository: triggerdotdev/trigger.dev
Length of output: 293
🏁 Script executed:
#!/bin/bash
# Check if ProjectSettingsService.updateBuildSettings has any githubAppEnabled checks
rg 'updateBuildSettings' -g '*.ts' -g '*.tsx' -B 5 -A 10Repository: triggerdotdev/trigger.dev
Length of output: 3817
🏁 Script executed:
#!/bin/bash
# Look for empty state patterns in the webapp
rg 'empty.*state|no.*settings' -g '*.tsx' -i -B 2 -A 2 | head -50Repository: triggerdotdev/trigger.dev
Length of output: 911
🏁 Script executed:
#!/bin/bash
# Check if there's a pattern of when githubAppEnabled affects what's rendered
rg 'githubAppEnabled.*\?' -g '*.tsx' -B 2 -A 5Repository: triggerdotdev/trigger.dev
Length of output: 51
🏁 Script executed:
#!/bin/bash
# Check if there's any error handling or empty state in IntegrationsSettingsPage when githubAppEnabled is false
sed -n '289,357p' apps/webapp/app/routes/_app.orgs.\$organizationSlug.projects.\$projectParam.env.\$envParam.settings.integrations/route.tsxRepository: triggerdotdev/trigger.dev
Length of output: 2933
🏁 Script executed:
#!/bin/bash
# Search for related issues or PRs that discuss this gating pattern
git log --oneline --all --grep='build.*settings\|github.*app' 2>/dev/null | head -10Repository: triggerdotdev/trigger.dev
Length of output: 752
🏁 Script executed:
#!/bin/bash
# Check the specific commits about build settings to understand the design intent
git show 501a383b --stat 2>/dev/null | head -20Repository: triggerdotdev/trigger.dev
Length of output: 907
🏁 Script executed:
#!/bin/bash
# Look for any comments in the code about why build settings require GitHub
rg -i 'build.*prerequisite|github.*build|require.*github' -g '*.ts' -g '*.tsx' -B 2 -A 2Repository: triggerdotdev/trigger.dev
Length of output: 4541
🏁 Script executed:
#!/bin/bash
# Check if there are any tests for this route that explain the gating behavior
find . -name '*.test.*' -o -name '*.spec.*' | xargs grep -l 'IntegrationsSettingsPage\|githubAppEnabled.*buildSettings' 2>/dev/null | head -5Repository: triggerdotdev/trigger.dev
Length of output: 51
🏁 Script executed:
#!/bin/bash
# Look at the related PR or discussions about this design
git log --all --oneline --grep='integrations.*settings\|build.*settings' | head -10Repository: triggerdotdev/trigger.dev
Length of output: 360
🏁 Script executed:
#!/bin/bash
# Check if there's documentation about when GitHub app is required
find . -name '*.md' | xargs grep -i 'github.*app.*required\|build.*settings.*github' 2>/dev/null | head -10Repository: triggerdotdev/trigger.dev
Length of output: 51
Add an empty state message or ensure backend validates githubAppEnabled before allowing build settings updates.
When the GitHub app is disabled, the entire integration panel (including Build settings) correctly hides, since the GitHub app is responsible for builds per the system architecture. However, there's no empty state message to guide users, leaving a blank page. Additionally, the action handler should validate that githubAppEnabled is true before processing build settings updates, rather than relying solely on UI gating.
Either render an informational empty state explaining the GitHub app requirement, or add backend validation in the action handler to match the UI's gating behavior.
| integrationId: slackIntegration.id, | ||
| }); | ||
|
|
||
| return redirect(`/orgs/${organizationSlug}/settings`); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, find and inspect the pathBuilder.ts file to see available path builders
cat apps/webapp/app/utils/pathBuilder.ts | head -200Repository: triggerdotdev/trigger.dev
Length of output: 5553
🏁 Script executed:
#!/bin/bash
# Check for organizationSettings related path builders
rg -n 'organizationSettings' apps/webapp/app/utils/pathBuilder.tsRepository: triggerdotdev/trigger.dev
Length of output: 141
🏁 Script executed:
#!/bin/bash
# Get the exact context of line 186 in the file under review
sed -n '180,195p' apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.integrations.slack.tsxRepository: triggerdotdev/trigger.dev
Length of output: 177
🏁 Script executed:
#!/bin/bash
# Find the file with proper handling of special characters
find apps/webapp/app/routes -name "*slack.tsx" -type fRepository: triggerdotdev/trigger.dev
Length of output: 151
🏁 Script executed:
#!/bin/bash
# Once we find it, inspect the context around line 186
fd "slack.tsx" apps/webapp/app/routes --exec wc -l {} \;Repository: triggerdotdev/trigger.dev
Length of output: 155
🏁 Script executed:
#!/bin/bash
# Get context around line 186
sed -n '180,195p' apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.integrations.slack.tsxRepository: triggerdotdev/trigger.dev
Length of output: 177
🏁 Script executed:
#!/bin/bash
# Check the imports at the top of the file to see if organizationSettingsPath is imported
head -50 apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.integrations.slack.tsxRepository: triggerdotdev/trigger.dev
Length of output: 193
🏁 Script executed:
#!/bin/bash
# Use fd to find and directly read the file
fd "slack.tsx" apps/webapp/app/routes --exec cat {} \; | sed -n '180,195p'Repository: triggerdotdev/trigger.dev
Length of output: 602
🏁 Script executed:
#!/bin/bash
# Get imports from the file
fd "slack.tsx" apps/webapp/app/routes --exec head -50 {} \;Repository: triggerdotdev/trigger.dev
Length of output: 1857
🏁 Script executed:
#!/bin/bash
# Check if organizationSettingsPath is imported in the file
fd "slack.tsx" apps/webapp/app/routes --exec grep -n "organizationSettingsPath" {} \;Repository: triggerdotdev/trigger.dev
Length of output: 51
🏁 Script executed:
#!/bin/bash
# Check the full imports section and verify what's imported from pathBuilder
fd "slack.tsx" apps/webapp/app/routes --exec grep -A 5 "from \"~/utils/pathBuilder\"" {} \;Repository: triggerdotdev/trigger.dev
Length of output: 290
🏁 Script executed:
#!/bin/bash
# Check if organizationSettingsPath is used elsewhere in the codebase
rg -n "organizationSettingsPath" apps/webapp/app --type ts --type tsx -B 2 -A 2 | head -50Repository: triggerdotdev/trigger.dev
Length of output: 96
🏁 Script executed:
#!/bin/bash
# Check how path builders are typically imported and used in similar files
rg -n "redirect.*Path\(" apps/webapp/app/routes --type tsx -B 2 -A 2 | head -60Repository: triggerdotdev/trigger.dev
Length of output: 96
🏁 Script executed:
#!/bin/bash
# Search for organizationSettingsPath usage in the webapp
rg -n "organizationSettingsPath" apps/webapp/app -B 2 -A 2 | head -80Repository: triggerdotdev/trigger.dev
Length of output: 5695
🏁 Script executed:
#!/bin/bash
# Check how path builders are typically used in redirect statements
rg -n "return redirect.*Path\(" apps/webapp/app/routes -B 1 -A 1 | head -100Repository: triggerdotdev/trigger.dev
Length of output: 7968
Use path builder utility for organization settings redirect.
The hardcoded redirect path should use organizationSettingsPath() to maintain consistency with other routes in the codebase and avoid fragility if the routing structure changes.
Proposed fix
+ import { organizationSettingsPath } from "~/utils/pathBuilder";
...
- return redirect(`/orgs/${organizationSlug}/settings`);
+ return redirect(organizationSettingsPath({ slug: organizationSlug }));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return redirect(`/orgs/${organizationSlug}/settings`); | |
| import { organizationSettingsPath } from "~/utils/pathBuilder"; | |
| return redirect(organizationSettingsPath({ slug: organizationSlug })); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/webapp/app/routes/_app.orgs`.$organizationSlug.settings.integrations.slack.tsx
at line 186, The redirect uses a hardcoded string
(`redirect(`/orgs/${organizationSlug}/settings`)`) which is fragile; replace it
to use the shared path builder `organizationSettingsPath()` so the route stays
consistent—call `organizationSettingsPath(organizationSlug)` (or the correct arg
shape) in place of the hardcoded path in the function where the redirect occurs
to return `redirect(organizationSettingsPath(...))`.
| export function buildVercelDeploymentUrl( | ||
| vercelTeamSlug: string | undefined, | ||
| vercelProjectName: string, | ||
| integrationDeploymentId: string | ||
| ): string { | ||
| const vercelId = integrationDeploymentId.replace(/^dpl_/, ""); | ||
| return `https://vercel.com/${vercelTeamSlug}/${vercelProjectName}/${vercelId}`; | ||
| } |
There was a problem hiding this comment.
vercelTeamSlug: string | undefined produces a broken URL when undefined.
If vercelTeamSlug is undefined, the function returns https://vercel.com/undefined/${vercelProjectName}/${vercelId}, which is an invalid Vercel URL. Current callers guard against this, but the function signature is misleading — it accepts undefined yet cannot handle it correctly.
Either narrow the parameter type to string (preferred, making callers responsible for the guard) or return undefined when vercelTeamSlug is missing.
🛡️ Suggested fix — narrow the parameter type
export function buildVercelDeploymentUrl(
- vercelTeamSlug: string | undefined,
+ vercelTeamSlug: string,
vercelProjectName: string,
integrationDeploymentId: string
): string {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/webapp/app/v3/vercel/vercelProjectIntegrationSchema.ts` around lines 205
- 212, The buildVercelDeploymentUrl function currently accepts vercelTeamSlug:
string | undefined but produces a broken URL when undefined; change the
parameter type to vercelTeamSlug: string so callers must validate/preserve a
team slug before calling buildVercelDeploymentUrl (or alternatively make the
function return undefined when vercelTeamSlug is missing), and update any
callers that pass undefined to either perform the guard or handle the undefined
return. Ensure the change targets the buildVercelDeploymentUrl signature and all
call sites that rely on passing undefined.
0ski
changed the title
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| await this.removeAllVercelEnvVarsByKey({ | ||
| client, | ||
| vercelProjectId: params.vercelProjectId, | ||
| teamId: params.teamId, | ||
| key: "TRIGGER_SECRET_KEY", | ||
| }); |
There was a problem hiding this comment.
🔴 Non-atomic delete-then-create in syncApiKeysToVercel can permanently lose TRIGGER_SECRET_KEY env vars on partial failure
In syncApiKeysToVercel, the new code first deletes ALL TRIGGER_SECRET_KEY env vars from Vercel via removeAllVercelEnvVarsByKey, and then attempts to recreate them via batchUpsertVercelEnvVars. If the delete succeeds but the subsequent create/upsert fails (e.g., Vercel API outage, rate limiting, network error), all TRIGGER_SECRET_KEY environment variables are permanently lost from the Vercel project with no automatic recovery.
Root Cause and Impact
Before this change, syncApiKeysToVercel only called batchUpsertVercelEnvVars, which safely found existing env vars and updated them in place, or created new ones if missing. This was an atomic-per-variable approach.
The new code at apps/webapp/app/models/vercelIntegration.server.ts:978-983 adds a removeAllVercelEnvVarsByKey call before the upsert:
await this.removeAllVercelEnvVarsByKey({
client,
vercelProjectId: params.vercelProjectId,
teamId: params.teamId,
key: "TRIGGER_SECRET_KEY",
});
const result = await this.batchUpsertVercelEnvVars({ ... });The batchUpsertVercelEnvVars at line 1419 makes another API call to filterProjectEnvs which will find no matching vars (since they were just deleted), then calls createProjectEnv to create them. If this create call fails, the secret keys are gone from Vercel.
Impact: Production, staging, and preview Vercel deployments would lose their TRIGGER_SECRET_KEY environment variable, causing Trigger.dev task execution to fail for all affected environments until the keys are manually re-synced.
Prompt for agents
In apps/webapp/app/models/vercelIntegration.server.ts, inside the syncApiKeysToVercel method (around lines 978-1001), refactor the delete-then-create approach to be more resilient. Instead of calling removeAllVercelEnvVarsByKey first (which permanently deletes before the replacement is guaranteed), either:
1. Perform the batch creation first, and only then remove any stale env vars that no longer match the current set of targets. This way, if the creation fails, the old keys are still in place.
2. Alternatively, wrap the operation so that if batchUpsertVercelEnvVars fails after removeAllVercelEnvVarsByKey, a retry/recovery path is attempted to re-create the env vars.
The goal is to never leave the Vercel project in a state where TRIGGER_SECRET_KEY env vars have been deleted but not replaced.
Was this helpful? React with 👍 or 👎 to provide feedback.
| OR: [ | ||
| { integrationId: slackIntegration.id }, | ||
| { | ||
| properties: { | ||
| path: ["integrationId"], | ||
| equals: slackIntegration.id, | ||
| }, | ||
| }, | ||
| ], |
There was a problem hiding this comment.
🚩 Slack alert channel query uses OR with integrationId and JSON path — verify both are needed
At apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.integrations.slack.tsx:77-85, the query for Slack alert channels uses OR: [{ integrationId: slackIntegration.id }, { properties: { path: ['integrationId'], equals: slackIntegration.id } }]. This suggests the integration ID may be stored in two different places depending on when/how the channel was created. The same pattern appears in the uninstall transaction at line 140-148. This is likely correct for backward compatibility but worth verifying that both paths are necessary and that no channels are missed.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Closes #
✅ Checklist
Testing
Slack + GitHub + Vercel + Builds + Deployments
Changelog
Settings changes:
Vercel improvements:
Slack improvements:
Webhook/Email alerts:
Screenshots
💯