From 2692f42bcb85fb101769f02eeb11f880757ad28f Mon Sep 17 00:00:00 2001 From: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com> Date: Sun, 28 Jun 2026 15:00:26 -0700 Subject: [PATCH] Refine the connect modal's API-key credential UX - Merged credential field: the placement's lead + prefix is a fixed, non-editable addon inside the field, so it reads as the header value being built ("Authorization: Bearer ") and the separate preview line is dropped. autoComplete/ignore attrs stop the browser and password managers from offering to GENERATE a password here; the app's own 1Password picker still covers filling an existing secret. - Attached tabs: the auth-method tabs join the panel as one card (full-width rounded-top header, hairline divider) instead of a pill bar floating above a separate card. - Prefix trailing-space warning (amber): a non-empty prefix with no trailing space is sent joined to the value ("Bearer" + token -> "Bearertoken"). Advisory, not blocking. --- .../connect-modal-credential-ux.test.ts | 86 ++++++++++++++++ .../src/components/add-account-modal.tsx | 99 +++++++++++++++---- .../react/src/components/placement-editor.tsx | 24 ++++- 3 files changed, 189 insertions(+), 20 deletions(-) create mode 100644 e2e/selfhost/connect-modal-credential-ux.test.ts diff --git a/e2e/selfhost/connect-modal-credential-ux.test.ts b/e2e/selfhost/connect-modal-credential-ux.test.ts new file mode 100644 index 000000000..0dd631f74 --- /dev/null +++ b/e2e/selfhost/connect-modal-credential-ux.test.ts @@ -0,0 +1,86 @@ +// Selfhost-only (browser): the connect modal's API-key credential UX. +// - the credential field MERGES the placement's lead + prefix as an affix, so +// it reads as the header value being built ("Authorization: Bearer ▏token"); +// - the "Add authentication method" editor offers placement presets; +// - a prefix with no trailing space (sent joined to the value) warns, and the +// warning clears once the space is restored. +// Video is the artifact. +import { randomBytes } from "node:crypto"; + +import { Effect } from "effect"; +import { composePluginApi } from "@executor-js/api/server"; +import { openApiHttpPlugin } from "@executor-js/plugin-openapi/api"; +import { IntegrationSlug } from "@executor-js/sdk/shared"; + +import { scenario } from "../src/scenario"; +import { Api, Browser, Target } from "../src/services"; + +const api = composePluginApi([openApiHttpPlugin()] as const); + +const bearerSpec = (): string => + JSON.stringify({ + openapi: "3.0.3", + info: { title: "Bearer Fixture", version: "1.0.0" }, + servers: [{ url: "https://api.bearerfix.test" }], + security: [{ bearerAuth: [] }], + components: { securitySchemes: { bearerAuth: { type: "http", scheme: "bearer" } } }, + paths: { + "/ping": { get: { operationId: "ping", responses: { "200": { description: "ok" } } } }, + }, + }); + +scenario( + "Connect modal · API key credential UX: merged affix, add-method, prefix warning", + {}, + Effect.scoped( + Effect.gen(function* () { + const target = yield* Target; + const { client: makeApiClient } = yield* Api; + const browser = yield* Browser; + const identity = yield* target.newIdentity(); + const apiClient = yield* makeApiClient(api, identity); + const slug = `connect_ux_${randomBytes(4).toString("hex")}`; + + yield* Effect.ensuring( + Effect.gen(function* () { + yield* apiClient.openapi.addSpec({ + payload: { spec: { kind: "blob", value: bearerSpec() }, slug }, + }); + + yield* browser.session(identity, async ({ page, step }) => { + await step("Open the connect modal", async () => { + await page.goto(`/integrations/${slug}?addAccount=1`, { waitUntil: "networkidle" }); + await page.getByRole("heading", { name: /Add connection/ }).waitFor(); + }); + + await step("The credential field merges the placement prefix", async () => { + // The placement's lead + prefix renders as a non-editable affix + // inside the field, so there is no separate preview line. + await page.getByText("Authorization: Bearer").first().waitFor(); + }); + + await step("The add-method editor offers placement presets", async () => { + await page.getByRole("button", { name: "Add authentication method" }).click(); + await page.getByRole("heading", { name: "Add authentication method" }).waitFor(); + await page.getByRole("button", { name: "Bearer header" }).waitFor(); + await page.getByRole("button", { name: "API key query" }).waitFor(); + }); + + await step("A prefix with no trailing space warns", async () => { + await page.getByPlaceholder("Bearer ").first().fill("Bearer"); + await page.getByText("Prefix has no trailing space").waitFor(); + }); + + await step("Restoring the trailing space clears the warning", async () => { + await page.getByPlaceholder("Bearer ").first().fill("Bearer "); + await page.getByText("Prefix has no trailing space").waitFor({ state: "detached" }); + }); + }); + }), + apiClient.openapi + .removeSpec({ params: { slug: IntegrationSlug.make(slug) } }) + .pipe(Effect.ignore), + ); + }), + ), +); diff --git a/packages/react/src/components/add-account-modal.tsx b/packages/react/src/components/add-account-modal.tsx index b4e2f2b8c..677f7ed14 100644 --- a/packages/react/src/components/add-account-modal.tsx +++ b/packages/react/src/components/add-account-modal.tsx @@ -154,6 +154,10 @@ function PasteCredentialInputs(props: { /** Force the per-input label even for a single input (env vars name their * field rather than relying on a generic "value / token" placeholder). */ readonly showLabels?: boolean; + /** Single-input only: the placement's lead + prefix (e.g. "Authorization: + * Bearer "), rendered as a non-editable affix INSIDE the field so the input + * reads as the header value being built. Replaces the separate preview line. */ + readonly affix?: string | null; readonly values: Readonly>; readonly onChange: (values: Record) => void; }) { @@ -173,7 +177,11 @@ function PasteCredentialInputs(props: { ) => @@ -201,20 +209,55 @@ function PasteCredentialInputs(props: { {labelled && ( )} - ) => - props.onChange({ - ...props.values, - [input.variable]: e.target.value, - }) - } - className="font-mono" - data-ph-block - /> + {props.affix ? ( + // Merged field: the placement's lead + prefix is a FIXED addon (its + // own muted segment, divider, non-selectable), and the user types + // only the token after it — the field reads as the header value being + // built. Only the border reacts to focus (no full-field glow). The + // autoComplete/ignore attrs stop the browser and password managers + // from offering to GENERATE a password here; the app's own 1Password + // picker covers filling an existing secret. +
+ + {props.affix} + + {/* oxlint-disable-next-line react/forbid-elements */} + ) => + props.onChange({ ...props.values, [input.variable]: e.target.value }) + } + className="min-w-0 flex-1 bg-transparent px-3 outline-none placeholder:text-muted-foreground/60" + data-ph-block + /> +
+ ) : ( + ) => + props.onChange({ + ...props.values, + [input.variable]: e.target.value, + }) + } + className="font-mono" + data-ph-block + /> + )} ))} @@ -292,6 +335,8 @@ function CredentialValueFields(props: { readonly inputs: readonly CredentialInput[]; readonly singleInput: boolean; readonly showLabels?: boolean; + /** Single-input only: placement lead + prefix to merge into the field. */ + readonly affix?: string | null; /** Allow an external provider (1Password) origin. Off for env methods: the * wire's external `from` is keyed under the canonical `token` variable, but * an env credential must be keyed under its env var name, so a 1Password @@ -346,6 +391,7 @@ function CredentialValueFields(props: { inputs={props.inputs} singleInput={props.singleInput} showLabels={props.showLabels} + affix={props.affix} values={props.values} onChange={props.onValuesChange} /> @@ -882,6 +928,20 @@ function AddAccountModalView(props: AddAccountModalProps) { method != null && method.placements.length > 0 && method.placements.every((p) => p.carrier === "env"); + // Single-input header/query methods: the placement's lead + prefix (e.g. + // "Authorization: Bearer ") merges INTO the credential field as a non-editable + // affix, so the input reads as the header value being built and the separate + // preview line is dropped. Env and multi-input methods keep the plain field. + const singleCredentialAffix = useMemo(() => { + if (!method || !singleInput || isEnvMethod) return null; + const placement = method.placements.find((p) => p.carrier !== "env"); + if (!placement) return null; + const lead = + placement.carrier === "header" + ? `${placement.name || "Authorization"}: ` + : `?${placement.name || "api_key"}=`; + return `${lead}${placement.prefix ?? ""}`; + }, [method, singleInput, isEnvMethod]); // DCR-capable: the integration advertises dynamic registration (MCP oauth2), // OR carries a discovery URL we can probe at connect time. When DCR-capable // and not yet fallen back, we skip the app picker entirely (Option A). @@ -1394,9 +1454,9 @@ function AddAccountModalView(props: AddAccountModalProps) { - +
{allMethods.map((m: AuthMethod) => (
- {method?.placements && !isEnvMethod && singleInput + {method?.placements && !isEnvMethod && singleInput && !singleCredentialAffix ? (() => { const shown = method.placements.filter((p) => p.carrier !== "env"); return shown.length > 0 ? ( @@ -1573,6 +1633,7 @@ function AddAccountModalView(props: AddAccountModalProps) { inputs={credentialInputs} singleInput={singleInput} showLabels={isEnvMethod} + affix={singleCredentialAffix} allowExternalProvider={!isEnvMethod} values={values} onValuesChange={setValues} diff --git a/packages/react/src/components/placement-editor.tsx b/packages/react/src/components/placement-editor.tsx index 50cbcae47..3e539e6dd 100644 --- a/packages/react/src/components/placement-editor.tsx +++ b/packages/react/src/components/placement-editor.tsx @@ -1,4 +1,4 @@ -import { PlusIcon, XIcon } from "lucide-react"; +import { PlusIcon, TriangleAlertIcon, XIcon } from "lucide-react"; import { emptyPlacement, @@ -47,6 +47,10 @@ export function PlacementEditor(props: { const remove = (index: number): void => onChange(placements.filter((_p: Placement, j: number) => j !== index)); + // A non-empty prefix with no trailing space is sent JOINED to the credential + // (e.g. "Bearer" + token -> "Bearertoken"). Almost always a mistake: warn. + const barePrefix = placements.some((p) => p.prefix.length > 0 && !p.prefix.endsWith(" ")); + return (
@@ -147,6 +151,24 @@ export function PlacementEditor(props: { ) : null}
))} + {barePrefix ? ( +
+ +
+

+ Prefix has no trailing space +

+

+ It is sent joined to the value (Bearer••••••). Most + APIs expect a space, like "Bearer ". +

+
+
+ ) : null} +