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 `<App>` 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 <noreply@anthropic.com>
This commit is contained in:
Tonio 2026-08-15 01:54:47 -07:00 committed by GitHub
parent 6542ad1f4d
commit b38d6ddb81
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 238 additions and 5 deletions

View File

@ -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 <App>'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<typeof import("react-router-dom")>("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(
<QueryClientProvider client={queryClient}>
<OnboardingRoutePage />
</QueryClientProvider>,
);
});
}
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();
});
});

View File

@ -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() {
<Button
onClick={() =>
matchedCompany
? openOnboarding({ initialStep: 2, companyId: matchedCompany.id })
? openOnboarding({
// "Add another agent" to a company that already has its
// mission must not stop to ask for the mission again. An
// unsettled or failed lookup reads as "no mission" and
// costs the step, which the customer can pass - and the
// mission step now updates the existing goal rather than
// adding a second one.
initialStep: onboardingStepForCompany(hasMission),
companyId: matchedCompany.id,
})
: openOnboarding()
}
>