diff --git a/ui/src/components/OnboardingWizard.test.tsx b/ui/src/components/OnboardingWizard.test.tsx index 1159bf65f6..67b1a72ffe 100644 --- a/ui/src/components/OnboardingWizard.test.tsx +++ b/ui/src/components/OnboardingWizard.test.tsx @@ -57,6 +57,16 @@ const mockAgentsApi = vi.hoisted(() => ({ instructionsBundle: vi.fn(async () => ({ entryFile: "AGENTS.md" })), saveInstructionsFile: vi.fn(async () => ({})), })); +// The Connect path loads environment settings before probing; without these +// the probe dies on "Could not load environment settings" and the hire never +// runs — which reads as a mysterious 0-call assertion, not an error. +const mockEnvironmentsApi = vi.hoisted(() => ({ + list: vi.fn(async () => []), +})); +const mockInstanceSettingsApi = vi.hoisted(() => ({ + get: vi.fn(async () => ({ defaultEnvironmentId: null })), + getExperimental: vi.fn(async () => ({ enableManagedSandboxOnly: false })), +})); const mockApprovalsApi = vi.hoisted(() => ({ create: vi.fn(), })); @@ -93,6 +103,8 @@ vi.mock("../api/agents", () => ({ agentsApi: mockAgentsApi })); vi.mock("../api/approvals", () => ({ approvalsApi: mockApprovalsApi })); vi.mock("../api/issues", () => ({ issuesApi: mockIssuesApi })); vi.mock("../api/projects", () => ({ projectsApi: mockProjectsApi })); +vi.mock("../api/environments", () => ({ environmentsApi: mockEnvironmentsApi })); +vi.mock("../api/instanceSettings", () => ({ instanceSettingsApi: mockInstanceSettingsApi })); vi.mock("../adapters", () => ({ listUIAdapters: () => mockAdapterRegistry.list, getUIAdapter: () => ({ buildAdapterConfig: () => ({}) }), @@ -257,6 +269,96 @@ describe("OnboardingWizard restore-gate (stale localStorage across accounts)", ( await act(async () => root.unmount()); }); + it("shows no environment-check card on the model step, and no Mission row on review", async () => { + // Round-3 walk feedback: the adapter environment check still runs — + // Connect probes before hiring and blocks on a fail — but its idle card + // (explainer plus "Test now") is gone. And the review checklist lost its + // "Mission" row: onboarding stopped asking, so the row could only ever + // render unchecked. Both asserted against positive anchors so an + // unrendered step cannot pass as an absence. + mockCompaniesApi.create.mockResolvedValue({ id: "company-new", issuePrefix: "INI" }); + const { root } = await openStepOne("create"); + await clickByText((t) => t.startsWith("Next")); + expect(document.body.textContent).toContain("Create your first agent"); + + // Step 3 → 4 needs an agent name; choosing a role fills it. + const roleTrigger = document.body.querySelector("#onboarding-agent-role") as HTMLElement; + await act(async () => { + roleTrigger.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + await flushReact(); + const ceo = [...document.body.querySelectorAll('[role="option"]')].find( + (o) => o.textContent?.trim() === "CEO", + ) as HTMLElement; + await act(async () => { + ceo.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + await flushReact(); + await clickByText((t) => t.startsWith("Next")); + + expect(document.body.textContent).toContain("Connect a model"); + expect(document.body.textContent).not.toContain("Adapter environment check"); + expect(document.body.textContent).not.toContain("Test now"); + + // Through Connect to Review, so the Mission-row assertion runs against + // the checklist that actually renders it — stopping at the model step + // would let a Mission regression pass unseen. + await clickByText((t) => t.startsWith("Connect")); + expect(document.body.textContent).toContain("Review"); + expect(document.body.textContent).toContain("Organization name"); + expect(document.body.textContent).toContain("Agent created"); + expect(document.body.textContent).toContain("Model connected"); + expect(document.body.textContent).not.toContain("Mission"); + + await act(async () => root.unmount()); + }); + + it("hires one agent when Connect fires twice in one breath", async () => { + // The Connect handler re-runs a cached failed probe now that "Test now" + // is gone — so two overlapping submissions could both pass the fresh + // probe and both hire. `loading` cannot stop the second caller: it is + // state, unwritten while the first call is still awaiting. The ref + // guard must make the second submission a no-op. + let resolveHire: (v: { agent: { id: string }; approval: null }) => void = () => {}; + mockAgentsApi.hire.mockReturnValue( + new Promise((resolve) => { + resolveHire = resolve; + }), + ); + mockCompaniesApi.create.mockResolvedValue({ id: "company-new", issuePrefix: "INI" }); + const { root } = await openStepOne("create"); + await clickByText((t) => t.startsWith("Next")); + const roleTrigger = document.body.querySelector("#onboarding-agent-role") as HTMLElement; + await act(async () => { + roleTrigger.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + await flushReact(); + const ceo = [...document.body.querySelectorAll('[role="option"]')].find( + (o) => o.textContent?.trim() === "CEO", + ) as HTMLElement; + await act(async () => { + ceo.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + await flushReact(); + await clickByText((t) => t.startsWith("Next")); + expect(document.body.textContent).toContain("Connect a model"); + + const connect = [...document.body.querySelectorAll("button")].find((b) => + b.textContent?.trim().startsWith("Connect"), + )!; + await act(async () => { + connect.dispatchEvent(new MouseEvent("click", { bubbles: true })); + connect.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + await flushReact(); + await act(async () => resolveHire({ agent: { id: "agent-1" }, approval: null })); + await flushReact(); + + expect(mockAgentsApi.hire).toHaveBeenCalledTimes(1); + + await act(async () => root.unmount()); + }); + it("creates one company for one keystroke, modifier or not", async () => { // The name field handles Enter itself and does not check for a modifier, // so Cmd+Enter in that field reaches the field's handler *and* the diff --git a/ui/src/components/OnboardingWizard.tsx b/ui/src/components/OnboardingWizard.tsx index 146fcf5cb7..1f06aec913 100644 --- a/ui/src/components/OnboardingWizard.tsx +++ b/ui/src/components/OnboardingWizard.tsx @@ -461,6 +461,11 @@ function OnboardingWizardInner({ // either, because it is not set until the request it guards has resolved. A // ref is written before the request goes out, so the second caller sees it. const creatingCompanyRef = useRef(false); + // Same shape for the hire. Greptile (round-3 PR): with "Test now" gone the + // Connect handler re-runs a cached failed probe — and two overlapping + // submissions could then both pass the fresh probe and both hire. `loading` + // cannot stop the second caller for the same reason as above. + const hiringAgentRef = useRef(false); createdCompanyIdRef.current = createdCompanyId; // The mission of the company actually in hand, which is not always the one @@ -1256,6 +1261,8 @@ function OnboardingWizardInner({ setStep(5); return; } + if (hiringAgentRef.current) return; + hiringAgentRef.current = true; setLoading(true); setError(null); try { @@ -1293,7 +1300,13 @@ function OnboardingWizardInner({ } if (isLocalAdapter) { - const result = adapterEnvResult ?? (await runAdapterEnvironmentTest()); + // A cached pass or warn is still good; a cached fail is retried. With + // the "Test now" card gone, this button is the only way to re-probe, + // and reusing a stale fail would lock a customer out of a machine + // they have since fixed. + const cachedUsable = + adapterEnvResult && adapterEnvResult.status !== "fail" ? adapterEnvResult : null; + const result = cachedUsable ?? (await runAdapterEnvironmentTest()); if (!result) return; // Block the hire on a failed environment test. A pass or a warn may // proceed; a fail means the agent cannot run as configured. @@ -1366,6 +1379,7 @@ function OnboardingWizardInner({ } catch (err) { setError(err instanceof Error ? err.message : "Failed to create agent"); } finally { + hiringAgentRef.current = false; setLoading(false); } } @@ -2232,29 +2246,15 @@ function OnboardingWizardInner({ )} - {isLocalAdapter && ( + {/* The environment check runs without being shown: Connect + probes the adapter before hiring (see handleGiveHeartbeat) + and blocks the hire on a fail. The idle card — probe + explainer plus a "Test now" button — is gone from this + step, so this block renders only when a probe has actually + found something: the checks the blocking error tells the + customer to fix have to be visible somewhere. */} + {isLocalAdapter && (adapterEnvError || (adapterEnvResult && adapterEnvResult.status !== "pass")) && (
-
-
-

