From f4a13fea2e245fea4ddfc8ffa2fe1a1e7c523e46 Mon Sep 17 00:00:00 2001 From: badcuban <108198679+badcuban@users.noreply.github.com> Date: Fri, 7 Aug 2026 03:27:15 -0400 Subject: [PATCH 1/3] Unify provider sign-in on the hidden flow and match actions to errors Every Sign in button (setup card row, composer notices) now drives the same server-side login flow the settings panel uses, through a shared useProviderConnectFlow hook: inline "Signing in" status with the live output line where the user clicked, no thread terminal takeover, and a hand-off to the settings panel (deep-linked to the instance) when a run stalls past the auto-expand threshold or fails while interactive. On the held-send notice, a successful sign-in triggers the existing recheck, so the held message releases itself through the normal preflight gate with no second click. A replay guard keeps an attached surface from treating a finished session from minutes ago as the run the user just started. The provider status notice now earns its actions from the structured snapshot instead of always offering Refresh and Diagnostics: signed out gets Sign in alone, a missing CLI gets Open Settings plus Refresh, disabled gets Open Settings, and probe trouble keeps Refresh plus Diagnostics. URLs in provider status details render as real links via a shared linkifier, so the Claude install address stops being dead text. --- apps/web/src/components/ChatView.browser.tsx | 123 ++++++++- apps/web/src/components/ChatView.tsx | 53 ++-- .../chat/FirstRunSetupCard.browser.tsx | 171 +++++++++++- .../src/components/chat/FirstRunSetupCard.tsx | 53 ++-- apps/web/src/components/chat/firstRunSetup.ts | 7 +- .../chat/providerReadinessNotice.tsx | 75 +++--- .../src/components/chat/providerSignIn.tsx | 184 +++++++++++++ .../chat/providerStatusNotice.browser.tsx | 108 +++++++- .../chat/providerStatusNotice.logic.test.ts | 54 ++++ .../components/chat/providerStatusNotice.tsx | 103 ++++++- apps/web/src/components/chat/statusNotice.tsx | 29 +- .../chat/threadErrorNotice.test.tsx | 50 +++- .../src/components/chat/threadErrorNotice.tsx | 9 +- .../settings/ProviderConnectFlow.tsx | 131 ++------- .../settings/ProviderInstanceCard.tsx | 7 +- .../components/settings/SettingsPanels.tsx | 21 +- .../components/settings/settingsNavigation.ts | 16 ++ .../settings/useProviderConnectFlow.ts | 253 ++++++++++++++++++ apps/web/src/lib/linkifiedText.test.tsx | 27 ++ apps/web/src/lib/linkifiedText.tsx | 97 +++++++ apps/web/src/routes/settings.providers.tsx | 5 +- apps/web/test/wsRpcHarness.ts | 1 + 22 files changed, 1354 insertions(+), 223 deletions(-) create mode 100644 apps/web/src/components/chat/providerSignIn.tsx create mode 100644 apps/web/src/components/settings/useProviderConnectFlow.ts create mode 100644 apps/web/src/lib/linkifiedText.test.tsx create mode 100644 apps/web/src/lib/linkifiedText.tsx diff --git a/apps/web/src/components/ChatView.browser.tsx b/apps/web/src/components/ChatView.browser.tsx index 90b426949..2321e83ce 100644 --- a/apps/web/src/components/ChatView.browser.tsx +++ b/apps/web/src/components/ChatView.browser.tsx @@ -3761,6 +3761,23 @@ describe("ChatView timeline estimator parity (full app)", () => { } }); + /** + * Push one provider-auth event down the same subscription the composer + * notice attaches to when it starts a sign-in. + */ + function emitProviderAuthEvent( + event: + | { type: "command"; flow: string; command: string } + | { type: "output"; data: string } + | { type: "status"; status: string; exitCode: number | null; detail: string | null }, + ) { + rpcHarness.emitStreamValue(WS_METHODS.providerAuthSubscribe, { + instanceId: "codex", + createdAt: new Date().toISOString(), + ...event, + }); + } + async function mountSignedOutProviderSend(options: { /** Providers the recheck behind "I've signed in" resolves with. */ refreshedProviders: (signedOut: ServerProvider) => ReadonlyArray; @@ -3785,6 +3802,11 @@ describe("ChatView timeline estimator parity (full app)", () => { if (body._tag === ORCHESTRATION_WS_METHODS.dispatchCommand) { return { sequence: fixture.snapshot.snapshotSequence + 1 }; } + // `providerAuth.start` succeeds with void; the harness's default `{}` + // would fail the response decode and surface as a start error. + if (body._tag === WS_METHODS.providerAuthStart) { + return null; + } if (body._tag === WS_METHODS.serverRefreshProviders) { return { providers: encodeServerConfig({ @@ -3817,9 +3839,106 @@ describe("ChatView timeline estimator parity (full app)", () => { "Explain this repo", ); - return { confirmSignedIn, mounted, turnStartRequests }; + const providerAuthStartRequests = () => + wsRequests.filter((request) => request._tag === WS_METHODS.providerAuthStart); + + return { confirmSignedIn, mounted, providerAuthStartRequests, turnStartRequests }; } + it("signs in from the held-send notice and releases the message without a second click", async () => { + const { mounted, providerAuthStartRequests, turnStartRequests } = + await mountSignedOutProviderSend({ + refreshedProviders: (signedOut) => [ + { ...signedOut, status: "ready", auth: { status: "authenticated" } }, + ], + }); + + try { + (await waitForButtonByText("Sign in")).click(); + + // The login runs in the server's own PTY, never in the thread's terminal. + await vi.waitFor( + () => { + expect(providerAuthStartRequests()).toHaveLength(1); + }, + { timeout: 8_000, interval: 16 }, + ); + expect(providerAuthStartRequests()[0]).toMatchObject({ instanceId: "codex", flow: "login" }); + + emitProviderAuthEvent({ type: "command", flow: "login", command: "codex login" }); + emitProviderAuthEvent({ type: "status", status: "running", exitCode: null, detail: null }); + emitProviderAuthEvent({ type: "output", data: "Opening browser to complete sign-in\r\n" }); + + await vi.waitFor( + () => { + expect(document.body.textContent).toContain( + "Signing in… Opening browser to complete sign-in", + ); + }, + { timeout: 8_000, interval: 16 }, + ); + + emitProviderAuthEvent({ type: "status", status: "succeeded", exitCode: 0, detail: null }); + + await vi.waitFor( + () => { + expect(turnStartRequests()).toHaveLength(1); + }, + { timeout: 8_000, interval: 16 }, + ); + // The success recheck is not a bypass, and it fires exactly once. + await waitForLayout(); + expect(turnStartRequests()).toHaveLength(1); + } finally { + await mounted.cleanup(); + } + }); + + it("keeps the held message and shows the last line when the sign-in fails", async () => { + const { mounted, providerAuthStartRequests, turnStartRequests } = + await mountSignedOutProviderSend({ + refreshedProviders: (signedOut) => [signedOut], + }); + + try { + (await waitForButtonByText("Sign in")).click(); + + await vi.waitFor( + () => { + expect(providerAuthStartRequests()).toHaveLength(1); + }, + { timeout: 8_000, interval: 16 }, + ); + + emitProviderAuthEvent({ type: "command", flow: "login", command: "codex login" }); + emitProviderAuthEvent({ type: "status", status: "running", exitCode: null, detail: null }); + emitProviderAuthEvent({ type: "output", data: "error: could not reach auth.openai.com\r\n" }); + emitProviderAuthEvent({ + type: "status", + status: "failed", + exitCode: 1, + detail: "codex login exited with code 1.", + }); + + await vi.waitFor( + () => { + expect(document.body.textContent).toContain( + "Sign-in failed. codex login exited with code 1.", + ); + }, + { timeout: 8_000, interval: 16 }, + ); + expect(turnStartRequests()).toHaveLength(0); + // The action comes back so the user can try again, and the draft is intact. + await expect.element(page.getByRole("button", { name: "Sign in" })).toBeVisible(); + expect(useComposerDraftStore.getState().draftsByThreadKey[THREAD_KEY]?.prompt).toBe( + "Explain this repo", + ); + } finally { + await mounted.cleanup(); + } + }); + it("sends the held message once the recheck behind I've signed in comes back clean", async () => { const { confirmSignedIn, mounted, turnStartRequests } = await mountSignedOutProviderSend({ refreshedProviders: (signedOut) => [ @@ -3859,7 +3978,7 @@ describe("ChatView timeline estimator parity (full app)", () => { { timeout: 8_000, interval: 16 }, ); expect(turnStartRequests()).toHaveLength(0); - expect(document.body.textContent).toContain("The terminal shows where the sign-in stopped."); + expect(document.body.textContent).toContain("The last sign-in did not complete."); expect(useComposerDraftStore.getState().draftsByThreadKey[THREAD_KEY]?.prompt).toBe( "Explain this repo", ); diff --git a/apps/web/src/components/ChatView.tsx b/apps/web/src/components/ChatView.tsx index 342d542bc..74ab375e5 100644 --- a/apps/web/src/components/ChatView.tsx +++ b/apps/web/src/components/ChatView.tsx @@ -217,6 +217,8 @@ import { } from "./chat/providerStatusNotice"; import { useSessionStartupNotice } from "./chat/sessionStartupNotice"; import { buildProviderSendPreflightNotice } from "./chat/providerReadinessNotice"; +import { toProviderSignInFlowView } from "./chat/providerSignIn"; +import { useProviderConnectFlow } from "./settings/useProviderConnectFlow"; import { buildThreadErrorNotice } from "./chat/threadErrorNotice"; import { type ComposerNotice, selectComposerNotices } from "./chat/composerNotices"; import { @@ -2494,6 +2496,33 @@ export default function ChatView(props: ChatViewProps) { // The recheck runs the ordinary send path, which is rebuilt every render. // Holding it behind a ref keeps the notice itself stable. const confirmProviderSignedInRef = useRef<() => void>(() => {}); + // One sign-in flow serves every composer notice: the held-send row and the + // provider-status row are mutually suppressed, and both speak about the + // instance the composer would send to. + const composerSignInInstanceId = + providerSendPreflight?.instanceId ?? activeProviderStatus?.instanceId ?? null; + const hasHeldSendRef = useRef(false); + hasHeldSendRef.current = providerSendPreflight !== null; + const composerSignInController = useProviderConnectFlow({ + instanceId: composerSignInInstanceId, + flow: "login", + // A held message is waiting on exactly this: re-probe and, if the provider + // agrees, send it. Without this the user would sign in and then still have + // to click "I've signed in". + onSucceeded: () => { + if (hasHeldSendRef.current) { + confirmProviderSignedInRef.current(); + } + }, + }); + const composerSignInView = useMemo( + () => + toProviderSignInFlowView({ + instanceId: composerSignInInstanceId, + controller: composerSignInController, + }), + [composerSignInController, composerSignInInstanceId], + ); useEffect(() => { // The notice belongs to the send it interrupted, so it must not follow the // user into another thread. @@ -3410,14 +3439,6 @@ export default function ChatView(props: ChatViewProps) { projectCwd={firstRunProject?.cwd ?? null} projectEnvironmentId={firstRunProject?.environmentId ?? environmentId} isOnlyWorkspaceProject={firstRunWorkspaceProjects.length === 1} - onSignIn={(row) => { - if (!row.signInCommand) return; - void runProviderAuthReconnect({ - provider: row.driverKind, - command: row.signInCommand, - message: `${row.name} is not signed in.`, - }); - }} onChooseProject={() => useCommandPaletteStore.getState().openAddProject()} onSkip={dismissFirstRunSetupForEnvironment} onStart={() => { @@ -3432,7 +3453,6 @@ export default function ChatView(props: ChatViewProps) { firstRunProject, firstRunWorkspaceProjects.length, providerInstanceEntries, - runProviderAuthReconnect, scheduleComposerFocus, showFirstRunSetupCard, ]); @@ -5240,6 +5260,7 @@ export default function ChatView(props: ChatViewProps) { const providerStatusNotice = useProviderStatusNotice({ status: activeProviderStatus, activeTurnInProgress, + signIn: composerSignInView, // The held-send notice and the setup card each already state this // provider's problem with the actions that fix it; a second ambient row // saying it again is the stacking noise the dock exists to end. @@ -5262,15 +5283,15 @@ export default function ChatView(props: ChatViewProps) { usageReset: threadErrorUsageResetAction, retry: threadErrorRetryAction, providerLabel: activeProviderLabel, - onRunAuthReconnect: runProviderAuthReconnect, + signIn: composerSignInView, onDismiss: () => setThreadError(activeThread?.id ?? null, null), }), [ activeProviderLabel, activeThread?.error, activeThread?.id, + composerSignInView, providerAuthReconnectPrompt, - runProviderAuthReconnect, setThreadError, threadErrorNoticeVisible, threadErrorRetryAction, @@ -5284,13 +5305,7 @@ export default function ChatView(props: ChatViewProps) { prompt: providerSendPreflight, recheckFailed: providerSendPreflightRecheckFailed, isRechecking: isRecheckingProviderSendPreflight, - onRunSignIn: (prompt) => { - void runProviderAuthReconnect({ - provider: prompt.provider, - command: prompt.command ?? "", - message: `${prompt.providerLabel} is not signed in.`, - }); - }, + signIn: composerSignInView, onConfirmSignedIn: () => confirmProviderSignedInRef.current(), onDismiss: () => { setProviderSendPreflight(null); @@ -5299,10 +5314,10 @@ export default function ChatView(props: ChatViewProps) { }) : null, [ + composerSignInView, isRecheckingProviderSendPreflight, providerSendPreflight, providerSendPreflightRecheckFailed, - runProviderAuthReconnect, ], ); const composerNotices = useMemo( diff --git a/apps/web/src/components/chat/FirstRunSetupCard.browser.tsx b/apps/web/src/components/chat/FirstRunSetupCard.browser.tsx index a5f33ef08..9a6f32a77 100644 --- a/apps/web/src/components/chat/FirstRunSetupCard.browser.tsx +++ b/apps/web/src/components/chat/FirstRunSetupCard.browser.tsx @@ -13,11 +13,107 @@ import { RouterProvider, } from "@tanstack/react-router"; import { page } from "vite-plus/test/browser"; -import { afterEach, describe, expect, it, vi } from "vite-plus/test"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vite-plus/test"; import { render } from "vitest-browser-react"; +/** + * The sign-in row drives the real server-side auth flow, so the card needs a + * primary environment. This is the same shape `SettingsPanels.browser` mocks: + * events reach only the subscribers of the matching instance. + */ +const providerAuthHarness = vi.hoisted(() => { + type AuthEvent = { + readonly instanceId: string; + readonly createdAt: string; + } & ( + | { readonly type: "command"; readonly flow: string; readonly command: string } + | { readonly type: "output"; readonly data: string } + | { + readonly type: "status"; + readonly status: string; + readonly exitCode: number | null; + readonly detail: string | null; + } + ); + + const listeners = new Set<{ + readonly instanceId: string; + readonly listener: (event: AuthEvent) => void; + }>(); + const startCalls: Array<{ instanceId: string; flow: string }> = []; + + return { + startCalls, + reset() { + listeners.clear(); + startCalls.length = 0; + }, + emit(event: AuthEvent) { + for (const entry of listeners) { + if (entry.instanceId === event.instanceId) { + entry.listener(event); + } + } + }, + client: { + start: (input: { instanceId: string; flow: string }) => { + startCalls.push({ instanceId: input.instanceId, flow: input.flow }); + return Promise.resolve(); + }, + write: () => Promise.resolve(), + resize: () => Promise.resolve(), + stop: () => Promise.resolve(), + subscribe: (input: { instanceId: string }, listener: (event: AuthEvent) => void) => { + const entry = { instanceId: input.instanceId, listener }; + listeners.add(entry); + return () => { + listeners.delete(entry); + }; + }, + }, + }; +}); + +vi.mock("../../environments/runtime", () => { + const primaryConnection = { client: { providerAuth: providerAuthHarness.client } } as never; + const notUsed = () => undefined as never; + return { + environmentUsesRelayTransport: () => false, + getEnvironmentHttpBaseUrl: () => "http://localhost:3000", + getSavedEnvironmentRecord: () => null, + getSavedEnvironmentRuntimeState: () => null, + hasSavedEnvironmentRegistryHydrated: () => true, + listSavedEnvironmentRecords: () => [], + readSavedEnvironmentBearerToken: () => null, + resetSavedEnvironmentRegistryStoreForTests: notUsed, + resetSavedEnvironmentRuntimeStoreForTests: notUsed, + resolveEnvironmentHttpUrl: (_environmentId: unknown, path: string) => + new URL(path, "http://localhost:3000").toString(), + waitForSavedEnvironmentRegistryHydration: async () => undefined, + useSavedEnvironmentRegistryStore: (selector: (state: { byId: object }) => unknown) => + selector({ byId: {} }), + useSavedEnvironmentRuntimeStore: (selector: (state: { byId: object }) => unknown) => + selector({ byId: {} }), + addSavedEnvironment: notUsed, + connectDesktopSshEnvironment: notUsed, + disconnectSavedEnvironment: notUsed, + ensureEnvironmentConnectionBootstrapped: async () => undefined, + getPrimaryEnvironmentConnection: () => primaryConnection, + markRelaySavedEnvironmentLinkExpired: notUsed, + readBackendEnvironmentConnection: () => primaryConnection, + readEnvironmentConnection: () => primaryConnection, + reconnectSavedEnvironment: notUsed, + RELAY_LINK_EXPIRED_MESSAGE: "", + removeSavedEnvironment: notUsed, + requireEnvironmentConnection: () => primaryConnection, + resetEnvironmentServiceForTests: notUsed, + startEnvironmentConnectionService: notUsed, + subscribeEnvironmentConnections: () => () => {}, + }; +}); + import { FirstRunSetupCard } from "./FirstRunSetupCard"; -import type { FirstRunProviderRow, FirstRunSetupProvider } from "./firstRunSetup"; +import type { FirstRunSetupProvider } from "./firstRunSetup"; function buildProvider(input: { readonly instanceId: string; @@ -85,7 +181,6 @@ const SIGNED_IN_CLAUDE = buildProvider({ function renderCard(props: { readonly providers: ReadonlyArray; readonly projectName: string | null; - readonly onSignIn?: (row: FirstRunProviderRow) => void; readonly onChooseProject?: () => void; readonly onSkip?: () => void; readonly onStart?: () => void; @@ -98,7 +193,6 @@ function renderCard(props: { projectCwd={props.projectName === null ? null : "C:/code/B-git-project"} projectEnvironmentId={null} isOnlyWorkspaceProject - onSignIn={props.onSignIn ?? vi.fn()} onChooseProject={props.onChooseProject ?? vi.fn()} onSkip={props.onSkip ?? vi.fn()} onStart={props.onStart ?? vi.fn()} @@ -128,16 +222,18 @@ function rowStates(): Record { } describe("FirstRunSetupCard", () => { + beforeEach(() => { + providerAuthHarness.reset(); + }); + afterEach(() => { document.body.innerHTML = ""; }); it("gives every provider state its own dot and action, and holds the start button back", async () => { - const onSignIn = vi.fn(); const screen = await renderCard({ providers: [SIGNED_OUT_CODEX, MISSING_CLAUDE], projectName: "B-git-project", - onSignIn, }); expect(rowStates()).toEqual({ @@ -166,14 +262,67 @@ describe("FirstRunSetupCard", () => { document.querySelector('a[href="/settings/providers"]')?.textContent, ).toContain("Install guide"); + await expect.element(page.getByRole("button", { name: "Start first thread" })).toBeDisabled(); + + await screen.unmount(); + }); + + it("runs the sign-in in place and swaps the row action for its live status", async () => { + const screen = await renderCard({ + providers: [SIGNED_OUT_CODEX], + projectName: "B-git-project", + }); + await page.getByRole("button", { name: "Sign in to Codex" }).click(); - expect(onSignIn).toHaveBeenCalledTimes(1); - expect(onSignIn.mock.calls[0]?.[0]).toMatchObject({ - name: "Codex", - signInCommand: "codex login", + + await vi.waitFor(() => { + expect(providerAuthHarness.startCalls).toEqual([{ instanceId: "codex", flow: "login" }]); }); - await expect.element(page.getByRole("button", { name: "Start first thread" })).toBeDisabled(); + providerAuthHarness.emit({ + instanceId: "codex", + createdAt: "2026-08-06T00:00:00.000Z", + type: "command", + flow: "login", + command: "codex login", + }); + providerAuthHarness.emit({ + instanceId: "codex", + createdAt: "2026-08-06T00:00:00.000Z", + type: "status", + status: "running", + exitCode: null, + detail: null, + }); + providerAuthHarness.emit({ + instanceId: "codex", + createdAt: "2026-08-06T00:00:01.000Z", + type: "output", + data: "Opening browser to complete sign-in\r\n", + }); + + await expect + .element(page.getByText("Signing in… Opening browser to complete sign-in")) + .toBeVisible(); + // The action is gone while the run owns the row: clicking it again would + // only restart the flow the user is in the middle of. + await expect + .element(page.getByRole("button", { name: "Sign in to Codex" })) + .not.toBeInTheDocument(); + + providerAuthHarness.emit({ + instanceId: "codex", + createdAt: "2026-08-06T00:00:05.000Z", + type: "status", + status: "failed", + exitCode: 1, + detail: "The sign-in command exited with code 1.", + }); + + await expect + .element(page.getByText("Sign-in failed. The sign-in command exited with code 1.")) + .toBeVisible(); + await expect.element(page.getByRole("button", { name: "Sign in to Codex" })).toBeVisible(); await screen.unmount(); }); diff --git a/apps/web/src/components/chat/FirstRunSetupCard.tsx b/apps/web/src/components/chat/FirstRunSetupCard.tsx index 0a137b691..6b60fd277 100644 --- a/apps/web/src/components/chat/FirstRunSetupCard.tsx +++ b/apps/web/src/components/chat/FirstRunSetupCard.tsx @@ -5,8 +5,12 @@ * It is the same provider data the settings page shows, with the fix action on * the row instead of two clicks away, plus the folder the server bootstrapped * from. Rows are live: provider snapshots stream in over providers-updated - * events, so a dot flips from amber to green while the sign-in terminal is - * still open, and "Start first thread" enables at the same moment. + * events, so a dot flips from amber to green as soon as a sign-in lands, and + * "Start first thread" enables at the same moment. + * + * Signing in never leaves this card. The row starts the same server-side auth + * session the Providers settings panel runs and reports it in place; only a + * run that stalls long enough to need a real terminal hands off to settings. * * No container: typography, spacing, and hairline dividers on the empty * canvas, matching the rest of the app. @@ -19,8 +23,14 @@ import { useCallback, useMemo, useState, type ReactNode } from "react"; import { cn } from "../../lib/utils"; import { ProjectFavicon } from "../ProjectFavicon"; +import { useProviderConnectFlow } from "../settings/useProviderConnectFlow"; import { riseDelay, ThreadlinesFigure } from "../ThreadlinesFigure"; import { Button } from "../ui/button"; +import { + ProviderSignInButton, + ProviderSignInInlineStatus, + toProviderSignInFlowView, +} from "./providerSignIn"; import { buildFirstRunSetupDismissalKey, canStartFirstThread, @@ -107,29 +117,34 @@ function SetupRow({ ) : null} {description} - {action} + {action} ); } -function providerRowAction( - row: FirstRunProviderRow, - onSignIn: (row: FirstRunProviderRow) => void, -): ReactNode { +/** + * The sign-in cell for one provider row. While a run is in flight the button + * gives way to the status line, so the row states what is happening instead of + * offering an action that would only restart it. + */ +function ProviderSignInRowAction({ row }: { row: FirstRunProviderRow }) { + const controller = useProviderConnectFlow({ instanceId: row.instanceId, flow: "login" }); + const view = toProviderSignInFlowView({ instanceId: row.instanceId, controller }); + + return ( + <> + + + + ); +} + +function providerRowAction(row: FirstRunProviderRow): ReactNode { if (row.state === "ready") { return null; } if (row.state === "needsSignIn" && row.signInCommand) { - return ( - - ); + return ; } return ( + {signIn ? : null} {extraActions} ), @@ -63,9 +73,9 @@ export function buildProviderSignInNotice({ } /** - * Same shape for a provider whose CLI is missing. There is no terminal command - * we can run for the user here, so the action routes to the place that lists - * install and sign-in steps. + * Same shape for a provider whose CLI is missing. There is nothing to sign in + * to yet, so the action routes to the place that lists install and sign-in + * steps. */ export function buildProviderNotInstalledNotice({ id, @@ -103,26 +113,28 @@ export function buildProviderNotInstalledNotice({ * Shown in place of a turn we held back. Nothing is ever hard-blocked: the * snapshot behind the verdict can be stale, so "I've signed in" re-checks the * provider and, when the fresh snapshot agrees, sends the held message down - * the ordinary path. + * the ordinary path. A sign-in started from this row runs that same recheck by + * itself once the server reports success, so the held message releases without + * a second click. */ export function buildProviderSendPreflightNotice({ prompt, recheckFailed, isRechecking, - onRunSignIn, + signIn, onConfirmSignedIn, onDismiss, }: { prompt: ProviderSendPreflightPrompt; recheckFailed: boolean; isRechecking: boolean; - onRunSignIn: (prompt: ProviderSendPreflightPrompt) => void; + signIn: ProviderSignInFlowView; onConfirmSignedIn: () => void; onDismiss: () => void; }): ComposerNotice { // A provider that isn't installed has nothing to sign in to yet, and one - // without a known login command has no terminal step we can name, so both - // fall back to the install-and-connect notice. + // without a known login command has no flow we can start, so both fall back + // to the install-and-connect notice. if (prompt.reason === "notInstalled" || !prompt.command) { return buildProviderNotInstalledNotice({ id: "provider-send-preflight", @@ -148,20 +160,19 @@ export function buildProviderSendPreflightNotice({ id: "provider-send-preflight", severity: "warning", providerLabel: prompt.providerLabel, - command: prompt.command, - instruction: "then your message sends", - extraActions: confirmSignedIn, - onRunSignIn: () => onRunSignIn(prompt), + instruction: "Sign in and your held message sends by itself.", + extraActions: isProviderSignInInFlight(signIn) ? null : confirmSignedIn, + signIn, onDismiss, }); - if (!recheckFailed) { + if (!recheckFailed || isProviderSignInInFlight(signIn)) { return notice; } return { ...notice, lead: "Still signed out.", - detail: "The terminal shows where the sign-in stopped.", + detail: providerSignInStatusText(signIn) ?? "The last sign-in did not complete.", }; } diff --git a/apps/web/src/components/chat/providerSignIn.tsx b/apps/web/src/components/chat/providerSignIn.tsx new file mode 100644 index 000000000..a73784cdd --- /dev/null +++ b/apps/web/src/components/chat/providerSignIn.tsx @@ -0,0 +1,184 @@ +/** + * "Sign in" outside the settings page. + * + * The setup card and the composer notices used to open the thread's terminal + * and type the provider's login command into it, which put a raw shell in + * front of someone who had asked for one button. They now drive the same + * server-side flow the Providers settings panel uses + * (`useProviderConnectFlow`), and report it as one line of text on the row + * they already occupy. + * + * Neither surface has room for the interactive terminal that settings can + * open, so a run that stalls past the auto-expand threshold hands off: the + * action becomes "Open Settings", deep-linked to this instance's card, where + * the same live session is waiting with its terminal and copy fallback. + * + * @module providerSignIn + */ +import type { ProviderInstanceId } from "@threadlines/contracts"; +import { Link } from "@tanstack/react-router"; +import { LoaderIcon } from "lucide-react"; +import type { ReactNode } from "react"; + +import { cn } from "../../lib/utils"; +import type { ProviderConnectFlowController } from "../settings/useProviderConnectFlow"; +import { Button } from "../ui/button"; + +/** + * The slice of a `ProviderConnectFlowController` the chat surfaces read. + * Declared structurally so a controller passes through unchanged and the + * notice builders stay free of hooks. + */ +export interface ProviderSignInFlowView { + readonly instanceId: ProviderInstanceId | null; + readonly isActive: boolean; + readonly isStarting: boolean; + readonly hasRun: boolean; + readonly hasFailed: boolean; + /** True once the run has earned the terminal only settings can show. */ + readonly needsTerminal: boolean; + /** Last visible output line, already stripped of control sequences. */ + readonly lastLine: string; + /** The server's plain-language failure reason, when it gave one. */ + readonly failureDetail: string | null; + readonly start: () => void; +} + +export function toProviderSignInFlowView(input: { + readonly instanceId: ProviderInstanceId | null; + readonly controller: ProviderConnectFlowController; +}): ProviderSignInFlowView { + const { controller, instanceId } = input; + return { + instanceId, + isActive: controller.isActive, + isStarting: controller.isStarting, + hasRun: controller.hasRun, + hasFailed: controller.hasRun && controller.state.status === "failed", + needsTerminal: controller.hasRun && controller.needsTerminal, + lastLine: controller.state.lastLine, + failureDetail: controller.startError ?? controller.state.detail, + start: controller.start, + }; +} + +/** True while the surface should describe the flow instead of offering it. */ +export function isProviderSignInInFlight(view: ProviderSignInFlowView): boolean { + return view.isActive || view.isStarting; +} + +/** + * One line describing where the run is, or null when there is nothing to say + * yet. Deliberately short: every surface that shows it truncates. + */ +export function providerSignInStatusText(view: ProviderSignInFlowView): string | null { + if (isProviderSignInInFlight(view)) { + return view.lastLine.trim().length > 0 + ? `Signing in… ${view.lastLine}` + : "Signing in… finish it in your browser."; + } + if (view.hasFailed) { + const reason = view.failureDetail?.trim() || view.lastLine.trim(); + return reason.length > 0 ? `Sign-in failed. ${reason}` : "Sign-in failed."; + } + return null; +} + +/** + * The primary action: start the flow, retry a failed one, or step aside for + * the settings hand-off once the run needs a terminal. + */ +export function ProviderSignInButton({ + view, + label = "Sign in", + ariaLabel, + variant, + className, +}: { + readonly view: ProviderSignInFlowView; + readonly label?: string | undefined; + readonly ariaLabel?: string | undefined; + readonly variant?: "default" | "outline" | "ghost" | undefined; + readonly className?: string | undefined; +}): ReactNode { + if (view.needsTerminal && !view.hasFailed) { + return ; + } + if (isProviderSignInInFlight(view)) { + return null; + } + return ( + + ); +} + +/** + * The hand-off. Keeps the approved "Open Settings" label rather than minting + * a third phrase for what is the same destination as every other settings + * action in these rows. + */ +export function ProviderSignInSettingsLink({ + instanceId, + className, +}: { + readonly instanceId: ProviderInstanceId | null; + readonly className?: string | undefined; +}): ReactNode { + return ( + + ); +} + +/** + * Compact status for a list row: a spinner at the same size as every other + * inline loader in the app, the state, and the last output line. + */ +export function ProviderSignInInlineStatus({ + view, + className, +}: { + readonly view: ProviderSignInFlowView; + readonly className?: string | undefined; +}): ReactNode { + const text = providerSignInStatusText(view); + if (text === null) { + return null; + } + return ( + + {isProviderSignInInFlight(view) ? ( + + ) : null} + {text} + + ); +} diff --git a/apps/web/src/components/chat/providerStatusNotice.browser.tsx b/apps/web/src/components/chat/providerStatusNotice.browser.tsx index 095739cbf..0e1277c4e 100644 --- a/apps/web/src/components/chat/providerStatusNotice.browser.tsx +++ b/apps/web/src/components/chat/providerStatusNotice.browser.tsx @@ -30,18 +30,36 @@ vi.mock("../../localApi", () => ({ })); import { ComposerNoticeDock } from "./ComposerNoticeDock"; +import type { ProviderSignInFlowView } from "./providerSignIn"; import { useProviderStatusNotice } from "./providerStatusNotice"; +function makeSignInView(overrides: Partial = {}): ProviderSignInFlowView { + return { + instanceId: ProviderInstanceId.make("codex"), + isActive: false, + isStarting: false, + hasRun: false, + hasFailed: false, + needsTerminal: false, + lastLine: "", + failureDetail: null, + start: () => {}, + ...overrides, + }; +} + function ProviderStatusNoticeHarness({ status, activeTurnInProgress = false, suppressed = false, + signIn, }: { status: ServerProvider | null; activeTurnInProgress?: boolean; suppressed?: boolean; + signIn?: ProviderSignInFlowView; }) { - const notice = useProviderStatusNotice({ activeTurnInProgress, status, suppressed }); + const notice = useProviderStatusNotice({ activeTurnInProgress, status, suppressed, signIn }); return ; } @@ -53,8 +71,16 @@ function renderWithTestRouter(children: ReactNode) { getParentRoute: () => rootRoute, path: "/", }); + const diagnosticsRoute = createRoute({ + getParentRoute: () => rootRoute, + path: "/settings/diagnostics", + }); + const providersRoute = createRoute({ + getParentRoute: () => rootRoute, + path: "/settings/providers", + }); const router = createRouter({ - routeTree: rootRoute.addChildren([indexRoute]), + routeTree: rootRoute.addChildren([indexRoute, diagnosticsRoute, providersRoute]), history: createMemoryHistory({ initialEntries: ["/"] }), }); @@ -122,6 +148,84 @@ describe("provider status composer notice", () => { } }); + it("offers sign-in alone for a signed-out provider", async () => { + const start = vi.fn(); + const provider = makeProvider({ + status: "error", + auth: { status: "unauthenticated" }, + message: "Codex CLI is not authenticated.", + }); + const screen = await renderWithTestRouter( + , + ); + + try { + await page.getByRole("button", { name: "Sign in" }).click(); + expect(start).toHaveBeenCalledTimes(1); + // Nothing to refresh and nothing in the logs: the snapshot already knows. + await expect + .element(page.getByRole("button", { name: "Refresh provider status" })) + .not.toBeInTheDocument(); + await expect + .element(page.getByRole("link", { name: "Open diagnostics" })) + .not.toBeInTheDocument(); + expect(refreshProvidersMock).not.toHaveBeenCalled(); + } finally { + await screen.unmount(); + } + }); + + it("routes a missing CLI to provider settings and keeps a refresh", async () => { + const provider = makeProvider({ + installed: false, + status: "error", + message: "Codex CLI not detected on PATH. Install it from https://codex.dev/install.", + }); + const screen = await renderWithTestRouter(); + + try { + await expect + .element(page.getByRole("link", { name: "Open Settings" })) + .toHaveAttribute("href", "/settings/providers"); + await expect + .element(page.getByRole("button", { name: "Refresh provider status" })) + .toBeVisible(); + await expect + .element(page.getByRole("link", { name: "Open diagnostics" })) + .not.toBeInTheDocument(); + // The install address in the server's message is clickable, not dead text. + await expect + .element(page.getByRole("link", { name: "https://codex.dev/install" })) + .toHaveAttribute("href", "https://codex.dev/install"); + } finally { + await screen.unmount(); + } + }); + + it("hands a stalled sign-in over to settings", async () => { + const provider = makeProvider({ status: "error", auth: { status: "unauthenticated" } }); + const screen = await renderWithTestRouter( + , + ); + + try { + await expect.element(page.getByText("Signing in to Codex.")).toBeVisible(); + await expect + .element(page.getByRole("link", { name: "Open provider settings to finish signing in" })) + .toHaveAttribute("href", "/settings/providers?instance=codex"); + } finally { + await screen.unmount(); + } + }); + it("does not show provider probe warnings over an active turn", async () => { const screen = await renderWithTestRouter( , diff --git a/apps/web/src/components/chat/providerStatusNotice.logic.test.ts b/apps/web/src/components/chat/providerStatusNotice.logic.test.ts index 7c7513f12..bd8e574c6 100644 --- a/apps/web/src/components/chat/providerStatusNotice.logic.test.ts +++ b/apps/web/src/components/chat/providerStatusNotice.logic.test.ts @@ -7,6 +7,7 @@ import { import { PROVIDER_STATUS_SLOW_NOTICE_DELAY_MS, + resolveProviderStatusNoticeActions, shouldShowProviderStatusNotice, } from "./providerStatusNotice"; @@ -102,3 +103,56 @@ describe("shouldShowProviderStatusNotice", () => { ).toBe(true); }); }); + +describe("resolveProviderStatusNoticeActions", () => { + it("offers only sign-in when the provider is unauthenticated", () => { + expect( + resolveProviderStatusNoticeActions( + makeProvider({ status: "error", auth: { status: "unauthenticated" } }), + ), + ).toEqual({ signIn: true, openSettings: false, refresh: false, diagnostics: false }); + }); + + it("offers only sign-in when the chat capability is unavailable", () => { + expect( + resolveProviderStatusNoticeActions( + makeProvider({ + status: "warning", + auth: { status: "unknown", capabilities: { chat: { status: "unavailable" } } }, + }), + ), + ).toEqual({ signIn: true, openSettings: false, refresh: false, diagnostics: false }); + }); + + it("sends a missing CLI to settings, with a refresh for an install that just landed", () => { + expect( + resolveProviderStatusNoticeActions( + makeProvider({ installed: false, status: "error", auth: { status: "unauthenticated" } }), + ), + ).toEqual({ signIn: false, openSettings: true, refresh: true, diagnostics: false }); + }); + + it("sends a disabled instance to settings and nowhere else", () => { + expect( + resolveProviderStatusNoticeActions(makeProvider({ enabled: false, status: "error" })), + ).toEqual({ signIn: false, openSettings: true, refresh: false, diagnostics: false }); + }); + + it("keeps refresh and diagnostics for probes and for anything it cannot name", () => { + const probeTimeout = makeProvider({ statusReason: "provider_probe_timeout" }); + const cannotVerify = makeProvider({ status: "warning", auth: { status: "unknown" } }); + + expect(resolveProviderStatusNoticeActions(probeTimeout)).toEqual({ + signIn: false, + openSettings: false, + refresh: true, + diagnostics: true, + }); + expect(resolveProviderStatusNoticeActions(cannotVerify)).toEqual({ + signIn: false, + openSettings: false, + refresh: true, + diagnostics: true, + }); + }); +}); diff --git a/apps/web/src/components/chat/providerStatusNotice.tsx b/apps/web/src/components/chat/providerStatusNotice.tsx index 874f887db..68eb4bcaa 100644 --- a/apps/web/src/components/chat/providerStatusNotice.tsx +++ b/apps/web/src/components/chat/providerStatusNotice.tsx @@ -1,13 +1,30 @@ /** * The composer notice for an unhealthy provider snapshot. * + * One row, but not one action: "Refresh and Diagnostics" is the right answer + * for a probe that timed out and the wrong answer for a provider that is + * simply signed out, where the only useful button is the one that signs in. + * `resolveProviderStatusNoticeActions` reads the structured snapshot fields + * (never the message text, which is provider prose) and says which actions the + * row earns. + * * @module providerStatusNotice */ import { ProviderDriverKind, type ServerProvider } from "@threadlines/contracts"; +import { Link } from "@tanstack/react-router"; +import { SettingsIcon } from "lucide-react"; import { useEffect, useMemo, useState } from "react"; +import { LinkifiedText } from "../../lib/linkifiedText"; import { formatProviderDriverKindLabel } from "../../providerModels"; +import { Button } from "../ui/button"; import type { ComposerNotice } from "./composerNotices"; +import { + isProviderSignInInFlight, + providerSignInStatusText, + ProviderSignInButton, + type ProviderSignInFlowView, +} from "./providerSignIn"; import { StatusNoticeActionButtons, useProviderStatusRefresh } from "./statusNotice"; const CODEX_DRIVER_KIND = ProviderDriverKind.make("codex"); @@ -66,6 +83,46 @@ export function shouldShowProviderStatusNotice( return true; } +/** + * Which actions a snapshot earns. Every field is decided from structured + * state, so a provider that reworded its error message cannot change the + * buttons the user is offered. + */ +export interface ProviderStatusNoticeActions { + /** Start the hidden sign-in flow. Never shares the row with the rest. */ + readonly signIn: boolean; + /** Route to the providers page, for something no button here can fix. */ + readonly openSettings: boolean; + readonly refresh: boolean; + readonly diagnostics: boolean; +} + +export function resolveProviderStatusNoticeActions( + status: ServerProvider, +): ProviderStatusNoticeActions { + // Turned off on purpose: the only thing to do about it is turn it back on, + // and re-probing a disabled provider tells nobody anything. + if (!status.enabled) { + return { signIn: false, openSettings: true, refresh: false, diagnostics: false }; + } + // No CLI on PATH. There is nothing to sign in to, but a refresh is worth + // offering because an install that just finished is invisible until we look. + if (!status.installed) { + return { signIn: false, openSettings: true, refresh: true, diagnostics: false }; + } + // Installed and definitely signed out. Refreshing and reading diagnostics + // both just confirm what the snapshot already says. + if ( + status.auth.capabilities?.chat?.status === "unavailable" || + status.auth.status === "unauthenticated" + ) { + return { signIn: true, openSettings: false, refresh: false, diagnostics: false }; + } + // Everything else is a probe that has not answered, timed out, or came back + // unhappy in a way we cannot name: look again, or go read the logs. + return { signIn: false, openSettings: false, refresh: true, diagnostics: true }; +} + /** * Builds the provider-status notice, including the delayed reveal for a Codex * probe that is merely slow: a probe that has not answered yet is usually @@ -75,6 +132,12 @@ export function shouldShowProviderStatusNotice( export function useProviderStatusNotice(input: { readonly status: ServerProvider | null; readonly activeTurnInProgress: boolean; + /** + * Live state of this instance's sign-in flow, supplied by the surface that + * owns it. Without it, a signed-out provider still gets the sign-in row but + * cannot report progress. + */ + readonly signIn?: ProviderSignInFlowView | undefined; /** * True while a held-send notice is up for this same instance. That notice * states the identical fact with the actions that resolve it, and severity @@ -82,7 +145,7 @@ export function useProviderStatusNotice(input: { */ readonly suppressed?: boolean; }): ComposerNotice | null { - const { activeTurnInProgress, status, suppressed = false } = input; + const { activeTurnInProgress, signIn, status, suppressed = false } = input; const [nowMs, setNowMs] = useState(() => Date.now()); const { isRefreshing, refreshError, refreshProvider } = useProviderStatusRefresh( status?.instanceId ?? null, @@ -120,6 +183,9 @@ export function useProviderStatusNotice(input: { } const providerLabel = status.displayName?.trim() || formatProviderDriverKindLabel(status.driver); + const actions = resolveProviderStatusNoticeActions(status); + const signInRunning = + actions.signIn && signIn !== undefined && isProviderSignInInFlight(signIn); const defaultMessage = status.status === "error" ? `${providerLabel} provider is unavailable.` @@ -128,19 +194,38 @@ export function useProviderStatusNotice(input: { status.statusReason === "provider_probe_pending" ? `${providerLabel} status check is taking longer than usual. Existing sessions may still work.` : (status.message ?? defaultMessage); + const flowStatus = actions.signIn && signIn ? providerSignInStatusText(signIn) : null; + const detailText = refreshError ? `${message} ${refreshError}` : message; return { id: `provider-status:${status.instanceId}`, severity: status.status === "error" ? "error" : "warning", - lead: `${providerLabel} provider status`, - detail: refreshError ? `${message} ${refreshError}` : message, + lead: signInRunning ? `Signing in to ${providerLabel}.` : `${providerLabel} provider status`, + detail: flowStatus ?? , actions: ( - + <> + {actions.signIn && signIn ? : null} + {actions.openSettings ? ( + + ) : null} + {actions.refresh || actions.diagnostics ? ( + + ) : null} + ), } satisfies ComposerNotice; - }, [isRefreshing, refreshError, refreshProvider, status, visible]); + }, [isRefreshing, refreshError, refreshProvider, signIn, status, visible]); } diff --git a/apps/web/src/components/chat/statusNotice.tsx b/apps/web/src/components/chat/statusNotice.tsx index 43c053904..3631dfaec 100644 --- a/apps/web/src/components/chat/statusNotice.tsx +++ b/apps/web/src/components/chat/statusNotice.tsx @@ -37,10 +37,17 @@ export function StatusNoticeActionButtons({ variant = "ghost", isRefreshing, onRefresh, + showDiagnostics = true, }: { variant?: "ghost" | "outline"; isRefreshing: boolean; onRefresh: (() => void) | null; + /** + * Diagnostics is the answer to "the server tried and something went wrong". + * Rows for a condition the logs cannot explain (a signed-out CLI, a missing + * install) turn it off rather than sending the user to read nothing. + */ + showDiagnostics?: boolean; }) { const buttonClassName = variant === "ghost" ? "h-6 px-1.5" : undefined; return ( @@ -62,16 +69,18 @@ export function StatusNoticeActionButtons({ {isRefreshing ? "Refreshing" : "Refresh"} ) : null} - + {showDiagnostics ? ( + + ) : null} ); } diff --git a/apps/web/src/components/chat/threadErrorNotice.test.tsx b/apps/web/src/components/chat/threadErrorNotice.test.tsx index 9281eab0f..5ad5df0e2 100644 --- a/apps/web/src/components/chat/threadErrorNotice.test.tsx +++ b/apps/web/src/components/chat/threadErrorNotice.test.tsx @@ -1,21 +1,37 @@ -import { ProviderDriverKind } from "@threadlines/contracts"; +import { ProviderDriverKind, ProviderInstanceId } from "@threadlines/contracts"; import { renderToStaticMarkup } from "react-dom/server"; import { describe, expect, it } from "vite-plus/test"; import type { ComposerNotice } from "./composerNotices"; import { ComposerNoticeDock } from "./ComposerNoticeDock"; +import type { ProviderSignInFlowView } from "./providerSignIn"; import { buildThreadErrorNotice } from "./threadErrorNotice"; function renderNotice(notice: ComposerNotice | null): string { return renderToStaticMarkup(); } +function idleSignIn(overrides: Partial = {}): ProviderSignInFlowView { + return { + instanceId: ProviderInstanceId.make("claudeAgent"), + isActive: false, + isStarting: false, + hasRun: false, + hasFailed: false, + needsTerminal: false, + lastLine: "", + failureDetail: null, + start: () => {}, + ...overrides, + }; +} + describe("buildThreadErrorNotice", () => { it("produces nothing without an error", () => { expect(buildThreadErrorNotice({ error: null })).toBe(null); }); - it("renders provider auth recovery steps and terminal action", () => { + it("offers the hidden sign-in flow, not a terminal command, for provider auth failures", () => { const markup = renderNotice( buildThreadErrorNotice({ error: "Failed to authenticate. API Error: 401 Invalid authentication credentials", @@ -25,19 +41,43 @@ describe("buildThreadErrorNotice", () => { command: "claude auth login", message: "Failed to authenticate. API Error: 401 Invalid authentication credentials", }, - onRunAuthReconnect: () => {}, + signIn: idleSignIn(), }), ); expect(markup).toContain("Claude needs sign-in."); - expect(markup).toContain("claude auth login"); - expect(markup).toContain("complete the browser sign-in, then retry"); + expect(markup).toContain("Complete the browser step and come back here."); expect(markup).toContain("Last error: Failed to authenticate."); expect(markup).toContain(">Sign in<"); + // The command never reaches the user: Threadlines runs it for them. + expect(markup).not.toContain("claude auth login"); expect(markup).toContain('data-composer-notice-severity="error"'); expect(markup).toContain('role="alert"'); }); + it("reports a running sign-in on the row instead of the action", () => { + const markup = renderNotice( + buildThreadErrorNotice({ + error: "Failed to authenticate. API Error: 401 Invalid authentication credentials", + providerLabel: "Claude", + authReconnect: { + provider: ProviderDriverKind.make("claudeAgent"), + command: "claude auth login", + message: "Failed to authenticate.", + }, + signIn: idleSignIn({ + isActive: true, + hasRun: true, + lastLine: "Opening browser to complete sign-in", + }), + }), + ); + + expect(markup).toContain("Signing in to Claude."); + expect(markup).toContain("Opening browser to complete sign-in"); + expect(markup).not.toContain(">Sign in<"); + }); + it("renders a Codex usage reset action for usage-limit errors", () => { const markup = renderNotice( buildThreadErrorNotice({ diff --git a/apps/web/src/components/chat/threadErrorNotice.tsx b/apps/web/src/components/chat/threadErrorNotice.tsx index f1e8095bd..57a295229 100644 --- a/apps/web/src/components/chat/threadErrorNotice.tsx +++ b/apps/web/src/components/chat/threadErrorNotice.tsx @@ -10,6 +10,7 @@ import { formatProviderRateLimitResetCreditTooltip } from "../ProviderRateLimitR import { Button } from "../ui/button"; import type { ComposerNotice } from "./composerNotices"; import { buildProviderSignInNotice } from "./providerReadinessNotice"; +import type { ProviderSignInFlowView } from "./providerSignIn"; interface UsageResetAction { readonly availableCount: number; @@ -28,7 +29,7 @@ export function buildThreadErrorNotice({ usageReset, retry, providerLabel, - onRunAuthReconnect, + signIn, onDismiss, }: { error: string | null; @@ -36,7 +37,8 @@ export function buildThreadErrorNotice({ usageReset?: UsageResetAction | null; retry?: TurnRetryAction | null; providerLabel?: string; - onRunAuthReconnect?: (action: ProviderAuthReconnectAction) => void; + /** Live state of the active instance's sign-in flow. */ + signIn?: ProviderSignInFlowView | undefined; onDismiss?: () => void; }): ComposerNotice | null { if (!error) { @@ -47,9 +49,8 @@ export function buildThreadErrorNotice({ return buildProviderSignInNotice({ id: "thread-error-auth", providerLabel: providerLabel?.trim() || "Provider", - command: authReconnect.command, detailSuffix: `Last error: ${error}`, - onRunSignIn: onRunAuthReconnect ? () => onRunAuthReconnect(authReconnect) : undefined, + signIn, ...(onDismiss ? { onDismiss } : {}), }); } diff --git a/apps/web/src/components/settings/ProviderConnectFlow.tsx b/apps/web/src/components/settings/ProviderConnectFlow.tsx index 943daa0b4..ccef68974 100644 --- a/apps/web/src/components/settings/ProviderConnectFlow.tsx +++ b/apps/web/src/components/settings/ProviderConnectFlow.tsx @@ -1,13 +1,9 @@ "use client"; -import type { - ProviderAuthEvent, - ProviderAuthFlow, - ProviderInstanceId, -} from "@threadlines/contracts"; +import type { ProviderAuthFlow, ProviderInstanceId } from "@threadlines/contracts"; import type { Terminal } from "@xterm/xterm"; import { CheckIcon, ChevronDownIcon, CopyIcon, LoaderIcon } from "lucide-react"; -import { useEffect, useEffectEvent, useRef, useState } from "react"; +import { useEffect, useRef, useState } from "react"; import { getPrimaryEnvironmentConnection } from "../../environments/runtime"; import { useCopyToClipboard } from "../../hooks/useCopyToClipboard"; @@ -15,18 +11,8 @@ import { cn } from "../../lib/utils"; import { Button } from "../ui/button"; import { stackedThreadToast, toastManager } from "../ui/toast"; import { createXtermSurface } from "../terminal/xtermSurface"; -import { - applyProviderAuthEvent, - initialProviderConnectFlowState, - isProviderConnectFlowActive, - providerConnectStatusLine, - shouldAutoExpandTerminal, - type ProviderConnectFlowState, -} from "./providerConnectFlow.logic"; - -const TERMINAL_COLS = 100; -const TERMINAL_ROWS = 20; -const AUTO_EXPAND_TICK_MS = 1_000; +import { providerConnectStatusLine } from "./providerConnectFlow.logic"; +import { useProviderConnectFlow } from "./useProviderConnectFlow"; interface ProviderConnectTerminalProps { readonly instanceId: ProviderInstanceId; @@ -141,17 +127,8 @@ export function ProviderConnectFlow({ statusRow, buttonVariant = "default", }: ProviderConnectFlowProps) { - const [state, setState] = useState(initialProviderConnectFlowState); - const [isStarting, setIsStarting] = useState(false); const [showFallback, setShowFallback] = useState(false); const [showTerminal, setShowTerminal] = useState(false); - const [runningForMs, setRunningForMs] = useState(0); - const outputBufferRef = useRef(""); - const terminalWriteRef = useRef<((data: string) => void) | null>(null); - // An instance has one auth session but can render two panels (sign-in and - // token setup). Sessions announce their flow in the "command" event; a - // panel ignores sessions that belong to the other flow. - const sessionFlowRef = useRef(null); const { copyToClipboard, isCopied } = useCopyToClipboard<"provider-auth-command">({ onError: (error) => { toastManager.add( @@ -164,58 +141,29 @@ export function ProviderConnectFlow({ }, }); - const handleEvent = useEffectEvent((event: ProviderAuthEvent) => { - if (event.type === "command") { - sessionFlowRef.current = event.flow; - if (event.flow !== flow) { - outputBufferRef.current = ""; - setShowTerminal(false); - setState(initialProviderConnectFlowState); - return; - } - } else if (sessionFlowRef.current !== flow) { - // The server announces a session's command (and flow) before any output - // or status, so anything arriving unclaimed belongs to the other panel. - return; - } - if (event.type === "output") { - outputBufferRef.current = `${outputBufferRef.current}${event.data}`.slice(-64_000); - terminalWriteRef.current?.(event.data); - } - setState((previous) => applyProviderAuthEvent(previous, event)); + const { + state, + isStarting, + isActive, + needsTerminal, + outputBufferRef, + terminalWriteRef, + start, + reset, + } = useProviderConnectFlow({ + instanceId, + flow, + onStartError: (error) => { + toastManager.add( + stackedThreadToast({ + type: "error", + title: `Could not start ${displayName} sign-in`, + description: error instanceof Error ? error.message : "The command could not be started.", + }), + ); + }, }); - // Subscribed for the component's whole lifetime, not just after a click: - // the server replays the command, buffered output, and status on attach, so - // a flow started before a tab switch or remount lands back on the panel. - useEffect(() => { - let cancelled = false; - const client = getPrimaryEnvironmentConnection().client; - const unsubscribe = client.providerAuth.subscribe({ instanceId }, (event) => { - if (cancelled) return; - handleEvent(event); - }); - return () => { - cancelled = true; - unsubscribe(); - }; - }, [instanceId]); - - const isActive = isProviderConnectFlowActive(state.status); - - useEffect(() => { - if (!isActive) { - setRunningForMs(0); - return; - } - const startedAt = Date.now(); - const timer = window.setInterval( - () => setRunningForMs(Date.now() - startedAt), - AUTO_EXPAND_TICK_MS, - ); - return () => window.clearInterval(timer); - }, [isActive]); - useEffect(() => { if (state.status === "succeeded") { // The job is done and, for token flows, the transcript is no longer @@ -223,41 +171,20 @@ export function ProviderConnectFlow({ setShowTerminal(false); return; } - if (shouldAutoExpandTerminal({ status: state.status, runningForMs })) { + if (needsTerminal) { setShowTerminal(true); } - }, [runningForMs, state.status]); + }, [needsTerminal, state.status]); const startFlow = () => { - outputBufferRef.current = ""; - setState(initialProviderConnectFlowState); setShowTerminal(false); - setIsStarting(true); - void getPrimaryEnvironmentConnection() - .client.providerAuth.start({ instanceId, flow, cols: TERMINAL_COLS, rows: TERMINAL_ROWS }) - .catch((error: unknown) => { - toastManager.add( - stackedThreadToast({ - type: "error", - title: `Could not start ${displayName} sign-in`, - description: - error instanceof Error ? error.message : "The command could not be started.", - }), - ); - }) - .finally(() => { - setIsStarting(false); - }); + start(); }; // Cancel and Dismiss both clear the server-side session too, so a finished // run doesn't replay a stale success/failure panel on the next visit. const dismissFlow = () => { - void getPrimaryEnvironmentConnection() - .client.providerAuth.stop({ instanceId }) - .catch(() => {}); - outputBufferRef.current = ""; - setState(initialProviderConnectFlowState); + reset(); setShowTerminal(false); }; diff --git a/apps/web/src/components/settings/ProviderInstanceCard.tsx b/apps/web/src/components/settings/ProviderInstanceCard.tsx index dd1e2bb98..fc841918f 100644 --- a/apps/web/src/components/settings/ProviderInstanceCard.tsx +++ b/apps/web/src/components/settings/ProviderInstanceCard.tsx @@ -37,6 +37,7 @@ import { upsertClaudeLongLivedOAuthTokenEnvironment, } from "@threadlines/shared/providerAuthCommands"; +import { LinkifiedText } from "../../lib/linkifiedText"; import { cn } from "../../lib/utils"; import { deriveProviderAccountUsagePresentationForProvider, @@ -1468,7 +1469,11 @@ export function ProviderInstanceCard({ )} - {summary.detail ? - {summary.detail} : null} + {summary.detail ? ( + + - + + ) : null}

); diff --git a/apps/web/src/components/settings/SettingsPanels.tsx b/apps/web/src/components/settings/SettingsPanels.tsx index ba622f208..83a0ab6f4 100644 --- a/apps/web/src/components/settings/SettingsPanels.tsx +++ b/apps/web/src/components/settings/SettingsPanels.tsx @@ -8,7 +8,7 @@ import { } from "lucide-react"; import { useQueryClient } from "@tanstack/react-query"; import { Link } from "@tanstack/react-router"; -import { useCallback, useMemo, useRef, useState } from "react"; +import { useCallback, useEffect, useMemo, useRef, useState } from "react"; import { AUTO_ARCHIVE_INACTIVE_THREADS_DAY_OPTIONS, type AutoArchiveInactiveThreadsDays, @@ -1060,7 +1060,16 @@ export function GeneralSettingsPanel({ surface = "full" }: { surface?: "full" | ); } -export function ProviderSettingsPanel() { +export function ProviderSettingsPanel({ + focusedInstanceId = null, +}: { + /** + * The card to open on arrival, from the route's `?instance=`. It is how a + * sign-in started elsewhere in the app hands off to this page, which owns + * the interactive terminal those surfaces have no room for. + */ + readonly focusedInstanceId?: string | null; +} = {}) { const settings = useSettings(); const { updateSettings } = useUpdateSettings(); const serverProviders = useServerProviders(); @@ -1073,6 +1082,14 @@ export function ProviderSettingsPanel() { ReadonlySet >(() => new Set()); const [openInstanceDetails, setOpenInstanceDetails] = useState>({}); + useEffect(() => { + if (focusedInstanceId === null) { + return; + } + setOpenInstanceDetails((existing) => + existing[focusedInstanceId] === true ? existing : { ...existing, [focusedInstanceId]: true }, + ); + }, [focusedInstanceId]); const { pendingRateLimitResetCredit, isConsumingRateLimitResetCredit, diff --git a/apps/web/src/components/settings/settingsNavigation.ts b/apps/web/src/components/settings/settingsNavigation.ts index 9c6ab149a..93f15550e 100644 --- a/apps/web/src/components/settings/settingsNavigation.ts +++ b/apps/web/src/components/settings/settingsNavigation.ts @@ -64,6 +64,22 @@ export function settingsSectionLabelForPath(pathname: string): string | null { return SETTINGS_NAV_ITEMS.find((item) => item.to === pathname)?.label ?? null; } +/** + * `?instance=` on the providers page names the card to open on arrival. It is + * how a surface elsewhere in the app hands a half-finished provider sign-in + * over to the settings panel, which owns the interactive terminal. + */ +export interface ProviderSettingsSearch { + readonly instance?: string; +} + +export function parseProviderSettingsSearch( + search: Record, +): ProviderSettingsSearch { + const instance = search["instance"]; + return typeof instance === "string" && instance.length > 0 ? { instance } : {}; +} + /** * Resolves where the settings `beforeLoad` guard should redirect, or null to * render the requested path. Mobile viewports render a full-page section diff --git a/apps/web/src/components/settings/useProviderConnectFlow.ts b/apps/web/src/components/settings/useProviderConnectFlow.ts new file mode 100644 index 000000000..bdd56999a --- /dev/null +++ b/apps/web/src/components/settings/useProviderConnectFlow.ts @@ -0,0 +1,253 @@ +/** + * The React side of the server-run provider sign-in. + * + * `ProviderAuthSessions` runs the provider's own login command in an ephemeral + * server-side PTY and streams command/output/status back over + * `providerAuth.subscribe`. This hook owns that subscription, folds the events + * into `ProviderConnectFlowState`, and decides when a run has stalled long + * enough to deserve the interactive terminal. + * + * It exists so every "Sign in" in the app drives the same flow: the settings + * panel, the first-run setup card, and the composer notices all mount this and + * differ only in how much of the state they draw. + * + * @module useProviderConnectFlow + */ +import type { + ProviderAuthEvent, + ProviderAuthFlow, + ProviderInstanceId, +} from "@threadlines/contracts"; +import { + useCallback, + useEffect, + useEffectEvent, + useMemo, + useRef, + useState, + type RefObject, +} from "react"; + +import { getPrimaryEnvironmentConnection } from "../../environments/runtime"; +import { + applyProviderAuthEvent, + initialProviderConnectFlowState, + isProviderConnectFlowActive, + shouldAutoExpandTerminal, + type ProviderConnectFlowState, +} from "./providerConnectFlow.logic"; + +type ProviderAuthClient = ReturnType< + typeof getPrimaryEnvironmentConnection +>["client"]["providerAuth"]; + +/** + * The primary environment throws before the app has one (a hosted browser tab + * that has not paired yet, or a test harness that mounts a surface on its + * own). Sign-in is not available there, and a surface must not crash for + * asking. + */ +function readProviderAuthClient(): ProviderAuthClient | null { + try { + return getPrimaryEnvironmentConnection().client.providerAuth; + } catch { + return null; + } +} + +/** Default PTY geometry for a flow started from a surface with no terminal. */ +export const PROVIDER_CONNECT_TERMINAL_COLS = 100; +export const PROVIDER_CONNECT_TERMINAL_ROWS = 20; +const AUTO_EXPAND_TICK_MS = 1_000; +/** Cap on replayed scrollback held for a terminal that mounts mid-flow. */ +const OUTPUT_BUFFER_CHARS = 64_000; + +export interface ProviderConnectFlowController { + readonly state: ProviderConnectFlowState; + /** True between the start click and the server's first status event. */ + readonly isStarting: boolean; + /** True while the server reports the flow starting or running. */ + readonly isActive: boolean; + /** + * True once this consumer has seen a run of its own begin. Attaching to a + * finished session replays its terminal status, so surfaces that only want + * to report runs the user just triggered gate on this rather than on + * `state.status`. + */ + readonly hasRun: boolean; + /** + * True once a still-running flow has passed the auto-expand threshold, or as + * soon as one fails: both mean the raw transcript is now the useful thing. + */ + readonly needsTerminal: boolean; + /** Why the start RPC itself failed, as opposed to the login command failing. */ + readonly startError: string | null; + readonly outputBufferRef: RefObject; + readonly terminalWriteRef: RefObject<((data: string) => void) | null>; + readonly start: () => void; + /** Cancels the server session and clears everything this hook is showing. */ + readonly reset: () => void; +} + +export function useProviderConnectFlow(input: { + readonly instanceId: ProviderInstanceId | null; + readonly flow: ProviderAuthFlow; + /** Fired once per run that ends in `succeeded`. */ + readonly onSucceeded?: (() => void) | undefined; + /** Fired when the start RPC rejects, for surfaces that toast instead of inlining. */ + readonly onStartError?: ((error: unknown) => void) | undefined; +}): ProviderConnectFlowController { + const { flow, instanceId, onStartError, onSucceeded } = input; + const [state, setState] = useState(initialProviderConnectFlowState); + const [isStarting, setIsStarting] = useState(false); + const [startError, setStartError] = useState(null); + const [runningForMs, setRunningForMs] = useState(0); + const [hasRun, setHasRun] = useState(false); + // Whether the success we are about to see belongs to a run we watched begin. + // Without it, attaching to a session that already succeeded would replay that + // status and fire `onSucceeded` for something the user did minutes ago. + const runObservedRef = useRef(false); + const outputBufferRef = useRef(""); + const terminalWriteRef = useRef<((data: string) => void) | null>(null); + // An instance has one auth session but can back two panels (sign-in and + // token setup). Sessions announce their flow in the "command" event; a + // consumer ignores sessions that belong to the other flow. + const sessionFlowRef = useRef(null); + + const handleEvent = useEffectEvent((event: ProviderAuthEvent) => { + if (event.type === "command") { + sessionFlowRef.current = event.flow; + if (event.flow !== flow) { + outputBufferRef.current = ""; + setState(initialProviderConnectFlowState); + return; + } + } else if (sessionFlowRef.current !== flow) { + // The server announces a session's command (and flow) before any output + // or status, so anything arriving unclaimed belongs to the other flow. + return; + } + if (event.type === "output") { + outputBufferRef.current = `${outputBufferRef.current}${event.data}`.slice( + -OUTPUT_BUFFER_CHARS, + ); + terminalWriteRef.current?.(event.data); + } + if (event.type === "status") { + if (event.status === "starting" || event.status === "running") { + runObservedRef.current = true; + setHasRun(true); + } else if (event.status === "succeeded") { + if (runObservedRef.current) { + runObservedRef.current = false; + onSucceeded?.(); + } + } else if (event.status === "failed") { + runObservedRef.current = false; + } + } + setState((previous) => applyProviderAuthEvent(previous, event)); + }); + + // Subscribed for the consumer's whole lifetime, not just after a click: the + // server replays the command, buffered output, and status on attach, so a + // flow started on another surface (or before a remount) lands here too. That + // replay is also what keeps two surfaces from racing a second sign-in — both + // see the same live session and disable their action. + useEffect(() => { + if (instanceId === null) { + return; + } + const providerAuth = readProviderAuthClient(); + if (providerAuth === null) { + return; + } + let cancelled = false; + const unsubscribe = providerAuth.subscribe({ instanceId }, (event) => { + if (cancelled) return; + handleEvent(event); + }); + return () => { + cancelled = true; + unsubscribe(); + }; + }, [instanceId]); + + const isActive = isProviderConnectFlowActive(state.status); + + useEffect(() => { + if (!isActive) { + setRunningForMs(0); + return; + } + const startedAt = Date.now(); + const timer = window.setInterval( + () => setRunningForMs(Date.now() - startedAt), + AUTO_EXPAND_TICK_MS, + ); + return () => window.clearInterval(timer); + }, [isActive]); + + const start = useCallback(() => { + const providerAuth = readProviderAuthClient(); + if (instanceId === null || providerAuth === null) { + return; + } + outputBufferRef.current = ""; + sessionFlowRef.current = null; + setState(initialProviderConnectFlowState); + setStartError(null); + setHasRun(true); + setIsStarting(true); + void providerAuth + .start({ + instanceId, + flow, + cols: PROVIDER_CONNECT_TERMINAL_COLS, + rows: PROVIDER_CONNECT_TERMINAL_ROWS, + }) + .catch((error: unknown) => { + setStartError( + error instanceof Error ? error.message : "The sign-in command could not be started.", + ); + onStartError?.(error); + }) + .finally(() => { + setIsStarting(false); + }); + }, [flow, instanceId, onStartError]); + + const reset = useCallback(() => { + if (instanceId !== null) { + void readProviderAuthClient() + ?.stop({ instanceId }) + .catch(() => {}); + } + outputBufferRef.current = ""; + sessionFlowRef.current = null; + runObservedRef.current = false; + setState(initialProviderConnectFlowState); + setStartError(null); + setHasRun(false); + }, [instanceId]); + + const needsTerminal = shouldAutoExpandTerminal({ status: state.status, runningForMs }); + + // Stable identity: consumers feed this straight into notice `useMemo`s that + // sit on the composer's render path. + return useMemo( + () => ({ + state, + isStarting, + isActive, + hasRun, + needsTerminal, + startError, + outputBufferRef, + terminalWriteRef, + start, + reset, + }), + [hasRun, isActive, isStarting, needsTerminal, reset, start, startError, state], + ); +} diff --git a/apps/web/src/lib/linkifiedText.test.tsx b/apps/web/src/lib/linkifiedText.test.tsx new file mode 100644 index 000000000..9a8bc9586 --- /dev/null +++ b/apps/web/src/lib/linkifiedText.test.tsx @@ -0,0 +1,27 @@ +import { renderToStaticMarkup } from "react-dom/server"; +import { describe, expect, it } from "vite-plus/test"; + +import { LinkifiedText } from "./linkifiedText"; + +describe("LinkifiedText", () => { + it("turns a URL in provider detail text into a new-tab link and leaves the sentence intact", () => { + const markup = renderToStaticMarkup( + , + ); + + expect(markup).toContain('href="https://claude.ai/download"'); + expect(markup).toContain('target="_blank"'); + expect(markup).toContain('rel="noopener noreferrer"'); + // The trailing comma belongs to the sentence, not the address. + expect(markup).not.toContain("https://claude.ai/download,""); + expect(markup.replace(/<[^>]*>/gu, "")).toBe( + "CLI not detected. Install it from https://claude.ai/download, then sign in.", + ); + }); + + it("renders text without a URL as plain text", () => { + expect(renderToStaticMarkup()).toBe( + "CLI not detected on PATH.", + ); + }); +}); diff --git a/apps/web/src/lib/linkifiedText.tsx b/apps/web/src/lib/linkifiedText.tsx new file mode 100644 index 000000000..60ea3aaef --- /dev/null +++ b/apps/web/src/lib/linkifiedText.tsx @@ -0,0 +1,97 @@ +/** + * Plain text with its http(s) URLs rendered as links. + * + * Provider status details are written by the server as prose, and some of + * them name an install page ("Install Claude Code from https://..."). Rendered + * as a bare string that address is dead text the user has to retype, so the + * surfaces that show provider detail run it through here instead. + * + * Deliberately not a Markdown renderer: this text is not authored as Markdown + * and the only thing worth promoting in it is a URL. + * + * @module linkifiedText + */ +import { Fragment, type ReactNode } from "react"; + +import { cn } from "./utils"; + +/** + * Stops at whitespace and at the characters that normally surround a URL in a + * sentence rather than belonging to it, so "(see https://x.dev/install)" + * does not swallow the closing bracket. + */ +const URL_PATTERN = /https?:\/\/[^\s<>"'`)\]}]+/gu; +/** Sentence punctuation that trails a URL far more often than it ends one. */ +const TRAILING_PUNCTUATION = /[.,;:!?]+$/u; + +export interface LinkifiedTextSegment { + readonly kind: "text" | "link"; + readonly value: string; + /** Offset in the source text. Unique per segment, so it doubles as a key. */ + readonly start: number; +} + +/** + * Splits text into alternating plain and link runs. Concatenating every + * segment's value reproduces the input exactly, so nothing is dropped or + * reordered by making a URL clickable. + */ +export function splitTextIntoLinkSegments(text: string): ReadonlyArray { + const segments: LinkifiedTextSegment[] = []; + let cursor = 0; + + for (const match of text.matchAll(URL_PATTERN)) { + const matchIndex = match.index; + const href = match[0].replace(TRAILING_PUNCTUATION, ""); + if (href.length === 0) { + continue; + } + if (matchIndex > cursor) { + segments.push({ kind: "text", value: text.slice(cursor, matchIndex), start: cursor }); + } + segments.push({ kind: "link", value: href, start: matchIndex }); + cursor = matchIndex + href.length; + } + + if (cursor < text.length) { + segments.push({ kind: "text", value: text.slice(cursor), start: cursor }); + } + return segments; +} + +/** + * Renders `text` with its URLs as new-tab anchors. Text without a URL renders + * as a plain string, so callers can use this anywhere a string used to sit. + */ +export function LinkifiedText({ + text, + className, +}: { + readonly text: string; + readonly className?: string; +}): ReactNode { + const segments = splitTextIntoLinkSegments(text); + if (!segments.some((segment) => segment.kind === "link")) { + return text; + } + + return ( + <> + {segments.map((segment) => + segment.kind === "link" ? ( + + {segment.value} + + ) : ( + {segment.value} + ), + )} + + ); +} diff --git a/apps/web/src/routes/settings.providers.tsx b/apps/web/src/routes/settings.providers.tsx index a7a86c2b5..4bbf77dfc 100644 --- a/apps/web/src/routes/settings.providers.tsx +++ b/apps/web/src/routes/settings.providers.tsx @@ -1,11 +1,14 @@ import { createFileRoute } from "@tanstack/react-router"; import { ProviderSettingsPanel } from "../components/settings/SettingsPanels"; +import { parseProviderSettingsSearch } from "../components/settings/settingsNavigation"; function SettingsProvidersRoute() { - return ; + const { instance } = Route.useSearch(); + return ; } export const Route = createFileRoute("/settings/providers")({ + validateSearch: (search) => parseProviderSettingsSearch(search), component: SettingsProvidersRoute, }); diff --git a/apps/web/test/wsRpcHarness.ts b/apps/web/test/wsRpcHarness.ts index 50d173797..606788dd7 100644 --- a/apps/web/test/wsRpcHarness.ts +++ b/apps/web/test/wsRpcHarness.ts @@ -34,6 +34,7 @@ const STREAM_METHODS = new Set([ WS_METHODS.subscribeTerminalEvents, WS_METHODS.subscribeServerConfig, WS_METHODS.subscribeServerLifecycle, + WS_METHODS.providerAuthSubscribe, ]); const ALL_RPC_METHODS = Array.from(WsRpcGroup.requests.keys()); From 54e2fdcf9923ad47b92e10783253b2544e86a948 Mon Sep 17 00:00:00 2001 From: badcuban <108198679+badcuban@users.noreply.github.com> Date: Fri, 7 Aug 2026 03:39:08 -0400 Subject: [PATCH 2/3] Assert the linkifier round-trip on segments instead of stripping tags CodeQL flagged the test's tag-stripping regex as incomplete multi character sanitization. It was never sanitizing, just recovering text content from static markup, but the pure segment splitter's documented invariant (concatenated segments reproduce the input) states the same property directly without the alert-bait pattern. --- apps/web/src/lib/linkifiedText.test.tsx | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/apps/web/src/lib/linkifiedText.test.tsx b/apps/web/src/lib/linkifiedText.test.tsx index 9a8bc9586..e295b1971 100644 --- a/apps/web/src/lib/linkifiedText.test.tsx +++ b/apps/web/src/lib/linkifiedText.test.tsx @@ -1,22 +1,25 @@ import { renderToStaticMarkup } from "react-dom/server"; import { describe, expect, it } from "vite-plus/test"; -import { LinkifiedText } from "./linkifiedText"; +import { LinkifiedText, splitTextIntoLinkSegments } from "./linkifiedText"; describe("LinkifiedText", () => { it("turns a URL in provider detail text into a new-tab link and leaves the sentence intact", () => { - const markup = renderToStaticMarkup( - , - ); + const text = "CLI not detected. Install it from https://claude.ai/download, then sign in."; + const markup = renderToStaticMarkup(); expect(markup).toContain('href="https://claude.ai/download"'); expect(markup).toContain('target="_blank"'); expect(markup).toContain('rel="noopener noreferrer"'); // The trailing comma belongs to the sentence, not the address. expect(markup).not.toContain("https://claude.ai/download,""); - expect(markup.replace(/<[^>]*>/gu, "")).toBe( - "CLI not detected. Install it from https://claude.ai/download, then sign in.", - ); + // Concatenating the segments reproduces the input byte for byte: making + // the URL clickable drops and reorders nothing. + const segments = splitTextIntoLinkSegments(text); + expect(segments.map((segment) => segment.value).join("")).toBe(text); + expect(segments.filter((segment) => segment.kind === "link").map((s) => s.value)).toEqual([ + "https://claude.ai/download", + ]); }); it("renders text without a URL as plain text", () => { From f98e777d8cedba2fa7574806207f9625ccb6c1cf Mon Sep 17 00:00:00 2001 From: badcuban <108198679+badcuban@users.noreply.github.com> Date: Fri, 7 Aug 2026 03:59:23 -0400 Subject: [PATCH 3/3] Open the hand-off terminal as soon as the flow is active Screenshot verification caught the settings hand-off restarting the stall timer: a user who already waited out the threshold in chat arrived at settings and waited it out again before the terminal appeared. The ?instance= deep link now threads through the provider card to the login flow, which opens its terminal immediately while the session is active. --- .../components/settings/ProviderConnectFlow.tsx | 17 ++++++++++++++--- .../settings/ProviderInstanceCard.tsx | 10 ++++++++++ .../src/components/settings/SettingsPanels.tsx | 1 + 3 files changed, 25 insertions(+), 3 deletions(-) diff --git a/apps/web/src/components/settings/ProviderConnectFlow.tsx b/apps/web/src/components/settings/ProviderConnectFlow.tsx index ccef68974..f612d3988 100644 --- a/apps/web/src/components/settings/ProviderConnectFlow.tsx +++ b/apps/web/src/components/settings/ProviderConnectFlow.tsx @@ -11,7 +11,10 @@ import { cn } from "../../lib/utils"; import { Button } from "../ui/button"; import { stackedThreadToast, toastManager } from "../ui/toast"; import { createXtermSurface } from "../terminal/xtermSurface"; -import { providerConnectStatusLine } from "./providerConnectFlow.logic"; +import { + isProviderConnectFlowActive, + providerConnectStatusLine, +} from "./providerConnectFlow.logic"; import { useProviderConnectFlow } from "./useProviderConnectFlow"; interface ProviderConnectTerminalProps { @@ -107,6 +110,13 @@ export interface ProviderConnectFlowProps { * a healthy status ("Sign in again"). */ readonly buttonVariant?: "default" | "outline" | "ghost"; + /** + * Open the terminal as soon as the flow is active instead of waiting out + * the stall threshold. Set when the user arrived via a sign-in hand-off + * (`?instance=`): another surface already waited the threshold out, and + * making them wait it out twice is the bug the hand-off exists to fix. + */ + readonly autoShowTerminal?: boolean; } /** @@ -126,6 +136,7 @@ export function ProviderConnectFlow({ description, statusRow, buttonVariant = "default", + autoShowTerminal = false, }: ProviderConnectFlowProps) { const [showFallback, setShowFallback] = useState(false); const [showTerminal, setShowTerminal] = useState(false); @@ -171,10 +182,10 @@ export function ProviderConnectFlow({ setShowTerminal(false); return; } - if (needsTerminal) { + if (needsTerminal || (autoShowTerminal && isProviderConnectFlowActive(state.status))) { setShowTerminal(true); } - }, [needsTerminal, state.status]); + }, [autoShowTerminal, needsTerminal, state.status]); const startFlow = () => { setShowTerminal(false); diff --git a/apps/web/src/components/settings/ProviderInstanceCard.tsx b/apps/web/src/components/settings/ProviderInstanceCard.tsx index fc841918f..9329adf97 100644 --- a/apps/web/src/components/settings/ProviderInstanceCard.tsx +++ b/apps/web/src/components/settings/ProviderInstanceCard.tsx @@ -728,6 +728,7 @@ function ProviderAccountSignInSection(props: { environment: ReadonlyArray, ) => void; readonly claudeSetupTokenCommand?: string | undefined; + readonly signInHandoffActive?: boolean; }) { const authBadge = providerAuthBadge(props.liveProvider?.auth); const needsSignIn = authBadge.variant === "warning"; @@ -757,6 +758,7 @@ function ProviderAccountSignInSection(props: { displayName={props.displayName} actionLabel={needsSignIn ? "Reconnect" : "Sign in again"} command={props.terminalLoginCommand} + autoShowTerminal={props.signInHandoffActive ?? false} buttonVariant={needsSignIn ? "default" : "ghost"} description={isClaude ? "Signing in covers both chat and usage." : undefined} statusRow={ @@ -1110,6 +1112,12 @@ interface ProviderInstanceCardProps { readonly driverOption: DriverOption | undefined; readonly liveProvider: ServerProvider | undefined; readonly isExpanded: boolean; + /** + * True when the user arrived via a sign-in hand-off (`?instance=`): the + * login flow's terminal opens as soon as it is active instead of waiting + * out the stall threshold a second time. + */ + readonly signInHandoffActive?: boolean; readonly onExpandedChange: (open: boolean) => void; readonly onUpdate: (nextInstance: ProviderInstanceConfig) => void; /** @@ -1171,6 +1179,7 @@ export function ProviderInstanceCard({ driverOption, liveProvider, isExpanded, + signInHandoffActive = false, onExpandedChange, onUpdate, onDelete, @@ -1705,6 +1714,7 @@ export function ProviderInstanceCard({ idPrefix={`provider-instance-${instanceId}`} environment={instance.environment ?? []} onEnvironmentChange={updateEnvironment} + signInHandoffActive={signInHandoffActive} {...(driverKind === CLAUDE_DRIVER_KIND ? { claudeSetupTokenCommand } : {})} /> diff --git a/apps/web/src/components/settings/SettingsPanels.tsx b/apps/web/src/components/settings/SettingsPanels.tsx index 83a0ab6f4..f3ba3f524 100644 --- a/apps/web/src/components/settings/SettingsPanels.tsx +++ b/apps/web/src/components/settings/SettingsPanels.tsx @@ -1440,6 +1440,7 @@ export function ProviderSettingsPanel({ driverOption={driverOption} liveProvider={liveProvider} isExpanded={openInstanceDetails[row.instanceId] ?? false} + signInHandoffActive={row.instanceId === focusedInstanceId} onExpandedChange={(open) => setOpenInstanceDetails((existing) => ({ ...existing,