From 6542ad1f4d90cf420f5121d938b7094cb6f89c6d Mon Sep 17 00:00:00 2001 From: Tonio Date: Sat, 15 Aug 2026 01:42:03 -0700 Subject: [PATCH] fix(onboarding): carry an existing company's mission into the wizard (#11416) Ports the substance of #11259, which predated the recent onboarding work and was still open. One of the parts it solves is a regression #11352 introduced. A company that already has its mission opens on the agent step, which is the point of #11352. What that missed is that the mission field is filled only by the step being skipped, and that the field is not decoration: `composeCeoInstructions` seeds the lead agent's instructions from it, and the Review checklist reads it. So every Cloud-seeded company hired its lead agent with no Mission line at all, having been routed there precisely *because* it had a mission. Before #11352 those companies dead-ended on the mission step; a dead end became a quiet data loss, which is worse, because it completes. `selectExistingCompanyMission` reads the company's own goal back into the shape the mission field holds, and the wizard hydrates from it - only when the field is empty, so a customer editing their mission is never overwritten by the stored copy. The marker recording that hydration travels with the field it describes, cleared wherever `companyGoal` is. `isExistingCompanyMissionUnresolved` holds the hire while that read is outstanding, counting an in-flight refetch over cached goals as unresolved. That is the rule #11382 settled on a day earlier for a different consumer - `isFetching`, not `isLoading`, because retained data is not an answer to the question being asked now. #11259 had it first, on 11 August. `canGoBackFromOnboardingStep` and `canJumpToOnboardingStep` bound how far back a run can walk by the step it entered on. The Back button already applied that rule inline; the progress bar applied only the "already completed" half, so a run holding a company could still jump to step 1 - the step whose job is to create one. The entry step is captured once, when the wizard opens, for the same reason the step itself is. `planMissionPersistence` came with them and turned out to be required rather than tidying. Hydration sets `createdCompanyGoalId` from the company's existing goal, and confirming the mission read that id as "already written" and skipped the write, discarding the customer's edit. That skip was safe only while the id could arrive one way - by writing. A goal in hand now means update it. Each piece was checked by removing it and confirming a specific case fails. The hydration case asserts on `saveInstructionsFile`'s content, the actual consumer, rather than on the mission textarea, because the entry path never renders that field and the navigation bound now prevents reaching it. One caveat recorded rather than smoothed over: the reopen case fails only when both marker-clears are removed, since `reset()` also clears the company id and the next introduction routes through `clearCompanyScopedState`. They are kept as one invariant rather than one guard plus a coincidence. ui typecheck clean; full ui suite 4004 pass. Two failures remain, in IssueProperties and StatusCards/format; both are date-dependent, both reproduce on master with these changes stashed, and neither file is touched here. Co-Authored-By: Jannes Stubbemann Co-Authored-By: Claude Opus 5 --- .../components/OnboardingWizard.step.test.tsx | 172 +++++++++++++++++- ui/src/components/OnboardingWizard.tsx | 134 +++++++++++--- ui/src/hooks/useCompanyMission.test.tsx | 37 +++- ui/src/hooks/useCompanyMission.ts | 24 ++- ui/src/lib/onboarding-goal.test.ts | 33 +++- ui/src/lib/onboarding-goal.ts | 20 ++ ui/src/lib/onboarding-mission.test.ts | 132 ++++++++++++++ ui/src/lib/onboarding-mission.ts | 106 +++++++++++ ui/src/lib/onboarding-route.ts | 34 ++++ 9 files changed, 657 insertions(+), 35 deletions(-) create mode 100644 ui/src/lib/onboarding-mission.test.ts create mode 100644 ui/src/lib/onboarding-mission.ts 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 (