diff --git a/ui/src/components/OnboardingWizard.step.test.tsx b/ui/src/components/OnboardingWizard.step.test.tsx index e4ac714472..0ed5cd6c6e 100644 --- a/ui/src/components/OnboardingWizard.step.test.tsx +++ b/ui/src/components/OnboardingWizard.step.test.tsx @@ -32,6 +32,14 @@ const mockGoalsApi = vi.hoisted(() => ({ update: vi.fn(), })); const mockAdaptersApi = vi.hoisted(() => ({ list: vi.fn() })); +const mockAgentsApi = vi.hoisted(() => ({ + create: vi.fn(), + adapterModels: vi.fn(), + hire: vi.fn(), + instructionsBundle: vi.fn(), + saveInstructionsFile: vi.fn(), + testEnvironment: vi.fn(), +})); const mockCompaniesApi = vi.hoisted(() => ({ create: vi.fn() })); const routerState = vi.hoisted(() => ({ pathname: "/" })); @@ -54,9 +62,7 @@ const companyState = vi.hoisted(() => ({ vi.mock("../api/goals", () => ({ goalsApi: mockGoalsApi })); vi.mock("@/api/adapters", () => ({ adaptersApi: mockAdaptersApi })); vi.mock("../api/companies", () => ({ companiesApi: mockCompaniesApi })); -vi.mock("../api/agents", () => ({ - agentsApi: { create: vi.fn(), adapterModels: vi.fn().mockResolvedValue([]) }, -})); +vi.mock("../api/agents", () => ({ agentsApi: mockAgentsApi })); vi.mock("../api/approvals", () => ({ approvalsApi: { create: vi.fn() } })); vi.mock("../api/issues", () => ({ issuesApi: { create: vi.fn() } })); vi.mock("../api/projects", () => ({ projectsApi: { list: vi.fn(), create: vi.fn() } })); @@ -176,6 +182,16 @@ describe("OnboardingWizard — which step it lands on", () => { dialogState.onboardingRouteDismissed = false; mockAdaptersApi.list.mockResolvedValue([]); mockGoalsApi.list.mockResolvedValue([]); + mockAgentsApi.adapterModels.mockResolvedValue([]); + mockAgentsApi.hire.mockResolvedValue({ agent: { id: "agent-1" }, approval: null }); + mockAgentsApi.instructionsBundle.mockResolvedValue({ entryFile: "AGENTS.md" }); + mockAgentsApi.saveInstructionsFile.mockResolvedValue({}); + mockAgentsApi.testEnvironment.mockResolvedValue({ + adapterType: "claude_local", + status: "pass", + checks: [], + testedAt: new Date("2026-03-02T00:00:00Z").toISOString(), + }); }); afterEach(async () => { @@ -702,4 +718,154 @@ describe("OnboardingWizard — which step it lands on", () => { expect(currentStep()).toBe("agent"); }); + + describe("a company that already has its mission", () => { + // It opens on the agent step, so steps 1 and 2 never run. Everything the + // mission feeds has to come from the company instead of the form. + + const MISSION_GOAL = { + ...COMPANY_GOAL, + title: "Scale the marketplace", + description: "Reach 1000 sellers", + }; + + async function openOnAgentStep() { + routerState.pathname = "/PC1/onboarding"; + mockGoalsApi.list.mockResolvedValue([MISSION_GOAL]); + await render(); + await settle(); + expect(currentStep()).toBe("agent"); + } + + it("seeds the lead agent's instructions with the mission it was never asked for", async () => { + // The regression this exists for. The agent step feeds + // `composeCeoInstructions` from the mission field, and a company entered + // here never types one — so the agent was hired knowing nothing of the + // mission the customer gave at signup, and nothing reported it. + await openOnAgentStep(); + + const next = [...document.body.querySelectorAll("button")].find((b) => + b.textContent?.includes("Next"), + )!; + await act(async () => { + next.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + await settle(); + + const connect = [...document.body.querySelectorAll("button")].find((b) => + b.textContent?.includes("Connect"), + )!; + expect(connect.hasAttribute("disabled")).toBe(false); + await act(async () => { + connect.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + await settle(); + + expect(mockAgentsApi.saveInstructionsFile).toHaveBeenCalled(); + const [, file] = mockAgentsApi.saveInstructionsFile.mock.calls[0]; + expect(file.content).toContain("Scale the marketplace"); + expect(file.content).toContain("Reach 1000 sellers"); + }); + + it("will not hire while the mission is being re-read", async () => { + // Cached goals plus an in-flight refetch: the field holds the right + // company's mission, but not necessarily its current one. Hiring inside + // that window seeds the agent from a value about to change, and reports + // nothing — the same "retained data is not an answer" rule the draft + // ownership gate follows. + await openOnAgentStep(); + + const next = [...document.body.querySelectorAll("button")].find((b) => + b.textContent?.includes("Next"), + )!; + await act(async () => { + next.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + await settle(); + expect( + [...document.body.querySelectorAll("button")] + .find((b) => b.textContent?.includes("Connect"))! + .hasAttribute("disabled"), + ).toBe(false); + + mockGoalsApi.list.mockReturnValue(new Promise(() => {})); + await act(async () => { + queryClient.invalidateQueries({ + queryKey: queryKeys.goals.list("company-1"), + }); + }); + await settle(2); + + const connect = [...document.body.querySelectorAll("button")].find((b) => + b.textContent?.includes("Connect"), + )!; + expect(connect.hasAttribute("disabled")).toBe(true); + expect(mockAgentsApi.hire).not.toHaveBeenCalled(); + }); + + it("hydrates again when the same company comes back through onboarding", async () => { + // The hydration marker is a ref, so it outlives the state it describes. + // `reset()` clears the mission field; leaving the marker set would make + // the second run believe a mission it no longer holds was already + // fetched — and hire the agent without it, exactly as before this fix. + await openOnAgentStep(); + + const close = [...document.body.querySelectorAll("button")].find((b) => + b.querySelector(".sr-only")?.textContent?.includes("Close"), + ); + expect(close).toBeDefined(); + await act(async () => { + close!.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + await settle(); + // `reset()` ran: the wizard is back at the front door with a cleared + // mission field, which is precisely the state the marker must not + // outlive. + expect(currentStep()).not.toBe("agent"); + + routerState.pathname = "/"; + await rerender(); + await settle(); + routerState.pathname = "/PC1/onboarding"; + dialogState.onboardingRouteDismissed = false; + await rerender(); + await settle(); + expect(currentStep()).toBe("agent"); + + const next = [...document.body.querySelectorAll("button")].find((b) => + b.textContent?.includes("Next"), + )!; + await act(async () => { + next.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + await settle(); + const connect = [...document.body.querySelectorAll("button")].find((b) => + b.textContent?.includes("Connect"), + )!; + await act(async () => { + connect.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + await settle(); + + expect(mockAgentsApi.saveInstructionsFile).toHaveBeenCalled(); + const [, file] = mockAgentsApi.saveInstructionsFile.mock.calls.at(-1)!; + expect(file.content).toContain("Scale the marketplace"); + }); + + it("does not offer a way back behind the step it entered on", async () => { + // Step 1 creates a company. A run that already holds one must not be + // able to walk into it, by the Back button or the progress bar. + await openOnAgentStep(); + + const back = [...document.body.querySelectorAll("button")].find((b) => + b.textContent?.includes("Back"), + ); + expect(back).toBeUndefined(); + + const nameSegment = document.body.querySelector( + '[aria-label="Step 1"]', + ) as HTMLButtonElement | null; + expect(nameSegment?.disabled).toBe(true); + }); + }); }); diff --git a/ui/src/components/OnboardingWizard.tsx b/ui/src/components/OnboardingWizard.tsx index 3dbe712eca..827e98832f 100644 --- a/ui/src/components/OnboardingWizard.tsx +++ b/ui/src/components/OnboardingWizard.tsx @@ -46,10 +46,16 @@ import { DEFAULT_CURSOR_LOCAL_MODEL } from "@paperclipai/adapter-cursor-local"; import { DEFAULT_GEMINI_LOCAL_MODEL } from "@paperclipai/adapter-gemini-local"; import { DEFAULT_OPENCODE_LOCAL_MODEL, isValidOpenCodeModelId } from "@paperclipai/adapter-opencode-local"; import { + canGoBackFromOnboardingStep, + canJumpToOnboardingStep, companyPrefixFromOnboardingPath, resolveRouteOnboardingOptions, } from "../lib/onboarding-route"; import { useCompanyMission } from "../hooks/useCompanyMission"; +import { + isExistingCompanyMissionUnresolved, + planMissionPersistence, +} from "../lib/onboarding-mission"; import { AsciiArtAnimation } from "./AsciiArtAnimation"; import { FrontDoor } from "./FrontDoor"; import { AgentCapsule } from "./AgentCapsule"; @@ -358,6 +364,12 @@ function OnboardingWizardInner({ const existingCompanyId = effectiveOnboardingOptions.companyId; const [step, setStep] = useState((saved?.step as Step) ?? initialStep); + // The step this run *entered* on, which bounds how far back it can walk. + // Captured once, when the wizard opens, for the same reason the step itself + // is: it derives from queries, so a live read would move the floor under a + // customer mid-flow — and here that would quietly re-open the "create a + // company" step to a run that already holds one. + const [entryStep, setEntryStep] = useState((saved?.step as Step) ?? initialStep); const [onboardingPath, setOnboardingPath] = useState<"create" | "grow" | null>((saved?.onboardingPath as "create" | "grow" | null) ?? null); // "Grow existing" questionnaire fields @@ -424,6 +436,54 @@ function OnboardingWizardInner({ // the user back to the route's initial step mid-flow. const createdCompanyIdRef = useRef(null); createdCompanyIdRef.current = createdCompanyId; + + // The mission of the company actually in hand, which is not always the one + // the route named - the dashboard opens the wizard with a company too. Same + // query key as the route lookup above, so when they agree this is one cache + // entry and no second request. + const { + mission: existingCompanyMission, + settled: existingMissionSettled, + fetching: existingMissionFetching, + } = useCompanyMission(createdCompanyId); + + // Seed the mission field from the company's own goal. + // + // A company that already has its mission opens on the agent step, so steps 1 + // and 2 never run and `companyGoal` stays empty. It is not only a display + // field: the Review checklist reads it, and `composeCeoInstructions` seeds + // the lead agent's instructions from it. Left empty, the agent is hired + // knowing nothing of the mission the customer gave at signup - which is the + // answer this whole flow exists to carry forward. + // + // Only when the field is empty, so a customer editing their mission is never + // overwritten by the stored copy. + const hydratedMissionForRef = useRef(null); + useEffect(() => { + if (!effectiveOnboardingOpen || !createdCompanyId) return; + if (hydratedMissionForRef.current === createdCompanyId) return; + if (!existingMissionSettled || existingMissionFetching) return; + hydratedMissionForRef.current = createdCompanyId; + if (!existingCompanyMission.goalInput) return; + setCompanyGoal((current) => (current.trim() ? current : existingCompanyMission.goalInput)); + setCreatedCompanyGoalId((current) => current ?? existingCompanyMission.goalId); + }, [ + effectiveOnboardingOpen, + createdCompanyId, + existingMissionSettled, + existingMissionFetching, + existingCompanyMission.goalInput, + existingCompanyMission.goalId, + ]); + + // Hiring seeds the agent's instructions from `companyGoal`, so it must not + // run while that field is still waiting to be hydrated - the agent would be + // created with an empty or foreign mission and nothing would report it. + const missionUnresolvedForHire = isExistingCompanyMissionUnresolved({ + existingCompanyId: createdCompanyId, + goalsLoaded: existingMissionSettled, + goalsFetching: existingMissionFetching, + }); // The step the request wants, mirrored for the same reason. `initialStep` is // *derived* - from the company list, and now from the goal list behind // `useCompanyMission` - so its value changes whenever one of those queries @@ -457,6 +517,12 @@ function OnboardingWizardInner({ setCreatedCompanyPrefix(null); setCompanyName(""); setCompanyGoal(""); + // The marker travels with the field it describes. It means "companyGoal + // holds this company's hydrated mission", so it is cleared wherever that + // field is - here and in `reset()`. Left behind, the next run believes a + // mission it no longer holds was already fetched, and hires the lead agent + // without one. + hydratedMissionForRef.current = null; setMissionPath(null); setMissionConfirmed(false); setCreatedCompanyGoalId(null); @@ -479,6 +545,7 @@ function OnboardingWizardInner({ // If explicit options are provided, they take precedence over saved state if (initialStepRef.current) { setStep(initialStepRef.current); + setEntryStep(initialStepRef.current); } const routeCompanyId = effectiveOnboardingOptions.companyId ?? null; if (routeCompanyId) { @@ -720,6 +787,8 @@ function OnboardingWizardInner({ function reset() { onboardingDraftStorage.clear(); + // Cleared with `companyGoal` below - see `clearCompanyScopedState`. + hydratedMissionForRef.current = null; setStep(0); setOnboardingPath(null); setGrowWorkflows(""); @@ -941,21 +1010,16 @@ function OnboardingWizardInner({ // without writing it would leave the company with no mission at all, // which is the state this whole change exists to remove. // - // Only when the wizard has not already written one. Returning to step 2 - // and confirming again must not add a second goal. - if (createdCompanyGoalId) { - setStep(3); - return; - } + // A goal already in hand means update it, not skip the write. It used + // to mean skip, which was safe only while the field could not hold an + // unsaved change: the id was set by *writing* the mission, so arriving + // here with one meant nothing had been typed since. Hydration breaks + // that - the id now also arrives from the company's existing goal, with + // the customer's edits sitting in the field beside it - and skipping + // would discard exactly the answer this step asked for. setLoading(true); setError(null); try { - const parsedGoal = parseOnboardingGoalInput(companyGoal); - const payload = { - title: parsedGoal.title, - ...(parsedGoal.description ? { description: parsedGoal.description } : {}) - }; - // The company may already have a mission this step could not see. // `useCompanyMission` fails open, so a goal lookup that exhausted its // retries sends a company that has one here anyway. Adding a second @@ -966,24 +1030,29 @@ function OnboardingWizardInner({ // customer just answered the question on a step that asked it, so // their answer is the mission. A read that fails still writes: an // unwritten mission is the failure this whole change exists to remove. - let existingGoalId: string | null = null; + let existingGoalId: string | null = createdCompanyGoalId; try { const goals = await queryClient.fetchQuery({ queryKey: queryKeys.goals.list(createdCompanyId), queryFn: () => goalsApi.list(createdCompanyId) }); - existingGoalId = selectDefaultCompanyGoalId(goals); + existingGoalId = existingGoalId ?? selectDefaultCompanyGoalId(goals); } catch { // Still cannot tell. Fall through and write. } - const goal = existingGoalId - ? await goalsApi.update(existingGoalId, payload) - : await goalsApi.create(createdCompanyId, { - ...payload, - level: "company", - status: "active" - }); + const plan = planMissionPersistence({ + goalInput: companyGoal, + existingGoalId, + }); + if (plan.kind === "skip") { + setStep(3); + return; + } + const goal = + plan.kind === "update" + ? await goalsApi.update(plan.goalId, plan.payload) + : await goalsApi.create(createdCompanyId, plan.payload); queryClient.invalidateQueries({ queryKey: queryKeys.goals.list(createdCompanyId) }); @@ -1046,6 +1115,11 @@ function OnboardingWizardInner({ // doesn't hire a second agent. async function handleGiveHeartbeat() { if (!createdCompanyId) return; + // Guarded at the button and the Enter path too; repeated here because this + // seeds the agent's instructions from `companyGoal`, and hiring with an + // unhydrated mission fails silently - the agent exists, and simply never + // learns what the company is for. + if (missionUnresolvedForHire) return; if (createdAgentId) { setStep(5); return; @@ -1214,7 +1288,8 @@ function OnboardingWizardInner({ if (step === 1 && companyName.trim()) setStep(2); else if (step === 2 && companyName.trim() && companyGoal.trim()) handleConfirmMission(); else if (step === 3 && agentName.trim()) setStep(4); - else if (step === 4 && agentName.trim()) handleGiveHeartbeat(); + else if (step === 4 && agentName.trim() && !missionUnresolvedForHire) + handleGiveHeartbeat(); else if (step === 5) handleLaunchToDashboard(); } } @@ -1272,7 +1347,11 @@ function OnboardingWizardInner({
{([1, 2, 3, 4, 5] as const).map((s) => { const filled = step >= s; - const canJump = s < step; + const canJump = canJumpToOnboardingStep({ + targetStep: s, + currentStep: step, + entryStep, + }); return (