Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions .vscode/launch.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,15 @@
"cwd": "${workspaceFolder}/apps/webapp",
"sourceMaps": true
},
{
"type": "node-terminal",
"request": "launch",
"name": "Debug opened test file",
"command": "pnpm run test -- ./${relativeFile}",
"envFile": "${workspaceFolder}/.env",
"cwd": "${workspaceFolder}",
"sourceMaps": true
},
{
"type": "chrome",
"request": "launch",
Expand Down
86 changes: 78 additions & 8 deletions apps/webapp/app/components/integrations/VercelOnboardingModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ import { type VercelOnboardingData } from "~/presenters/v3/VercelSettingsPresent
import { vercelAppInstallPath, v3ProjectSettingsPath, githubAppInstallPath, vercelResourcePath } from "~/utils/pathBuilder";
import type { loader } from "~/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.vercel";
import { useEffect, useState, useCallback, useRef } from "react";
import { usePostHogTracking } from "~/hooks/usePostHog";

function safeRedirectUrl(url: string): string | null {
try {
Expand Down Expand Up @@ -114,6 +115,7 @@ export function VercelOnboardingModal({
nextUrl?: string;
onDataReload?: (vercelStagingEnvironment?: string) => void;
}) {
const { capture, startSessionRecording } = usePostHogTracking();
const navigation = useNavigation();
const fetcher = useTypedFetcher<typeof loader>();
const envMappingFetcher = useFetcher();
Expand Down Expand Up @@ -172,6 +174,31 @@ export function VercelOnboardingModal({
prevIsOpenRef.current = isOpen;
}, [isOpen, state, computeInitialState]);

const trackOnboarding = useCallback(
(eventName: string, extraProperties?: Record<string, unknown>) => {
capture(eventName, {
origin: fromMarketplaceContext ? "marketplace" : "dashboard",
step: state,
organization_slug: organizationSlug,
project_slug: projectSlug,
...extraProperties,
});
},
[capture, fromMarketplaceContext, state, organizationSlug, projectSlug]
);

const hasTrackedStartRef = useRef(false);
useEffect(() => {
if (isOpen && state === "project-selection" && !hasTrackedStartRef.current) {
hasTrackedStartRef.current = true;
startSessionRecording();
trackOnboarding("vercel onboarding started");
}
if (!isOpen) {
hasTrackedStartRef.current = false;
}
}, [isOpen, state, trackOnboarding, startSessionRecording]);

const [selectedVercelProject, setSelectedVercelProject] = useState<{
id: string;
name: string;
Expand Down Expand Up @@ -337,14 +364,17 @@ export function VercelOnboardingModal({

useEffect(() => {
if (state === "project-selection" && fetcher.data && "success" in fetcher.data && fetcher.data.success && fetcher.state === "idle") {
trackOnboarding("vercel onboarding project selected", {
vercel_project_name: selectedVercelProject?.name,
});
setState("loading-env-mapping");
if (onDataReload) {
onDataReload();
}
} else if (fetcher.data && "error" in fetcher.data && typeof fetcher.data.error === "string") {
setProjectSelectionError(fetcher.data.error);
}
}, [state, fetcher.data, fetcher.state, onDataReload]);
}, [state, fetcher.data, fetcher.state, onDataReload, trackOnboarding, selectedVercelProject?.name]);

// For marketplace origin, skip env-mapping step
useEffect(() => {
Expand Down Expand Up @@ -449,14 +479,23 @@ export function VercelOnboardingModal({
method: "post",
action: actionUrl,
});
Comment on lines 479 to 481

Choose a reason for hiding this comment

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

🚩 trackOnboarding in handleSkipOnboarding dependency array but never called

trackOnboarding was added to the dependency array of handleSkipOnboarding at line 481 but is never invoked inside the function body. This contrasts with the Dialog's onOpenChange handler (lines 657-663) which does track "vercel onboarding abandoned" when the dialog is dismissed via overlay click or Escape key.

However, when the user clicks "Cancel" (which calls handleSkipOnboardingonClose()), the Dialog's onOpenChange does not fire because the open prop changes programmatically rather than via user interaction on the dialog overlay. This means cancellation via the Cancel button goes untracked, while cancellation via clicking outside the dialog is tracked.

This may be an oversight where the developer intended to add trackOnboarding("vercel onboarding abandoned") or a similar call inside handleSkipOnboarding but forgot. The unnecessary dependency also causes the callback to be recreated whenever state changes (since trackOnboarding depends on state).

(Refers to lines 469-481)

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

}, [actionUrl, fetcher, onClose, nextUrl, fromMarketplaceContext]);
}, [actionUrl, fetcher, onClose, nextUrl, fromMarketplaceContext, trackOnboarding]);

const handleSkipEnvMapping = useCallback(() => {
trackOnboarding("vercel onboarding env mapping completed", {
skipped: true,
staging_environment: null,
});
setVercelStagingEnvironment(null);
setState("loading-env-vars");
}, []);
}, [trackOnboarding]);

const handleUpdateEnvMapping = useCallback(() => {
trackOnboarding("vercel onboarding env mapping completed", {
skipped: false,
staging_environment: vercelStagingEnvironment?.displayName ?? null,
});

if (!vercelStagingEnvironment) {
setState("loading-env-vars");
return;
Expand All @@ -471,9 +510,11 @@ export function VercelOnboardingModal({
action: actionUrl,
});

}, [vercelStagingEnvironment, envMappingFetcher, actionUrl]);
}, [vercelStagingEnvironment, envMappingFetcher, actionUrl, trackOnboarding]);