- Adapter environment check -

-

- Runs a live probe that asks the adapter CLI to - respond with hello. -

-
- -
- {adapterEnvError && (
{adapterEnvError} @@ -2397,9 +2397,11 @@ function OnboardingWizardInner({
{/* Review checklist — everything that's now set up */}
+ {/* No "Mission" row: onboarding stopped asking for one, so a + checklist item for it could only ever render unchecked — + a permanent red mark for a question nobody was asked. */} {[ { label: "Organization name", done: Boolean(companyName.trim()) }, - { label: "Mission", done: Boolean(companyGoal.trim()) }, { label: "Agent created", done: Boolean(createdAgentId) }, { label: "Model connected", done: Boolean(createdAgentId) }, ].map(({ label, done }) => ( diff --git a/ui/src/components/PaperclipLockup.tsx b/ui/src/components/PaperclipLockup.tsx new file mode 100644 index 0000000000..80b5d0689a --- /dev/null +++ b/ui/src/components/PaperclipLockup.tsx @@ -0,0 +1,47 @@ +import type { SVGProps } from "react"; + +interface PaperclipLockupProps extends Omit, "children"> { + decorative?: boolean; + title?: string; +} + +/** + * The full Paperclip lockup — mark plus wordmark — from the brand asset + * (`paperclip_lockup_dark_mode alt.svg`). The export is white-filled for dark + * chrome; here every path fills `currentColor`, so the one geometry follows + * the theme the way the brand system expects ("one vocabulary, two surfaces"). + * The viewBox is cropped to the artwork's content bounds — the export carries + * stage padding that would render the lockup at a third of its container. + * + * Size it with a height class (`h-5 w-auto`); width follows the aspect. + */ +export function PaperclipLockup({ + decorative = false, + title = "Paperclip", + className, + ...rest +}: PaperclipLockupProps) { + return ( + + + + + + + + + + + + + ); +} diff --git a/ui/src/pages/Auth.tsx b/ui/src/pages/Auth.tsx index bbd4707a4a..d16592949d 100644 --- a/ui/src/pages/Auth.tsx +++ b/ui/src/pages/Auth.tsx @@ -8,7 +8,7 @@ import { Button } from "@/components/ui/button"; import { AsciiArtAnimation } from "@/components/AsciiArtAnimation"; import { PaperclipLoading } from "@/components/AnimatedPaperclipIcon"; import { ThemeToggle } from "@/components/ThemeToggle"; -import { Sparkles } from "lucide-react"; +import { PaperclipLockup } from "../components/PaperclipLockup"; type AuthMode = "sign_in" | "sign_up"; @@ -88,9 +88,8 @@ export function AuthPage() { {/* Left half — form */}
-
- - Paperclip +
+