From b38d6ddb811b3ff3145ff5df60174eb9e7313fe6 Mon Sep 17 00:00:00 2001 From: Tonio Date: Sat, 15 Aug 2026 01:54:47 -0700 Subject: [PATCH] fix(onboarding): choose the launcher's step from the company's mission too (#11429) The last of the three ways into onboarding for a company that already exists. The route resolver and the dashboard both pick the step from whether the company already has its mission; the "Add Agent" card on `/{prefix}/onboarding` hardcoded the mission step. So the one entry point whose own copy reads "Add another agent to X" was the one that stopped to ask X for the mission it already had. It now calls `onboardingStepForCompany`, like the other two. `matchedCompany` moves above the early return because a hook cannot be called after it. An unsettled or failed lookup still reads as "no mission" and costs the step, which the customer can answer - the same fail-open rule the other callers follow, and safe now that confirming the mission updates the company's existing goal rather than adding a second one. `OnboardingRoutePage` is exported so this can be driven directly. The alternative was the whole `` route table, which is a much heavier harness for a question about one button's argument. Four cases, and the first fails against the hardcoded step. The button lookup asserts it matched something before clicking, because a lookup that silently matches nothing turns the click into a no-op and the test into decoration. This is the last piece of #11259 that had not landed. ui typecheck clean; full ui suite 4010 pass. The one failure, in IssueProperties, is timezone-dependent and reproduces on master. Co-Authored-By: Claude Opus 5 --- ui/src/App.onboarding-launcher.test.tsx | 218 ++++++++++++++++++++++++ ui/src/App.tsx | 25 ++- 2 files changed, 238 insertions(+), 5 deletions(-) create mode 100644 ui/src/App.onboarding-launcher.test.tsx diff --git a/ui/src/App.onboarding-launcher.test.tsx b/ui/src/App.onboarding-launcher.test.tsx new file mode 100644 index 0000000000..84e14e530c --- /dev/null +++ b/ui/src/App.onboarding-launcher.test.tsx @@ -0,0 +1,218 @@ +// @vitest-environment jsdom + +import { act } from "react"; +import { createRoot, type Root } from "react-dom/client"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { + ONBOARDING_AGENT_STEP, + ONBOARDING_MISSION_STEP, +} from "./lib/onboarding-route"; + +/** + * The third way into onboarding for a company that already exists: the "Add + * Agent" card on `/{prefix}/onboarding`, shown once the wizard is dismissed. + * + * The route resolver and the dashboard both choose the step from whether the + * company already has its mission. This button hardcoded the mission step, so + * the one entry point whose own copy says "add another agent" was the one that + * stopped to ask for the mission again. + */ + +// eslint-disable-next-line @typescript-eslint/no-explicit-any +(globalThis as any).IS_REACT_ACT_ENVIRONMENT = true; + +// jsdom's CSS parser rejects the custom-property marker rule stitches inserts +// (`--sxs{--sxs:N}`), pulled into 's eager import graph transitively via +// @codesandbox/sandpack-react. Substitute a benign, valid rule on parse failure +// so stitches' index bookkeeping stays intact and the module graph evaluates. +vi.hoisted(() => { + const sheetProto = window.CSSStyleSheet.prototype as unknown as { + insertRule: (rule: string, index?: number) => number; + __papLauncherPatched?: boolean; + }; + if (!sheetProto.__papLauncherPatched) { + const original = sheetProto.insertRule; + sheetProto.insertRule = function patched(this: CSSStyleSheet, rule: string, index?: number) { + try { + return original.call(this, rule, index); + } catch { + try { + return original.call(this, ".pap-launcher-noop{}", index); + } catch { + return this.cssRules?.length ?? 0; + } + } + }; + sheetProto.__papLauncherPatched = true; + } +}); + +const routeState = vi.hoisted(() => ({ companyPrefix: "PC1" as string | undefined })); +const dialogState = vi.hoisted(() => ({ + onboardingOpen: false, + onboardingRouteDismissed: true, + openOnboarding: vi.fn(), +})); +const mockGoalsApi = vi.hoisted(() => ({ list: vi.fn() })); + +vi.mock("@/lib/router", async () => { + const actual = await vi.importActual("react-router-dom"); + return { + ...actual, + useParams: () => ({ companyPrefix: routeState.companyPrefix }), + useActiveCompanyPrefix: () => routeState.companyPrefix ?? null, + }; +}); + +vi.mock("./context/CompanyContext", () => ({ + useCompany: () => ({ + companies: [{ id: "company-1", name: "Acme", issuePrefix: "PC1" }], + selectedCompanyId: "company-1", + loading: false, + error: null, + }), +})); + +vi.mock("./context/DialogContext", () => ({ + useDialogActions: () => ({ openOnboarding: dialogState.openOnboarding }), + useDialogState: () => ({ + onboardingOpen: dialogState.onboardingOpen, + onboardingRouteDismissed: dialogState.onboardingRouteDismissed, + }), +})); + +vi.mock("./api/goals", () => ({ goalsApi: mockGoalsApi })); + +const { OnboardingRoutePage } = await import("./App"); + +const COMPANY_GOAL = { + id: "goal-1", + companyId: "company-1", + title: "Scale the marketplace", + description: null, + level: "company", + status: "active", + parentId: null, + ownerAgentId: null, + createdAt: new Date("2026-03-02T00:00:00Z"), + updatedAt: new Date("2026-03-02T00:00:00Z"), +}; + +describe("the onboarding launcher's Add Agent button", () => { + let container: HTMLDivElement; + let queryClient: QueryClient; + let root: Root | null = null; + + async function render() { + root = createRoot(container); + await act(async () => { + root!.render( + + + , + ); + }); + } + + async function settle(ticks = 12) { + for (let i = 0; i < ticks; i++) { + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 0)); + }); + } + } + + function addAgentButton(): HTMLButtonElement { + const button = [...container.querySelectorAll("button")].find((b) => + b.textContent?.includes("Add Agent"), + ); + // Asserted rather than optional-chained: a lookup that silently matches + // nothing turns the click below into a no-op and the test into decoration. + expect(button).toBeDefined(); + return button!; + } + + beforeEach(() => { + container = document.createElement("div"); + document.body.appendChild(container); + queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + routeState.companyPrefix = "PC1"; + dialogState.onboardingOpen = false; + dialogState.onboardingRouteDismissed = true; + mockGoalsApi.list.mockResolvedValue([]); + }); + + afterEach(async () => { + await act(async () => root?.unmount()); + root = null; + queryClient.clear(); + container.remove(); + document.body.innerHTML = ""; + vi.clearAllMocks(); + }); + + it("opens on the agent step for a company that already has its mission", async () => { + mockGoalsApi.list.mockResolvedValue([COMPANY_GOAL]); + await render(); + await settle(); + + await act(async () => { + addAgentButton().dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + + expect(dialogState.openOnboarding).toHaveBeenCalledWith({ + initialStep: ONBOARDING_AGENT_STEP, + companyId: "company-1", + }); + }); + + it("still asks for the mission when the company has none", async () => { + mockGoalsApi.list.mockResolvedValue([]); + await render(); + await settle(); + + await act(async () => { + addAgentButton().dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + + expect(dialogState.openOnboarding).toHaveBeenCalledWith({ + initialStep: ONBOARDING_MISSION_STEP, + companyId: "company-1", + }); + }); + + it("asks for the mission when the lookup fails, rather than skipping it", async () => { + // Same fail-open rule the other two entry points follow: an unknown + // mission costs the step, which the customer can answer. Confirming it now + // updates the company's existing goal rather than adding a second one. + mockGoalsApi.list.mockRejectedValue(new Error("goals unavailable")); + await render(); + await settle(); + + await act(async () => { + addAgentButton().dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + + expect(dialogState.openOnboarding).toHaveBeenCalledWith({ + initialStep: ONBOARDING_MISSION_STEP, + companyId: "company-1", + }); + }); + + it("opens with no company when the prefix matches none", async () => { + routeState.companyPrefix = "NOPE"; + await render(); + await settle(); + + const start = [...container.querySelectorAll("button")].find((b) => + b.textContent?.includes("Start Onboarding"), + ); + expect(start).toBeDefined(); + await act(async () => { + start!.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + + expect(dialogState.openOnboarding).toHaveBeenCalledWith(); + }); +}); diff --git a/ui/src/App.tsx b/ui/src/App.tsx index 5582747ee3..2960932266 100644 --- a/ui/src/App.tsx +++ b/ui/src/App.tsx @@ -93,8 +93,10 @@ import { useDialogActions, useDialogState } from "./context/DialogContext"; import { loadLastInboxTab } from "./lib/inbox"; import { isOnboardingWizardActive, + onboardingStepForCompany, shouldRedirectCompanylessRouteToOnboarding, } from "./lib/onboarding-route"; +import { useCompanyMission } from "./hooks/useCompanyMission"; import { normalizeRememberedInstanceSettingsPath } from "./lib/instance-settings"; function boardRoutes() { @@ -393,11 +395,18 @@ function legacyToolsRedirectTarget(tab?: string) { return `/apps/advanced/${tab}`; } -function OnboardingRoutePage() { +export function OnboardingRoutePage() { const { companies } = useCompany(); const { openOnboarding } = useDialogActions(); const { onboardingOpen, onboardingRouteDismissed } = useDialogState(); const { companyPrefix } = useParams<{ companyPrefix?: string }>(); + const matchedCompany = companyPrefix + ? companies.find((company) => company.issuePrefix.toUpperCase() === companyPrefix.toUpperCase()) ?? null + : null; + // Which step this company belongs on, by the same rule the route resolver + // and the dashboard already use. Resolved above the early return below, + // because a hook cannot be called after it. + const { hasMission } = useCompanyMission(matchedCompany?.id ?? null); // The OnboardingWizard auto-opens on this route (and can also be opened // explicitly). While it is showing it covers the whole screen, so the @@ -407,9 +416,6 @@ function OnboardingRoutePage() { if (isOnboardingWizardActive({ onboardingOpen, routeDismissed: onboardingRouteDismissed })) { return null; } - const matchedCompany = companyPrefix - ? companies.find((company) => company.issuePrefix.toUpperCase() === companyPrefix.toUpperCase()) ?? null - : null; const title = matchedCompany ? `Add another agent to ${matchedCompany.name}` @@ -431,7 +437,16 @@ function OnboardingRoutePage() {