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..f612d3988 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"; @@ -16,17 +12,10 @@ 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 { useProviderConnectFlow } from "./useProviderConnectFlow"; interface ProviderConnectTerminalProps { readonly instanceId: ProviderInstanceId; @@ -121,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; } /** @@ -140,18 +136,10 @@ export function ProviderConnectFlow({ description, statusRow, buttonVariant = "default", + autoShowTerminal = false, }: 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 +152,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 +182,20 @@ export function ProviderConnectFlow({ setShowTerminal(false); return; } - if (shouldAutoExpandTerminal({ status: state.status, runningForMs })) { + if (needsTerminal || (autoShowTerminal && isProviderConnectFlowActive(state.status))) { setShowTerminal(true); } - }, [runningForMs, state.status]); + }, [autoShowTerminal, 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..9329adf97 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, @@ -727,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"; @@ -756,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={ @@ -1109,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; /** @@ -1170,6 +1179,7 @@ export function ProviderInstanceCard({ driverOption, liveProvider, isExpanded, + signInHandoffActive = false, onExpandedChange, onUpdate, onDelete, @@ -1468,7 +1478,11 @@ export function ProviderInstanceCard({ )} - {summary.detail ? - {summary.detail} : null} + {summary.detail ? ( + + - + + ) : null}

); @@ -1700,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 ba622f208..f3ba3f524 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, @@ -1423,6 +1440,7 @@ export function ProviderSettingsPanel() { driverOption={driverOption} liveProvider={liveProvider} isExpanded={openInstanceDetails[row.instanceId] ?? false} + signInHandoffActive={row.instanceId === focusedInstanceId} onExpandedChange={(open) => setOpenInstanceDetails((existing) => ({ ...existing, 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..e295b1971 --- /dev/null +++ b/apps/web/src/lib/linkifiedText.test.tsx @@ -0,0 +1,30 @@ +import { renderToStaticMarkup } from "react-dom/server"; +import { describe, expect, it } from "vite-plus/test"; + +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 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,""); + // 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", () => { + 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());