Skip to content

Commit 61daef9

Browse files
authored
[OPIK-5898] [FE] fix: pairing page V2 routing and workspace-version gate (#6269)
* [OPIK-5898] [FE] fix: route /pair/v1 to V2 app and gate pairing on workspace version * [OPIK-5898] [FE] fix: make V2-only path detection basepath-independent * [OPIK-5898] [FE] fix: make pairing route basepath-relative TanStack strips the router basepath before matching, so the route path must be basepath-relative like every other route in the tree. With the hardcoded /opik/ prefix, the route only matched on OSS (basepath "/") and missed on cloud (basepath "/opik"), where the internal path is /pair/v1 after stripping — causing the pair link to fall through to the home redirect. * [OPIK-5898] [FE] fix: pairing UX polish and OSS fallback route - Add /opik/pair/v1 alias for OSS: the Python SDK hardcodes the /opik/ prefix, which only strips on cloud where basepath is /opik. Marked with a TODO to remove once the SDK builds basepath-aware URLs. - Resolve workspace version synchronously at module load when possible (override, V2-only path, or no workspace in URL) so the first paint renders the correct App — no Loader flash on /pair/*. - PairingPage: use project icons (CheckCircle2, CircleAlert, Spinner), redirect to agent-configuration on success instead of window.close, and treat 409 (runner already paired) as success so the user still gets redirected. * [OPIK-5898] [FE] fix: surface parse errors before workspace-version check - Check parseError before the workspacePhase === "checking" branch so a malformed pairing fragment doesn't sit on "Connecting…" while the version query is in flight. - Drop the useMemo around workspaceName so any re-render picks up the current query string instead of the value frozen at mount.
1 parent 67afb60 commit 61daef9

4 files changed

Lines changed: 200 additions & 77 deletions

File tree

apps/opik-frontend/src/WorkspaceVersionGate.tsx

Lines changed: 25 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,14 @@
99
*
1010
* Level 1 — WorkspaceVersionGate (this component, BEFORE the router):
1111
* 1. Check localStorage override ("opik-version-override") → use immediately
12-
* 2. Try to parse workspace name from window.location.pathname
13-
* 3. If found → fetch version from API with per-request Comet-Workspace header
14-
* 4. If not found (e.g. root "/") → default to v1 optimistically
15-
* 5. Render the correct App (V1App or V2App) based on resolved version
12+
* 2. Check if the path forces a version (e.g. /pair/* is V2-only)
13+
* 3. Else parse the workspace name from window.location.pathname
14+
* 4. If found → fetch version from API with per-request Comet-Workspace header
15+
* 5. If not found (e.g. root "/") → default to v1 optimistically
16+
*
17+
* Steps 1, 2, 5 are synchronous and run at module load, so the first paint
18+
* already knows which App to render — no Loader flash in the common cases.
19+
* Only step 4 (workspace in URL, version unknown) falls back to a Loader.
1620
*
1721
* Level 2 — WorkspaceVersionResolver (INSIDE the router, after WorkspacePreloader):
1822
* 1. Workspace is now fully resolved (auth, access, header set)
@@ -27,40 +31,36 @@ import React, { Suspense, useEffect } from "react";
2731
import useAppStore, { useWorkspaceVersion } from "@/store/AppStore";
2832
import { fetchWorkspaceVersion } from "@/api/workspaces/useWorkspaceVersion";
2933
import {
30-
DEFAULT_WORKSPACE_VERSION,
31-
getVersionOverride,
3234
getWorkspaceNameFromPath,
35+
resolveSyncWorkspaceVersion,
3336
} from "@/lib/workspaceVersion";
3437
import Loader from "@/shared/Loader/Loader";
3538

3639
const V1App = React.lazy(() => import("@/v1/App"));
3740
const V2App = React.lazy(() => import("@/v2/App"));
3841

42+
// Populate the store synchronously at module load so the first render
43+
// already has a version — avoids a Loader flash on /pair/*, root "/", and
44+
// any path with a localStorage version override.
45+
const initialVersion = resolveSyncWorkspaceVersion();
46+
if (initialVersion) {
47+
useAppStore.getState().setWorkspaceVersion(initialVersion);
48+
}
49+
3950
const WorkspaceVersionGate = () => {
4051
const version = useWorkspaceVersion();
4152

4253
useEffect(() => {
43-
let cancelled = false;
54+
if (useAppStore.getState().workspaceVersion) return;
55+
const workspaceName = getWorkspaceNameFromPath();
56+
if (!workspaceName) return;
4457

45-
async function resolve() {
46-
const override = getVersionOverride();
47-
if (override) {
48-
useAppStore.getState().setWorkspaceVersion(override);
49-
return;
50-
}
51-
52-
const workspaceName = getWorkspaceNameFromPath();
53-
if (workspaceName) {
54-
const resolved = await fetchWorkspaceVersion({ workspaceName });
55-
if (!cancelled) {
56-
useAppStore.getState().setWorkspaceVersion(resolved);
57-
}
58-
} else {
59-
useAppStore.getState().setWorkspaceVersion(DEFAULT_WORKSPACE_VERSION);
58+
let cancelled = false;
59+
fetchWorkspaceVersion({ workspaceName }).then((resolved) => {
60+
if (!cancelled) {
61+
useAppStore.getState().setWorkspaceVersion(resolved);
6062
}
61-
}
62-
resolve();
63-
63+
});
6464
return () => {
6565
cancelled = true;
6666
};

apps/opik-frontend/src/lib/workspaceVersion.ts

Lines changed: 37 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,17 +4,51 @@ export const DEFAULT_WORKSPACE_VERSION: WorkspaceVersion = "v1";
44

55
const OPIK_VERSION_OVERRIDE_KEY = "opik-version-override";
66

7+
// Path segments (directly under basepath) that are only valid in V2.
8+
// Add new V2-only top-level routes here — nothing else needs to change.
9+
const V2_ONLY_SEGMENTS: ReadonlySet<string> = new Set(["pair"]);
10+
711
export function getVersionOverride(): WorkspaceVersion | null {
812
const override = localStorage.getItem(OPIK_VERSION_OVERRIDE_KEY);
913
return override === "v1" || override === "v2" ? override : null;
1014
}
1115

12-
export function getWorkspaceNameFromPath(): string | null {
16+
function getRelativePathSegments(): string[] {
1317
const basePath = (import.meta.env.VITE_BASE_URL || "/").replace(/\/$/, "");
1418
const pathname = window.location.pathname;
1519
const relative = pathname.startsWith(basePath)
1620
? pathname.slice(basePath.length)
1721
: pathname;
18-
const segments = relative.split("/").filter(Boolean);
19-
return segments[0] || null;
22+
return relative.split("/").filter(Boolean);
23+
}
24+
25+
export function getWorkspaceNameFromPath(): string | null {
26+
return getRelativePathSegments()[0] || null;
27+
}
28+
29+
// Returns a version that the current path forces regardless of workspace.
30+
// Used by WorkspaceVersionGate to short-circuit V2-only routes (e.g. /pair/*)
31+
// without an API call and without touching localStorage.
32+
//
33+
// The pairing URL from the SDK is always of the form `.../opik/pair/v1`.
34+
// On cloud (VITE_BASE_URL=/opik), the `/opik` prefix is stripped by
35+
// getRelativePathSegments so the first segment is "pair". On OSS
36+
// (VITE_BASE_URL=/), nothing is stripped and "opik" remains as the first
37+
// segment — skip it so detection works in both deployments.
38+
export function getForcedVersionFromPath(): WorkspaceVersion | null {
39+
const segments = getRelativePathSegments();
40+
const head = segments[0] === "opik" ? segments[1] : segments[0];
41+
return head && V2_ONLY_SEGMENTS.has(head) ? "v2" : null;
42+
}
43+
44+
// Resolves the workspace version synchronously when possible. Returns null
45+
// only when the workspace name is in the URL but its version requires an
46+
// API fetch — the one case that needs to fall back to a Loader.
47+
export function resolveSyncWorkspaceVersion(): WorkspaceVersion | null {
48+
const override = getVersionOverride();
49+
if (override) return override;
50+
const forced = getForcedVersionFromPath();
51+
if (forced) return forced;
52+
if (!getWorkspaceNameFromPath()) return DEFAULT_WORKSPACE_VERSION;
53+
return null;
2054
}

apps/opik-frontend/src/v2/pages/PairingPage/PairingPage.tsx

Lines changed: 122 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
1-
import React, { useEffect, useMemo, useState } from "react";
1+
import React, { useEffect, useMemo } from "react";
2+
import { useMutation, useQuery } from "@tanstack/react-query";
3+
import { useNavigate } from "@tanstack/react-router";
4+
import { CheckCircle2, CircleAlert } from "lucide-react";
25
import api from "@/api/api";
6+
import { fetchWorkspaceVersion } from "@/api/workspaces/useWorkspaceVersion";
7+
import { Spinner } from "@/ui/spinner";
38

49
// ---------------------------------------------------------------------------
510
// Binary helpers
@@ -123,23 +128,20 @@ async function deriveBridgeKey(
123128
// Activate + derive + store — runs once on mount
124129
// ---------------------------------------------------------------------------
125130

126-
async function activate(payload: PairingPayload): Promise<void> {
127-
const workspace = new URLSearchParams(window.location.search).get(
128-
"workspace",
129-
);
130-
if (workspace) {
131-
api.defaults.headers.common["Comet-Workspace"] = workspace;
132-
}
133-
131+
async function activate(
132+
payload: PairingPayload,
133+
workspace: string | null,
134+
): Promise<void> {
134135
const hmac = await computeActivationHmac(
135136
payload.activationKey,
136137
payload.sessionId,
137138
payload.runnerName,
138139
);
139-
await api.post(`/v1/private/pairing/sessions/${payload.sessionId}/activate`, {
140-
runner_name: payload.runnerName,
141-
hmac,
142-
});
140+
await api.post(
141+
`/v1/private/pairing/sessions/${payload.sessionId}/activate`,
142+
{ runner_name: payload.runnerName, hmac },
143+
workspace ? { headers: { "Comet-Workspace": workspace } } : undefined,
144+
);
143145

144146
// Only CONNECT runners use bridge keys for HMAC-signed file commands.
145147
// ENDPOINT runners don't need one — storing it would overwrite the
@@ -160,7 +162,11 @@ async function activate(payload: PairingPayload): Promise<void> {
160162
// Page component
161163
// ---------------------------------------------------------------------------
162164

165+
type WorkspacePhase = "missing" | "checking" | "ok" | "v1";
166+
type Status = "loading" | "success" | "error";
167+
163168
const PairingPage: React.FC = () => {
169+
const navigate = useNavigate();
164170
const fragment = window.location.hash.slice(1);
165171

166172
const [payload, parseError] = useMemo<
@@ -174,52 +180,119 @@ const PairingPage: React.FC = () => {
174180
}
175181
}, [fragment]);
176182

177-
const [status, setStatus] = useState<"busy" | "done" | "error">(
178-
parseError ? "error" : "busy",
183+
const workspaceName = new URLSearchParams(window.location.search).get(
184+
"workspace",
179185
);
180-
const [error, setError] = useState(parseError ?? "");
181186

182-
// Auto-activate on mount
183-
useEffect(() => {
184-
if (!payload) return;
185-
if (!crypto?.subtle) {
186-
setStatus("error");
187-
setError("Pairing requires a secure connection (HTTPS).");
188-
return;
189-
}
190-
activate(payload)
191-
.then(() => setStatus("done"))
192-
.catch((err: unknown) => {
193-
setStatus("error");
194-
const s =
187+
const versionQuery = useQuery({
188+
queryKey: ["pairing-workspace-version", workspaceName],
189+
queryFn: ({ signal }) =>
190+
fetchWorkspaceVersion({ workspaceName: workspaceName!, signal }),
191+
enabled: !!workspaceName,
192+
staleTime: 5 * 60 * 1000,
193+
});
194+
195+
const workspacePhase: WorkspacePhase = useMemo(() => {
196+
if (!workspaceName) return "missing";
197+
if (versionQuery.isPending) return "checking";
198+
if (versionQuery.data === "v2") return "ok";
199+
return "v1";
200+
}, [workspaceName, versionQuery.isPending, versionQuery.data]);
201+
202+
const {
203+
mutate: runActivation,
204+
isIdle: activationIdle,
205+
isError: activationIsError,
206+
isSuccess: activationIsSuccess,
207+
error: activationError,
208+
} = useMutation({
209+
mutationFn: async (p: PairingPayload) => {
210+
if (!crypto?.subtle) throw new Error("SECURE_CONTEXT_REQUIRED");
211+
try {
212+
await activate(p, workspaceName);
213+
} catch (err) {
214+
// 409 = runner already paired; treat as success so the user still
215+
// gets redirected to their agent instead of seeing an error screen.
216+
const status =
195217
err && typeof err === "object" && "response" in err
196218
? (err as { response?: { status?: number } }).response?.status
197219
: undefined;
198-
if (s === 403)
199-
setError("This pairing link is invalid or has been tampered with.");
200-
else if (s === 404)
201-
setError("This pairing link has expired. Run the CLI command again.");
202-
else if (s === 409)
203-
setError("This runner is already connected. You can close this tab.");
204-
else setError("Could not reach Opik. Check your connection.");
220+
if (status !== 409) throw err;
221+
}
222+
},
223+
onSuccess: (_data, variables) => {
224+
if (!workspaceName) return;
225+
navigate({
226+
to: "/$workspaceName/projects/$projectId/agent-configuration",
227+
params: { workspaceName, projectId: variables.projectId },
205228
});
206-
}, [payload]);
229+
},
230+
});
207231

208-
// Auto-close on success
209232
useEffect(() => {
210-
if (status !== "done") return;
211-
const t = setTimeout(() => window.close(), 1500);
212-
return () => clearTimeout(t);
213-
}, [status]);
233+
if (workspacePhase !== "ok" || !payload || !activationIdle) return;
234+
runActivation(payload);
235+
}, [workspacePhase, payload, activationIdle, runActivation]);
236+
237+
function getActivationErrorMessage(err: unknown): string {
238+
if (err instanceof Error && err.message === "SECURE_CONTEXT_REQUIRED") {
239+
return "Pairing requires a secure connection (HTTPS).";
240+
}
241+
const s =
242+
err && typeof err === "object" && "response" in err
243+
? (err as { response?: { status?: number } }).response?.status
244+
: undefined;
245+
if (s === 403)
246+
return "This pairing link is invalid or has been tampered with.";
247+
if (s === 404)
248+
return "This pairing link has expired. Run the CLI command again.";
249+
return "Could not reach Opik. Check your connection.";
250+
}
251+
252+
function getDisplay(): { status: Status; message: string } {
253+
// Fragment-level errors surface first: a bad link is invalid regardless
254+
// of workspace state, and we shouldn't block on the version query to
255+
// tell the user.
256+
if (parseError) return { status: "error", message: parseError };
257+
if (workspacePhase === "missing") {
258+
return { status: "error", message: "This pairing link is invalid." };
259+
}
260+
if (workspacePhase === "v1") {
261+
return {
262+
status: "error",
263+
message:
264+
"Opik Connect requires Opik 2.0. Please upgrade your workspace to continue.",
265+
};
266+
}
267+
if (workspacePhase === "checking") {
268+
return { status: "loading", message: "Connecting…" };
269+
}
270+
if (activationIsError) {
271+
return {
272+
status: "error",
273+
message: getActivationErrorMessage(activationError),
274+
};
275+
}
276+
if (activationIsSuccess) return { status: "success", message: "Connected" };
277+
return { status: "loading", message: "Connecting…" };
278+
}
279+
280+
const { status, message } = getDisplay();
281+
282+
const icon =
283+
status === "loading" ? (
284+
<Spinner size="medium" />
285+
) : status === "success" ? (
286+
<CheckCircle2 className="size-8 shrink-0 text-green-600" />
287+
) : (
288+
<CircleAlert className="size-8 shrink-0 text-destructive" />
289+
);
214290

215291
return (
216-
<p>
217-
{status === "done"
218-
? "Connected ✔"
219-
: status === "error"
220-
? error
221-
: "Connecting…"}
222-
</p>
292+
<div className="flex min-h-screen flex-col items-center justify-center gap-3 p-6">
293+
{icon}
294+
<p className="comet-body text-center text-muted-slate">{message}</p>
295+
</div>
223296
);
224297
};
225298

apps/opik-frontend/src/v2/router.tsx

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,22 @@ const workspaceGuardEmptyLayoutRoute = createRoute({
9999
});
100100

101101
// ----------- pairing (root-level, no layout)
102+
// TanStack strips the router basepath before matching, so the canonical
103+
// route is basepath-relative: "/pair/v1" covers cloud (basepath "/opik"
104+
// → URL "/opik/pair/v1" strips to "/pair/v1").
105+
//
106+
// The legacy "/opik/pair/v1" alias below is an OSS-only fallback: the
107+
// Python SDK hardcodes "{origin}/opik/pair/v1" regardless of deployment
108+
// (see sdks/python/src/opik/cli/pairing.py), so on OSS (basepath "/") the
109+
// "/opik/" prefix isn't stripped and the router sees "/opik/pair/v1".
110+
// TODO: make the Python SDK build basepath-aware pairing URLs and drop
111+
// this alias once shipped CLI versions roll over.
102112
const pairingRoute = createRoute({
113+
getParentRoute: () => rootRoute,
114+
path: "/pair/v1",
115+
component: PairingPage,
116+
});
117+
const pairingRouteOssAlias = createRoute({
103118
getParentRoute: () => rootRoute,
104119
path: "/opik/pair/v1",
105120
component: PairingPage,
@@ -540,6 +555,7 @@ const v1RedirectRoutes = createV1RedirectRoutes(workspaceRoute);
540555

541556
const routeTree = rootRoute.addChildren([
542557
pairingRoute,
558+
pairingRouteOssAlias,
543559
workspaceGuardEmptyLayoutRoute.addChildren([automationLogsRoute]),
544560
workspaceGuardPartialLayoutRoute.addChildren([
545561
quickstartRoute,

0 commit comments

Comments
 (0)