diff --git a/frontend/editor/src/portal/PortalProviders.tsx b/frontend/editor/src/portal/PortalProviders.tsx index 08ae298d88..0f7e3a6c94 100644 --- a/frontend/editor/src/portal/PortalProviders.tsx +++ b/frontend/editor/src/portal/PortalProviders.tsx @@ -54,7 +54,7 @@ function LinkModalHost() { */ export function PortalProviders() { return ( - + diff --git a/frontend/editor/src/portal/components/LinkAccountFooterItem.tsx b/frontend/editor/src/portal/components/LinkAccountFooterItem.tsx index fd3e9d96a0..20ba0125ef 100644 --- a/frontend/editor/src/portal/components/LinkAccountFooterItem.tsx +++ b/frontend/editor/src/portal/components/LinkAccountFooterItem.tsx @@ -14,8 +14,9 @@ import { LinkIcon } from "@portal/components/icons"; export function LinkAccountFooterItem() { const { t } = useTranslation(); const { openLinkModal } = useUI(); - const { linkState } = useLink(); - if (linkState !== "unlinked") return null; + const { linkState, statusKnown } = useLink(); + // Hidden until the status is actually known, or a linked instance flashes a "link" button. + if (!statusKnown || linkState !== "unlinked") return null; return ( void; + /** + * Whether the instance's link status has actually been read back yet. + * + * `linkState` starts at "unlinked" because the type has no third value, so without this a + * linked instance is indistinguishable from one we have not asked about. Anything that + * BLOCKS on being unlinked has to wait for this; anything that merely reports state need not. + */ + statusKnown: boolean; + /** Called once the status endpoint has actually answered. */ + markStatusKnown: () => void; } const LinkContext = createContext(null); @@ -73,11 +83,19 @@ const LinkContext = createContext(null); export function LinkProvider({ children, initialState = "unlinked", + statusKnown: initialStatusKnown = true, }: { children: ReactNode; initialState?: LinkState; + /** + * Whether `initialState` is authoritative. Defaults to true so a pinned state (stories, tests) + * is believed as given; the app passes false because it has to go and ask first. + */ + statusKnown?: boolean; }) { const [linkState, setLinkState] = useState(initialState); + const [statusKnown, setStatusKnown] = useState(initialStatusKnown); + const markStatusKnown = useCallback(() => setStatusKnown(true), []); const [saasSessionNonce, setSaasSessionNonce] = useState(0); const markSaasSessionChanged = useCallback( () => setSaasSessionNonce((n) => n + 1), @@ -92,8 +110,16 @@ export function LinkProvider({ featuresUnlocked: unlocked, saasSessionNonce, markSaasSessionChanged, + statusKnown, + markStatusKnown, }; - }, [linkState, saasSessionNonce, markSaasSessionChanged]); + }, [ + linkState, + saasSessionNonce, + markSaasSessionChanged, + statusKnown, + markStatusKnown, + ]); return {children}; } diff --git a/frontend/editor/src/portal/hooks/connectGateStatus.test.tsx b/frontend/editor/src/portal/hooks/connectGateStatus.test.tsx new file mode 100644 index 0000000000..736faf4f59 --- /dev/null +++ b/frontend/editor/src/portal/hooks/connectGateStatus.test.tsx @@ -0,0 +1,101 @@ +import { describe, expect, it, vi } from "vitest"; +import { render, screen, waitFor } from "@testing-library/react"; +import { PortalTestProviders } from "@portal/test/TestQueryProvider"; +import { LinkProvider } from "@portal/contexts/LinkContext"; +import { UIProvider } from "@portal/contexts/UIContext"; + +/** + * The gate over the real provider stack, which is where it went wrong in the field: a linked + * instance was still shown the connect prompt and the sidebar button. + * + * `linkState` starts at "unlinked" because the type has no third value, and the status that + * corrects it arrives separately from the app config. Gating on `!isLinked` alone therefore reads + * "we have not asked yet" as "not linked". These pin the three answers apart: not yet known, + * never knowable, and actually not linked. + */ +const { json, fetchStatus } = vi.hoisted(() => ({ + json: vi.fn(), + fetchStatus: vi.fn(), +})); +vi.mock("@portal/api/http", () => ({ + apiClient: { local: { json } }, + errorMessage: String, +})); +vi.mock("@portal/api/link", () => ({ + fetchStatus, + linkInstance: vi.fn(), + unlinkInstance: vi.fn(), +})); +vi.mock("@portal/auth/saasSupabase", () => ({ + PENDING_LINK_KEY: "k", + isSaasSupabaseConfigured: false, + SAAS_OAUTH_PROVIDERS: [], + ensureSaasSupabase: () => null, +})); + +import { AccountLinkProvider } from "@portal/contexts/AccountLinkContext"; +import { useConnectGate } from "@portal/hooks/useConnectGate"; + +function Probe() { + const { gated, available } = useConnectGate(); + return ( + + {`${available ? "avail" : "unavail"}:${gated ? "gated" : "open"}`} + + ); +} + +const renderStack = () => + render( + + + + + + + + + , + ); + +const state = () => screen.getByTestId("g").textContent; +const availableConfig = () => + json.mockResolvedValue({ accountLinkAvailable: true }); + +describe("connect gate and the link status", () => { + it("stays open while the status is still in flight", async () => { + availableConfig(); + let resolve!: (v: unknown) => void; + fetchStatus.mockReturnValue(new Promise((r) => (resolve = r))); + renderStack(); + await waitFor(() => expect(state()).toContain("avail")); + expect(state()).toContain("open"); + resolve({ linked: true, name: "acme" }); + }); + + it("stays open once a linked status arrives", async () => { + availableConfig(); + fetchStatus.mockResolvedValue({ linked: true, name: "acme" }); + renderStack(); + await waitFor(() => expect(state()).toContain("avail")); + expect(state()).toContain("open"); + }); + + it("stays open when the status call fails, rather than assuming unlinked", async () => { + availableConfig(); + fetchStatus.mockRejectedValue( + new Error("401 after the admin session lapsed"), + ); + renderStack(); + await waitFor(() => expect(state()).toContain("avail")); + await new Promise((r) => setTimeout(r, 10)); + expect(state()).toContain("open"); + }); + + it("still gates once the status says the instance really is unlinked", async () => { + availableConfig(); + fetchStatus.mockResolvedValue({ linked: false, name: null }); + renderStack(); + await waitFor(() => expect(state()).toBe("avail:gated")); + }); +}); diff --git a/frontend/editor/src/portal/hooks/useAccountLink.ts b/frontend/editor/src/portal/hooks/useAccountLink.ts index dd4894f6d6..f955c7fcb1 100644 --- a/frontend/editor/src/portal/hooks/useAccountLink.ts +++ b/frontend/editor/src/portal/hooks/useAccountLink.ts @@ -46,7 +46,7 @@ export interface UseAccountLink { export function useAccountLink(): UseAccountLink { const applyLinkFacts = useApplyLinkFacts(); - const { markSaasSessionChanged } = useLink(); + const { markSaasSessionChanged, markStatusKnown } = useLink(); const [status, setStatus] = useState(null); const [phase, setPhase] = useState("idle"); const [error, setError] = useState(null); @@ -80,6 +80,9 @@ export function useAccountLink(): UseAccountLink { setStatus(s); // A linked instance is at least linked-free; subscription comes from the wallet. if (s.linked) applyLinkFacts(true, false); + // Only a real answer counts. Marking this on failure too would put us back to + // treating "could not ask" as "not linked", which is what gated linked instances. + markStatusKnown(); } }) .catch(() => { @@ -91,7 +94,7 @@ export function useAccountLink(): UseAccountLink { return () => { cancelled = true; }; - }, [applyLinkFacts]); + }, [applyLinkFacts, markStatusKnown]); // SSO return: an SSO sign-in we kicked off has redirected back and the SaaS // session is now in the shared Supabase client. The pending marker carries the diff --git a/frontend/editor/src/portal/hooks/useConnectGate.ts b/frontend/editor/src/portal/hooks/useConnectGate.ts index 824fc96ade..21325eee9f 100644 --- a/frontend/editor/src/portal/hooks/useConnectGate.ts +++ b/frontend/editor/src/portal/hooks/useConnectGate.ts @@ -40,7 +40,7 @@ export interface ConnectGate { * extra request. */ export function useConnectGate(): ConnectGate { - const { isLinked } = useLink(); + const { isLinked, statusKnown } = useLink(); const { openLinkModal } = useUI(); const devBypass = useDevConnectBypass(); @@ -51,10 +51,15 @@ export function useConnectGate(): ConnectGate { }); const available = Boolean(query.data?.accountLinkAvailable); - const loading = query.isPending; + // Both answers have to be in. The link status arrives separately from the app config and + // starts out as "unlinked" simply because the type has no third value, so gating before it + // lands blocks and nags an instance that is in fact linked. If the status call fails we never + // learn the answer and stay open: a gate that cannot read its own precondition should not be + // the thing standing in the way. + const loading = query.isPending || !statusKnown; // Dev-only, and absent from every build. See useDevConnectBypass for why this cannot be a // setting: the gate is currently the only thing enforcing that these features need a link. - const gated = available && !isLinked && !devBypass; + const gated = available && statusKnown && !isLinked && !devBypass; const connect = useCallback(() => openLinkModal(), [openLinkModal]);