const handleBuildSettingsNext = useCallback(() => {
trackOnboarding("vercel onboarding build settings completed");

if (nextUrl && fromMarketplaceContext && isGitHubConnectedForOnboarding) {
setIsRedirecting(true);
}
Expand Down Expand Up @@ -501,7 +542,7 @@ export function VercelOnboardingModal({
if (!isGitHubConnectedForOnboarding) {
setState("github-connection");
}
}, [vercelStagingEnvironment, pullEnvVarsBeforeBuild, atomicBuilds, discoverEnvVars, syncEnvVarsMapping, nextUrl, fromMarketplaceContext, isGitHubConnectedForOnboarding, completeOnboardingFetcher, actionUrl]);
}, [vercelStagingEnvironment, pullEnvVarsBeforeBuild, atomicBuilds, discoverEnvVars, syncEnvVarsMapping, nextUrl, fromMarketplaceContext, isGitHubConnectedForOnboarding, completeOnboardingFetcher, actionUrl, trackOnboarding]);

const handleFinishOnboarding = useCallback((e: React.FormEvent<HTMLFormElement>) => {
e.preventDefault();
Expand Down Expand Up @@ -531,9 +572,12 @@ export function VercelOnboardingModal({

useEffect(() => {
if (state === "completed") {
trackOnboarding("vercel onboarding completed", {
github_connected: isGitHubConnectedForOnboarding,
});
onClose();
}
}, [state, onClose]);
}, [state, onClose, trackOnboarding, isGitHubConnectedForOnboarding]);
Comment on lines 573 to +580
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Potential double-fire of "completed" event if deps change while state === "completed".

If trackOnboarding or isGitHubConnectedForOnboarding changes while state is still "completed" (before the parent unmounts/hides the modal), this effect re-runs and emits the event again. Consider adding a ref guard similar to hasTrackedStartRef to ensure the completion event fires exactly once.

Suggested fix
+  const hasTrackedCompletedRef = useRef(false);
   useEffect(() => {
     if (state === "completed") {
-      trackOnboarding("vercel onboarding completed", {
-        github_connected: isGitHubConnectedForOnboarding,
-      });
+      if (!hasTrackedCompletedRef.current) {
+        hasTrackedCompletedRef.current = true;
+        trackOnboarding("vercel onboarding completed", {
+          github_connected: isGitHubConnectedForOnboarding,
+        });
+      }
       onClose();
     }
   }, [state, onClose, trackOnboarding, isGitHubConnectedForOnboarding]);

Reset the ref when the modal reopens (alongside the existing hasTrackedStartRef reset):

   if (!isOpen) {
     hasTrackedStartRef.current = false;
+    hasTrackedCompletedRef.current = false;
   }
🤖 Prompt for AI Agents
In `@apps/webapp/app/components/integrations/VercelOnboardingModal.tsx` around
lines 573 - 580, The effect that calls trackOnboarding when state ===
"completed" can fire multiple times if dependencies change; add a ref guard
(e.g., hasTrackedCompleteRef) analogous to hasTrackedStartRef and check it
inside the useEffect for completion to ensure trackOnboarding("vercel onboarding
completed", ...) runs only once, set the ref after firing, and reset this ref
when the modal reopens (same place where hasTrackedStartRef is reset) so the
completion event can fire again on a new session; update the effect that depends
on state, trackOnboarding, isGitHubConnectedForOnboarding, and onClose to
consult and set hasTrackedCompleteRef accordingly.


useEffect(() => {
if (state === "installing") {
Expand Down Expand Up @@ -565,6 +609,12 @@ export function VercelOnboardingModal({
}
}, [state, customEnvironments, vercelStagingEnvironment]);

useEffect(() => {
if (state === "project-selection" && availableProjects.length > 0 && !selectedVercelProject) {
setSelectedVercelProject(availableProjects[0]);
}
}, [state, availableProjects, selectedVercelProject]);

if (!isOpen || onboardingData?.authInvalid) {
return null;
}
Expand All @@ -578,7 +628,14 @@ export function VercelOnboardingModal({

if (isLoadingState) {
return (
<Dialog open={isOpen} onOpenChange={(open) => !open && !fromMarketplaceContext && onClose()}>
<Dialog open={isOpen} onOpenChange={(open) => {
if (!open && !fromMarketplaceContext) {
if (state as string !== "completed") {
trackOnboarding("vercel onboarding abandoned");
}
onClose();
}
}}>
<DialogContent className="max-w-lg">
<DialogHeader>
<div className="flex items-center gap-2">
Expand All @@ -601,7 +658,14 @@ export function VercelOnboardingModal({
const showGitHubConnection = state === "github-connection";

return (
<Dialog open={isOpen} onOpenChange={(open) => !open && !fromMarketplaceContext && onClose()}>
<Dialog open={isOpen} onOpenChange={(open) => {
if (!open && !fromMarketplaceContext) {
if (state !== "completed") {
trackOnboarding("vercel onboarding abandoned");
}
onClose();
}
}}>
<DialogContent className="max-w-lg">
<DialogHeader>
<div className="flex items-center gap-2">
Expand All @@ -625,6 +689,7 @@ export function VercelOnboardingModal({
</Callout>
) : (
<Select
disabled={availableProjects.length === 1}
value={selectedVercelProject?.id || ""}
setValue={(value) => {
if (!Array.isArray(value)) {
Expand All @@ -634,6 +699,7 @@ export function VercelOnboardingModal({
}
}}
items={availableProjects}
filter={availableProjects.length > 5 ? { keys: ["name"] } : undefined}
variant="tertiary/medium"
placeholder="Select a Vercel project"
dropdownIcon
Expand Down Expand Up @@ -894,6 +960,10 @@ export function VercelOnboardingModal({
<Button
variant="primary/medium"
onClick={() => {
trackOnboarding("vercel onboarding env vars configured", {
env_vars_enabled: enabledEnvVars.length,
env_vars_total: syncableEnvVars.length,
});
if (fromMarketplaceContext) {
handleBuildSettingsNext();
} else {
Expand Down
17 changes: 16 additions & 1 deletion apps/webapp/app/hooks/usePostHog.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { useLocation } from "@remix-run/react";
import posthog from "posthog-js";
import { useEffect, useRef } from "react";
import { useCallback, useEffect, useRef } from "react";
import { useOrganizationChanged } from "./useOrganizations";
import { useOptionalUser, useUserChanged } from "./useUser";
import { useProjectChanged } from "./useProject";
Expand Down Expand Up @@ -68,3 +68,18 @@ export const usePostHog = (apiKey?: string, logging = false, debug = false): voi
posthog.capture("$pageview");
}, [location, logging]);
};

export function usePostHogTracking() {
const capture = useCallback(
(eventName: string, properties?: Record<string, unknown>) => {
posthog.capture(eventName, properties);
},
[]
);

const startSessionRecording = useCallback(() => {
posthog.startSessionRecording();
}, []);

return { capture, startSessionRecording };
}
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ export async function action({ request, params }: ActionFunctionArgs) {
contentHash: body.data.contentHash,
git: body.data.gitMeta,
runtime: body.data.runtime,
buildServerMetadata: body.data.buildServerMetadata,
})
.match(
() => {
Expand Down
59 changes: 52 additions & 7 deletions apps/webapp/app/v3/services/deployment.server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,12 @@ import { type AuthenticatedEnvironment } from "~/services/apiAuth.server";
import { BaseService } from "./baseService.server";
import { errAsync, fromPromise, okAsync, type ResultAsync } from "neverthrow";
import { type WorkerDeployment, type Project } from "@trigger.dev/database";
import { BuildServerMetadata, logger, type GitMeta, type DeploymentEvent } from "@trigger.dev/core/v3";
import {
BuildServerMetadata,
logger,
type GitMeta,
type DeploymentEvent,
} from "@trigger.dev/core/v3";
import { TimeoutDeploymentService } from "./timeoutDeployment.server";
import { env } from "~/env.server";
import { createRemoteImageBuild } from "../remoteImageBuilder.server";
Expand Down Expand Up @@ -38,9 +43,20 @@ export class DeploymentService extends BaseService {
public progressDeployment(
authenticatedEnv: AuthenticatedEnvironment,
friendlyId: string,
updates: Partial<Pick<WorkerDeployment, "contentHash" | "runtime"> & { git: GitMeta }>
updates: Partial<
Pick<WorkerDeployment, "contentHash" | "runtime"> & {
git: GitMeta;
buildServerMetadata: BuildServerMetadata;
}
>
) {
const validateDeployment = (deployment: Pick<WorkerDeployment, "id" | "status"> & { buildServerMetadata?: BuildServerMetadata }) => {
const { buildServerMetadata: newBuildServerMetadata, ...restUpdates } = updates;

const validateDeployment = (
deployment: Pick<WorkerDeployment, "id" | "status"> & {
buildServerMetadata?: BuildServerMetadata;
}
) => {
if (deployment.status !== "PENDING" && deployment.status !== "INSTALLING") {
logger.warn(
"Attempted progressing deployment that is not in PENDING or INSTALLING status",
Expand All @@ -54,12 +70,27 @@ export class DeploymentService extends BaseService {
return okAsync(deployment);
};

const progressToInstalling = (deployment: Pick<WorkerDeployment, "id">) =>
fromPromise(
const progressToInstalling = (
deployment: Pick<WorkerDeployment, "id"> & { buildServerMetadata?: BuildServerMetadata }
) => {
const existingBuildServerMetadata = deployment.buildServerMetadata as
| BuildServerMetadata
| null
| undefined;

return fromPromise(
this._prisma.workerDeployment.updateMany({
where: { id: deployment.id, status: "PENDING" }, // status could've changed in the meantime, we're not locking the row
data: {
...updates,
...restUpdates,
...(newBuildServerMetadata
? {
buildServerMetadata: {
...(existingBuildServerMetadata ?? {}),
...newBuildServerMetadata,
},
}
: {}),
status: "INSTALLING",
startedAt: new Date(),
},
Expand All @@ -74,6 +105,7 @@ export class DeploymentService extends BaseService {
}
return okAsync({ id: deployment.id, status: "INSTALLING" as const });
});
};

const progressToBuilding = (
deployment: Pick<WorkerDeployment, "id"> & { buildServerMetadata?: BuildServerMetadata }
Expand All @@ -85,13 +117,26 @@ export class DeploymentService extends BaseService {
cause: error,
}));

const existingBuildServerMetadata = deployment.buildServerMetadata as
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: can avoid the typeassertion by doing a safe parse in getDeployment

| BuildServerMetadata
| null
| undefined;

return createRemoteBuildIfNeeded
.andThen((externalBuildData) =>
fromPromise(
this._prisma.workerDeployment.updateMany({
where: { id: deployment.id, status: "INSTALLING" }, // status could've changed in the meantime, we're not locking the row
data: {
...updates,
...restUpdates,
...(newBuildServerMetadata
? {
buildServerMetadata: {
...(existingBuildServerMetadata ?? {}),
...newBuildServerMetadata,
},
}
: {}),
externalBuildData,
status: "BUILDING",
installedAt: new Date(),
Expand Down
4 changes: 3 additions & 1 deletion apps/webapp/app/v3/services/initializeDeployment.server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,7 @@ export class InitializeDeploymentService extends BaseService {
artifactKey: payload.artifactKey,
skipPromotion: payload.skipPromotion,
configFilePath: payload.configFilePath,
skipEnqueue: payload.skipEnqueue,
}
: {}),
}
Expand Down Expand Up @@ -238,7 +239,8 @@ export class InitializeDeploymentService extends BaseService {
new Date(Date.now() + timeoutMs)
);

if (payload.isNativeBuild) {
// For github integration there is no artifactKey, hence we skip it here
if (payload.isNativeBuild && payload.artifactKey && !payload.skipEnqueue) {
const result = await deploymentService
.enqueueBuild(environment, deployment, payload.artifactKey, {
skipPromotion: payload.skipPromotion,
Expand Down
Loading