diff --git a/.pyrit_conf_example b/.pyrit_conf_example index 51488ee0d8..4fbad18e24 100644 --- a/.pyrit_conf_example +++ b/.pyrit_conf_example @@ -161,6 +161,21 @@ allow_custom_initializers: false # as scenario_presets_source. Defaults to ~/.pyrit/scenario_suites. # scenario_suites_source: https://account.blob.core.windows.net/container/scenario_suites +# Optional storage for saved targets, converters, scorers, and techniques, so they +# come back when the backend restarts. Each kind has its own folder under the source. +# This supports the same local directory and Azure Blob sources as +# scenario_presets_source. If any saved entry can't be restored, the backend stays +# not ready and lists the failures in its runtime status. Fix or delete the listed +# files, then apply the configuration or restart the backend. Defaults to +# ~/.pyrit/registry_recipes. +# +# ⚠️ WARNING: The backend rebuilds every saved entry in this source as an +# administrator, at startup and live reinitialization. Anyone who can write to this +# source can make the backend run a model's repository code or use its identity. +# Restrict write access to trusted administrators, and use a dedicated folder or +# blob prefix. +# registry_recipes_source: https://account.blob.core.windows.net/container/registry_recipes + # Upload Folder for API-Created Targets # ------------------------------------- # Folder that targets created through the REST API may upload local files from diff --git a/doc/getting_started/pyrit_conf.md b/doc/getting_started/pyrit_conf.md index bba64b45d4..af4d551a4f 100644 --- a/doc/getting_started/pyrit_conf.md +++ b/doc/getting_started/pyrit_conf.md @@ -194,6 +194,20 @@ Suites store ordered references to scenario presets and resolve each preset's cu A SAS query string may be included; otherwise, PyRIT uses `DefaultAzureCredential`. The default is `~/.pyrit/scenario_suites`. Local writes use the same hidden lock-file and atomic replacement behavior as scenario presets. Startup scans the configured source so malformed suite documents are reported immediately without preventing the backend from starting. +### `registry_recipes_source` + +Stores saved targets, converters, scorers, and techniques in a local directory or Azure Blob container, so they come back when the backend restarts. Targets created through `POST /api/targets` with `"persist": true` are saved here. An Azure URI may include a blob-name prefix: + +```yaml +registry_recipes_source: https://account.blob.core.windows.net/pyrit-storage/registry-recipes +``` + +Each kind has its own folder under the source (`targets/`, `converters/`, `scorers/`, `techniques/`), with one JSON file per saved entry named after the entry. A file holds the create request the entry was made with, and restore builds the entry through the API's create for its kind. A file never holds a credential: restore refuses a file that stores a value for a parameter its class marks as a credential, such as `api_key`. A saved target also can't use a key from this server: it signs in with Microsoft Entra ID (`"auth_mode": "identity"`), which uses this server's Azure identity for everyone using the app, or needs no key. Targets from the server's configuration that it uses, such as a round-robin's members, keep their own sign-in. So restore refuses a target file that would read an empty key from an environment variable or leave an address to this server's settings. The addresses each target type declares (such as `endpoint`, `container_url` for Blob storage, or an A2A agent card path) can't hold a user name, password, SAS token, fragment, path parameters (`;`, `&`, `=`), or a query other than `api-version`, and a path can't have a query at all. An address the server would otherwise fill in must be a full http(s) or ws(s) address. In identity mode, an OpenAI target file sets `"headers": "{}"` in its `params` so that the server's `OPENAI_ADDITIONAL_REQUEST_HEADERS` aren't added. The OpenAI SDK adds the headers in `OPENAI_CUSTOM_HEADERS` to every request, so while the server sets it, OpenAI targets can't be saved and saved ones aren't restored. The SDK's `OPENAI_ORG_ID` and `OPENAI_PROJECT_ID` still apply to every OpenAI target on this server. GitHub Copilot, Hugging Face, HackAPrompt, LiteLLM, HTTP, and Copilot WebSocket targets can't be saved yet. A SAS query string may be included in the source; otherwise, PyRIT uses `DefaultAzureCredential`. The default is `~/.pyrit/registry_recipes`. + +Restore can't tell who wrote a saved file, even one that names the user who saved it, so the backend rebuilds every entry in this source as an administrator, at startup and after a live apply. A file here can, for example, make the backend run a model's repository code, or use the server's identity on a storage container that the file names. Write access to this source is administrator authority on the backend: grant it only to principals you already trust as backend administrators, and use a dedicated directory or blob prefix that less-trusted users and services cannot write to. + +The backend restores saved entries at startup and after a live apply, once the configured initializers have registered theirs. If any saved entry can't be restored, the backend doesn't become ready (state `failed`, outcome `restore-failed`) and lists each failure with its reason in `GET /api/config/runtime`, so a scan can't quietly run without an entry it expects. Until then, only the configuration and status routes answer; the rest return 503. Fix or delete the listed files, or, when a folder couldn't be listed, check that the backend can read this source. Then apply the configuration, if `enable_live_reinitialization` is on, or restart the backend. Files whose names aren't valid entry names (up to 64 lowercase letters, digits, and underscores, starting with a letter) are skipped without blocking startup; administrators can see them under `restore_ignored` in `GET /api/config/runtime`. + ### `initialization_scripts` Local paths to custom Python scripts containing `PyRITInitializer` subclasses. Paths can be absolute or relative to the current working directory. diff --git a/doc/gui/0_gui.md b/doc/gui/0_gui.md index 85ce95fb5d..a429a8deac 100644 --- a/doc/gui/0_gui.md +++ b/doc/gui/0_gui.md @@ -209,8 +209,28 @@ Editing a preset preserves unchanged parameter values exactly, including commas and whitespace inside list entries, empty lists, duplicates, and their order. Editing a list field uses the comma-separated format shown in the editor. -Targets, converters, and techniques added only to the live registry must be -recreated after a backend restart or reinitialization. Use +**New Target** saves the target, so it comes back after the backend restarts or +is reinitialized. Give it a registry name of lowercase letters, digits, and +underscores that starts with a letter; the dialog suggests one from the target +type and model. Saved targets are kept in `registry_recipes_source` +(`~/.pyrit/registry_recipes` unless set in your `.pyrit_conf`). To remove a saved +target, delete its file (`targets/.json`) there, and the file of any saved +round robin that uses it, then restart the backend. If a startup initializer later +registers a target under a saved target's name, the saved one isn't restored and +the backend isn't ready until that file is renamed or removed and the backend +restarted. + +Saved targets can't hold credentials yet, so choose Identity-based or No key +authentication when the target supports it. A target that needs an API key or +another credential value, a type that can't be saved, or a round robin that uses +a Temporary target is created as **Temporary** instead, and the dialog says why. +A Temporary target is gone after the backend restarts or is reinitialized. + +The Targets table labels each target **Saved**, **From configuration** +(registered by an initializer at startup), or **Temporary**. + +Converters and techniques added only to the live registry must be recreated +after a backend restart or reinitialization. Use [initializers](../code/setup/0_setup.md) for repeatable component setup. Use **Auto-detect capabilities** to select targets and run live checks. Results diff --git a/frontend/e2e/config.spec.ts b/frontend/e2e/config.spec.ts index d72cef8dc4..b24f715c47 100644 --- a/frontend/e2e/config.spec.ts +++ b/frontend/e2e/config.spec.ts @@ -302,7 +302,7 @@ test.describe("Target Registry Page", () => { // Dialog should open await expect(page.getByText("Create New Target")).toBeVisible(); - await expect(page.getByText("Create Target")).toBeVisible(); + await expect(page.getByRole("button", { name: "Save Target" })).toBeVisible(); }); test("should update targets after a visible minute without a Refresh control", async ({ page }) => { @@ -406,12 +406,14 @@ test.describe("Create Target Dialog", () => { await expect(selectedDetails).toContainText("Supported authentication: API key"); }); - test("should create a target through the dialog", async ({ page }) => { + test("should save a target through the dialog", async ({ page }) => { let createdTarget: FlatTarget | null = null; + let createBody: Record | null = null; await page.route(/\/api\/targets/, async (route) => { if (route.request().method() === "POST") { const body = JSON.parse(route.request().postData() ?? "{}"); + createBody = body; createdTarget = { target_registry_name: "new-target-1", target_type: body.type, @@ -429,6 +431,27 @@ test.describe("Create Target Dialog", () => { await route.fulfill(mockTargetsList(items)); } }); + // Registered last, so it answers the type list instead of the route above. + await page.route(/\/api\/targets\/types(?:\?.*)?$/, async (route) => { + await route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify({ + items: [ + { + target_type: "OpenAIChatTarget", + parameters: [ + { name: "endpoint", type_name: "str", required: true, default: null }, + { name: "model_name", type_name: "str", required: true, default: null }, + ], + supported_auth_modes: ["api_key", "identity", "none"], + saveable_auth_modes: ["identity", "none"], + not_saveable_reason: null, + }, + ], + }), + }); + }); await goToTargets(page); @@ -453,11 +476,16 @@ test.describe("Create Target Dialog", () => { // Fill model name await dialog.getByPlaceholder("e.g. gpt-4o, my-deployment").fill("gpt-4o-test"); - // Click Create Target - await dialog.getByRole("button", { name: "Create Target" }).click(); + await dialog.getByRole("button", { name: "Save Target" }).click(); // Dialog should close and target should appear in the list await expect(page.getByText("Create New Target")).not.toBeVisible({ timeout: 5_000 }); + expect(createBody).toMatchObject({ + name: "openai_chat_gpt_4o_test", + type: "OpenAIChatTarget", + auth_mode: "identity", + persist: true, + }); await expect(page.getByText("gpt-4o-test", { exact: true })).toBeVisible({ timeout: 10_000 }); await expect(page.getByText("OpenAIChatTarget")).toBeVisible(); }); @@ -496,7 +524,7 @@ test.describe("Create Target Dialog", () => { await expect(page.getByText("Create New Target")).toBeVisible(); const dialog = page.locator('[role="dialog"]'); - const createBtn = dialog.getByRole("button", { name: "Create Target" }); + const createBtn = dialog.getByRole("button", { name: "Save Target" }); await expect(createBtn).toBeDisabled(); await selectTargetType( diff --git a/frontend/e2e/fixtures/manual_send_backend.py b/frontend/e2e/fixtures/manual_send_backend.py index 0c46ed2398..8be9b44941 100644 --- a/frontend/e2e/fixtures/manual_send_backend.py +++ b/frontend/e2e/fixtures/manual_send_backend.py @@ -4,6 +4,7 @@ """Offline backend for manual-send browser tests, with no real configuration or providers.""" import os +import tempfile from collections.abc import AsyncGenerator from contextlib import asynccontextmanager from unittest.mock import AsyncMock, patch @@ -27,19 +28,22 @@ async def _initialize_offline_async(self: ConfigurationLoader, *, raise_on_initi @asynccontextmanager async def _offline_lifespan_async(application: FastAPI) -> AsyncGenerator[None, None]: - config = ConfigurationLoader(memory_db_type="in_memory", env_files=[], env_akv_ref=[]) - with ( - patch.dict(os.environ, {"PYRIT_AUTH_MODE": "local", "PYRIT_DEV_MODE": "true"}, clear=True), - patch.object( - ConfigurationFileService, - "_read_source_async", - new=AsyncMock(return_value="memory_db_type: in_memory\nenv_files: []\ninitializers: []\n"), - ), - patch.object(ConfigurationLoader, "load_with_overrides", return_value=config), - patch.object(ConfigurationLoader, "initialize_pyrit_async", new=_initialize_offline_async), - ): - async with lifespan(application): - yield + with tempfile.TemporaryDirectory() as recipes: + config = ConfigurationLoader( + memory_db_type="in_memory", env_files=[], env_akv_ref=[], registry_recipes_source=recipes + ) + with ( + patch.dict(os.environ, {"PYRIT_AUTH_MODE": "local", "PYRIT_DEV_MODE": "true"}, clear=True), + patch.object( + ConfigurationFileService, + "_read_source_async", + new=AsyncMock(return_value="memory_db_type: in_memory\nenv_files: []\ninitializers: []\n"), + ), + patch.object(ConfigurationLoader, "load_with_overrides", return_value=config), + patch.object(ConfigurationLoader, "initialize_pyrit_async", new=_initialize_offline_async), + ): + async with lifespan(application): + yield app.router.lifespan_context = _offline_lifespan_async diff --git a/frontend/e2e/registry.spec.ts b/frontend/e2e/registry.spec.ts index ec7c5d4aef..06aab85e9b 100644 --- a/frontend/e2e/registry.spec.ts +++ b/frontend/e2e/registry.spec.ts @@ -142,6 +142,32 @@ test("creates and selects a runtime technique with the real backend @seeded", as expect(promptRequests).toEqual([]); }); +test("saves a target from the New Target dialog and still lists it as Saved after a reload @seeded", async ({ page, request }) => { + const name = `saved_chat_${Date.now()}`; + await page.goto("/registry/targets"); + await page.getByRole("button", { name: "New Target", exact: true }).click(); + const dialog = page.getByRole("dialog"); + const picker = dialog.getByRole("combobox", { name: "Target Type" }); + await expect(picker).toBeEnabled(); + await picker.click(); + await page.getByRole("option", { name: /Implementation: OpenAIChatTarget/ }).click(); + await dialog.getByRole("radio", { name: /^No key/ }).check(); + await dialog.getByPlaceholder("https://your-resource.openai.azure.com/").fill("http://127.0.0.1:9/v1"); + await dialog.getByPlaceholder("e.g. gpt-4o, my-deployment").fill("local-test"); + await dialog.getByRole("textbox", { name: /Registry Name/ }).fill(name); + await expect(dialog.getByText("This target can't be saved")).toHaveCount(0); + await dialog.getByRole("button", { name: "Save Target" }).click(); + await expect(dialog).toHaveCount(0); + + await expect(page.getByTestId(`target-row-${name}`).getByText("Saved", { exact: true })).toBeVisible(); + const saved = await request.get(`/api/targets/${name}`, { headers: compatibilityHeaders() }); + expect(saved.ok(), await saved.text()).toBe(true); + expect((await saved.json()).origin).toBe("saved"); + + await page.reload(); + await expect(page.getByTestId(`target-row-${name}`).getByText("Saved", { exact: true })).toBeVisible(); +}); + interface RegisteredConverter { converter_id: string; identifier: { diff --git a/frontend/playwright.config.ts b/frontend/playwright.config.ts index 23140b23a0..547d6b29e3 100644 --- a/frontend/playwright.config.ts +++ b/frontend/playwright.config.ts @@ -1,3 +1,7 @@ +import { mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import path from "node:path"; + import { defineConfig, devices } from "@playwright/test"; import type { UserPreferences } from "./src/types"; @@ -24,6 +28,28 @@ const VITE_SERVER_OPTIONS = const E2E_BACKEND_PORT = process.env.PYRIT_E2E_BACKEND_PORT ?? "18000"; const E2E_BACKEND_URL = `http://127.0.0.1:${E2E_BACKEND_PORT}`; +/** + * Copies the seeded backend's configuration into a fresh folder that also holds + * its saved targets, converters, scorers, and techniques, so a run never reads + * or writes the ones saved on the machine running it. + */ +function createSeededBackendConfigFile(): string { + const folder = mkdtempSync(path.join(tmpdir(), "pyrit-seeded-backend-")); + // The runner stops the backend before it exits, so nothing still uses the folder by then. + process.once("exit", () => rmSync(folder, { recursive: true, force: true })); + const configFile = path.join(folder, "browser_config.yaml"); + const config = readFileSync(new URL("../tests/end_to_end/browser_config.yaml", import.meta.url), "utf-8"); + const recipesSource = JSON.stringify(path.join(folder, "registry_recipes")); + writeFileSync(configFile, `${config}\nregistry_recipes_source: ${recipesSource}\n`); + return configFile; +} + +// Workers load this file too, but only the main process starts the backend. +const SEEDED_BACKEND_CONFIG_FILE = + SEEDED_MODE && process.env.TEST_WORKER_INDEX === undefined + ? createSeededBackendConfigFile() + : ""; + export default defineConfig({ testDir: "./e2e", testIgnore: ["**/analytics.spec.ts", "**/paired-outcomes.spec.ts", "**/scenario-hierarchy.spec.ts", "**/scenario-datasets.spec.ts", "**/scenario-techniques.spec.ts", "**/scan-launch.seeded.spec.ts"], @@ -113,7 +139,7 @@ export default defineConfig({ command: `cd .. && uv run --no-sync python -m pyrit.backend.pyrit_backend ` + `--auth-mode local --host 127.0.0.1 --port ${E2E_BACKEND_PORT} --log-level warning ` + - "--config-file tests/end_to_end/browser_config.yaml", + `--config-file "${SEEDED_BACKEND_CONFIG_FILE}"`, env: { PYRIT_DEV_MODE: "true" }, url: `${E2E_BACKEND_URL}/api/health`, reuseExistingServer: false, diff --git a/frontend/src/components/Config/CreateTargetDialog.test.tsx b/frontend/src/components/Config/CreateTargetDialog.test.tsx index 410ef5ece7..5c94679394 100644 --- a/frontend/src/components/Config/CreateTargetDialog.test.tsx +++ b/frontend/src/components/Config/CreateTargetDialog.test.tsx @@ -2,7 +2,7 @@ import { act, render, screen, waitFor, fireEvent, within } from "@testing-librar import userEvent from "@testing-library/user-event"; import { FluentProvider, webLightTheme } from "@fluentui/react-components"; import { makeTarget } from "@/test-utils/targetFixtures"; -import type { Parameter, TargetTypeListResponse } from "@/types"; +import type { Parameter, TargetTypeEntry, TargetTypeListResponse } from "@/types"; import CreateTargetDialog from "./CreateTargetDialog"; import { parseWeight, MAX_WEIGHT } from "./weightValidation"; import { targetsApi } from "@/services/api"; @@ -361,8 +361,16 @@ const TARGET_DISPLAY_NAMES: Record = { HTTPTarget: "HTTPTarget", TextTarget: "TextTarget", AzureBlobStorageTarget: "AzureBlobStorageTarget", + A2ATarget: "A2ATarget", }; +// The primary action reads Save Target, or Create as Temporary when the target can't be saved. +const SUBMIT_BUTTON_NAME = /^(Save Target|Create as Temporary|Creating\.\.\.)$/; + +function getSubmitButton(): HTMLElement { + return screen.getByRole("button", { name: SUBMIT_BUTTON_NAME }); +} + const TestWrapper: React.FC<{ children: React.ReactNode }> = ({ children, }) => {children}; @@ -542,7 +550,7 @@ describe("CreateTargetDialog", () => { await flushTargetTypesFetch(); expect(screen.getByText("Create New Target")).toBeInTheDocument(); - expect(screen.getByText("Create Target")).toBeInTheDocument(); + expect(getSubmitButton()).toBeInTheDocument(); expect(screen.getByText("Cancel")).toBeInTheDocument(); }); @@ -743,7 +751,7 @@ describe("CreateTargetDialog", () => { ); await flushTargetTypesFetch(); - const createButton = screen.getByText("Create Target"); + const createButton = getSubmitButton(); expect(createButton.closest("button")).toBeDisabled(); }); @@ -812,10 +820,12 @@ describe("CreateTargetDialog", () => { fireEvent.change(modelInput, { target: { value: "gpt-4" } }); // Submit - await user.click(screen.getByText("Create Target")); + await user.click(getSubmitButton()); await waitFor(() => { expect(mockedTargetsApi.createTarget).toHaveBeenCalledWith({ + name: "openai_chat_gpt_4", + persist: true, type: "OpenAIChatTarget", params: { endpoint: "https://api.openai.com", @@ -860,10 +870,12 @@ describe("CreateTargetDialog", () => { fireEvent.change(underlyingInput, { target: { value: "gpt-4o" } }); // Submit - await user.click(screen.getByText("Create Target")); + await user.click(getSubmitButton()); await waitFor(() => { expect(mockedTargetsApi.createTarget).toHaveBeenCalledWith({ + name: "openai_chat_my_gpt4o_deployment", + persist: true, type: "OpenAIChatTarget", params: { endpoint: "https://api.azure.com", @@ -901,10 +913,12 @@ describe("CreateTargetDialog", () => { // Do NOT toggle the underlying model switch - await user.click(screen.getByText("Create Target")); + await user.click(getSubmitButton()); await waitFor(() => { expect(mockedTargetsApi.createTarget).toHaveBeenCalledWith({ + name: "openai_chat_gpt_4o", + persist: true, type: "OpenAIChatTarget", params: { endpoint: "https://api.openai.com", @@ -936,7 +950,7 @@ describe("CreateTargetDialog", () => { fireEvent.change(endpointInput, { target: { value: "https://example.com" } }); // Submit - await user.click(screen.getByText("Create Target")); + await user.click(getSubmitButton()); await waitFor(() => { expect(screen.getByText("Invalid API key")).toBeInTheDocument(); @@ -971,7 +985,7 @@ describe("CreateTargetDialog", () => { const requestField = screen.getByTestId('target-param-http_request') as HTMLTextAreaElement; fireEvent.change(requestField, { target: { value: "not an HTTP request" } }); - const createButton = screen.getByRole("button", { name: "Create Target" }); + const createButton = getSubmitButton(); await act(async () => createButton.focus()); await user.keyboard(" "); @@ -989,10 +1003,10 @@ describe("CreateTargetDialog", () => { expect(requestField.getAttribute("aria-describedby")?.split(/\s+/)).toContain(error.id); }); - expect(mockedTargetsApi.createTarget).toHaveBeenCalledWith({ + expect(mockedTargetsApi.createTarget).toHaveBeenCalledWith(expect.objectContaining({ type: "HTTPTarget", params: { http_request: "not an HTTP request" }, - }); + })); fireEvent.change(requestField, { target: { value: "POST /chat HTTP/1.1\nHost: example.com" }, @@ -1039,7 +1053,7 @@ describe("CreateTargetDialog", () => { await selectTargetType("HTTPTarget"); const requestField = screen.getByRole("textbox", { name: /http request/i }); await user.type(requestField, "not an HTTP request"); - await user.click(screen.getByRole("button", { name: "Create Target" })); + await user.click(getSubmitButton()); await waitFor(() => expect(requestField).toBeDisabled()); await user.click(screen.getByText("Advanced settings", { exact: true })); @@ -1093,7 +1107,7 @@ describe("CreateTargetDialog", () => { const requestField = screen.getByTestId('target-param-http_request') as HTMLTextAreaElement; fireEvent.change(requestField, { target: { value: "not an HTTP request" } }); - const createButton = screen.getByRole("button", { name: "Create Target" }); + const createButton = getSubmitButton(); await user.click(createButton); await waitFor(() => { @@ -1144,7 +1158,7 @@ describe("CreateTargetDialog", () => { target: { value: "sk-test-key-123" }, }); - await user.click(screen.getByText("Create Target")); + await user.click(getSubmitButton()); await waitFor(() => { expect(mockedTargetsApi.createTarget).toHaveBeenCalledWith( @@ -1233,7 +1247,7 @@ describe("CreateTargetDialog", () => { render(); await selectTargetType("LiteLLMChatTarget"); await user.type(screen.getByRole("textbox", { name: "Model / Deployment Name", exact: true }), "provider/model"); - const create = screen.getByRole("button", { name: "Create Target", exact: true }); + const create = screen.getByRole("button", { name: SUBMIT_BUTTON_NAME }); const endpoint = screen.getByRole("textbox", { name: "Endpoint URL", exact: true }); if (required) { expect(endpoint).toBeRequired(); @@ -1245,9 +1259,9 @@ describe("CreateTargetDialog", () => { expect(endpoint).not.toBeRequired(); expect(create).toBeEnabled(); await user.click(create); - expect(mockedTargetsApi.createTarget).toHaveBeenCalledWith({ + expect(mockedTargetsApi.createTarget).toHaveBeenCalledWith(expect.objectContaining({ type: "LiteLLMChatTarget", params: { model_name: "provider/model" }, - }); + })); } }); @@ -1256,7 +1270,7 @@ describe("CreateTargetDialog", () => { render(); await screen.findByText(/Target details could not be loaded/); await selectTargetType("OpenAIChatTarget"); - expect(screen.getByRole("button", { name: "Create Target", exact: true })).toBeDisabled(); + expect(screen.getByRole("button", { name: SUBMIT_BUTTON_NAME })).toBeDisabled(); fireEvent.submit(screen.getByTestId("create-target-form")); expect(await screen.findByText("Please provide an endpoint URL")).toBeVisible(); expect(mockedTargetsApi.createTarget).not.toHaveBeenCalled(); @@ -1279,7 +1293,7 @@ describe("CreateTargetDialog", () => { ); fireEvent.change(endpointInput, { target: { value: "https://example.com" } }); - await user.click(screen.getByText("Create Target")); + await user.click(getSubmitButton()); await waitFor(() => { expect(screen.getByText("string error")).toBeInTheDocument(); @@ -1316,10 +1330,12 @@ describe("CreateTargetDialog", () => { fireEvent.change(modelInput, { target: { value: "Llama-3.2-3B-Instruct" } }); // Submit without overriding the displayed constructor defaults. - await user.click(screen.getByText("Create Target")); + await user.click(getSubmitButton()); await waitFor(() => { expect(mockedTargetsApi.createTarget).toHaveBeenCalledWith({ + name: "azure_machine_learning_chat_llama_3_2_3b_instruct", + persist: true, type: "AzureMLChatTarget", params: { endpoint: "https://my-llama.eastus.inference.ml.azure.com/score", @@ -1412,10 +1428,12 @@ describe("CreateTargetDialog", () => { target: { value: "1.2" }, }); - await user.click(screen.getByText("Create Target")); + await user.click(getSubmitButton()); await waitFor(() => { expect(mockedTargetsApi.createTarget).toHaveBeenCalledWith({ + name: "azure_machine_learning_chat", + persist: true, type: "AzureMLChatTarget", params: { endpoint: "https://my-model.eastus.inference.ml.azure.com/score", @@ -1457,10 +1475,12 @@ describe("CreateTargetDialog", () => { expect(screen.getByRole("textbox", { name: "Extra Body Parameters" })).toBeVisible(); - await user.click(screen.getByText("Create Target")); + await user.click(getSubmitButton()); await waitFor(() => { expect(mockedTargetsApi.createTarget).toHaveBeenCalledWith({ + name: "openai_chat", + persist: true, type: "OpenAIChatTarget", params: { endpoint: "https://api.openai.com", @@ -1517,10 +1537,12 @@ describe("CreateTargetDialog", () => { target: { value: "8" }, }); - await user.click(screen.getByText("Create Target")); + await user.click(getSubmitButton()); await waitFor(() => { expect(mockedTargetsApi.createTarget).toHaveBeenCalledWith({ + name: "openai_video_sora_2", + persist: true, type: "OpenAIVideoTarget", params: { endpoint: "https://api.openai.com", @@ -1559,10 +1581,12 @@ describe("CreateTargetDialog", () => { expect(screen.queryByRole("textbox", { name: "Custom Functions" })).not.toBeInTheDocument(); - await user.click(screen.getByText("Create Target")); + await user.click(getSubmitButton()); await waitFor(() => { expect(mockedTargetsApi.createTarget).toHaveBeenCalledWith({ + name: "openai_responses_api", + persist: true, type: "OpenAIResponseTarget", params: { endpoint: "https://api.openai.com", @@ -1596,10 +1620,12 @@ describe("CreateTargetDialog", () => { target: { value: "END, DONE" }, }); - await user.click(screen.getByText("Create Target")); + await user.click(getSubmitButton()); await waitFor(() => { expect(mockedTargetsApi.createTarget).toHaveBeenCalledWith({ + name: "openai_text_completion", + persist: true, type: "OpenAICompletionTarget", params: { endpoint: "https://api.openai.com", @@ -1628,10 +1654,12 @@ describe("CreateTargetDialog", () => { }); await user.click(screen.getByText("Advanced settings")); await user.click(screen.getByRole("checkbox", { name: "Use empty list for stop" })); - await user.click(screen.getByText("Create Target")); + await user.click(getSubmitButton()); await waitFor(() => { expect(mockedTargetsApi.createTarget).toHaveBeenCalledWith({ + name: "openai_text_completion", + persist: true, type: "OpenAICompletionTarget", params: { endpoint: "https://api.openai.com", @@ -1667,10 +1695,12 @@ describe("CreateTargetDialog", () => { target: { value: '{"timeout":180}' }, }); - await user.click(screen.getByText("Create Target")); + await user.click(getSubmitButton()); await waitFor(() => { expect(mockedTargetsApi.createTarget).toHaveBeenCalledWith({ + name: "openai_chat", + persist: true, type: "OpenAIChatTarget", params: { endpoint: "https://api.openai.com", @@ -1707,7 +1737,7 @@ describe("CreateTargetDialog", () => { }); expect(screen.getByText("Extra Body Parameters must be a JSON object.")).toBeVisible(); - await user.click(screen.getByText("Create Target")); + await user.click(getSubmitButton()); expect(mockedTargetsApi.createTarget).not.toHaveBeenCalled(); }); @@ -1752,7 +1782,7 @@ describe("CreateTargetDialog", () => { expect(screen.queryByLabelText("Endpoint URL")).not.toBeInTheDocument(); expect(screen.queryByPlaceholderText("API key (stored in memory only)")).not.toBeInTheDocument(); - expect(screen.getByText("Create Target").closest("button")).toBeDisabled(); + expect(getSubmitButton().closest("button")).toBeDisabled(); // A real raw HTTP request template needs actual line breaks between the // request line, headers, and body — the backend parser splits on real @@ -1765,11 +1795,13 @@ describe("CreateTargetDialog", () => { target: { value: httpRequestTemplate }, }); - expect(screen.getByText("Create Target").closest("button")).toBeEnabled(); - await user.click(screen.getByText("Create Target")); + expect(getSubmitButton().closest("button")).toBeEnabled(); + await user.click(getSubmitButton()); await waitFor(() => { expect(mockedTargetsApi.createTarget).toHaveBeenCalledWith({ + name: "http", + persist: true, type: "HTTPTarget", params: { http_request: httpRequestTemplate, @@ -1812,13 +1844,15 @@ describe("CreateTargetDialog", () => { expect(screen.queryByRole("radio", { name: /Identity-based/ })).not.toBeInTheDocument(); expect(screen.getByLabelText("GitHub Token", { selector: "input" })).toBeVisible(); expect(screen.getByLabelText("GitHub Token", { selector: "input" })).toHaveAttribute("type", "password"); - expect(screen.getByRole("button", { name: "Create Target" })).toBeDisabled(); + expect(getSubmitButton()).toBeDisabled(); await user.type(screen.getByRole("textbox", { name: /Model \/ Deployment Name/ }), "gpt-4.1"); - await user.click(screen.getByRole("button", { name: "Create Target" })); + await user.click(getSubmitButton()); await waitFor(() => { expect(mockedTargetsApi.createTarget).toHaveBeenCalledWith({ + name: "github_copilot_gpt_4_1", + persist: true, type: "GitHubCopilotTarget", params: { model_name: "gpt-4.1" }, }); @@ -1847,17 +1881,19 @@ describe("CreateTargetDialog", () => { await selectTargetType("GitHubCopilotTarget"); await user.type(screen.getByRole("textbox", { name: /Model \/ Deployment Name/ }), "gpt-4.1"); - await user.click(screen.getByRole("button", { name: "Create Target" })); + await user.click(getSubmitButton()); expect(await screen.findByText(detail)).toBeVisible(); expect(defaultProps.onCreated).not.toHaveBeenCalled(); expect(screen.getByRole("textbox", { name: /Model \/ Deployment Name/ })).toHaveValue("gpt-4.1"); await user.type(screen.getByLabelText("GitHub Token", { selector: "input" }), "test-github-token"); - await user.click(screen.getByRole("button", { name: "Create Target" })); + await user.click(getSubmitButton()); await waitFor(() => expect(defaultProps.onCreated).toHaveBeenCalledTimes(1)); expect(mockedTargetsApi.createTarget).toHaveBeenLastCalledWith({ + name: "github_copilot_gpt_4_1", + persist: false, type: "GitHubCopilotTarget", params: { model_name: "gpt-4.1", github_token: "test-github-token" }, }); @@ -1884,10 +1920,12 @@ describe("CreateTargetDialog", () => { await user.selectOptions(screen.getByRole("combobox", { name: "Retain Session" }), "false"); await user.type(screen.getByRole("spinbutton", { name: "Response Timeout Seconds" }), "120"); await user.type(screen.getByRole("spinbutton", { name: "Max Requests Per Minute" }), "10"); - await user.click(screen.getByRole("button", { name: "Create Target" })); + await user.click(getSubmitButton()); await waitFor(() => { expect(mockedTargetsApi.createTarget).toHaveBeenCalledWith({ + name: "github_copilot_gpt_4_1", + persist: false, type: "GitHubCopilotTarget", params: { model_name: "gpt-4.1", @@ -1933,15 +1971,17 @@ describe("CreateTargetDialog", () => { expect(screen.queryByLabelText(/Endpoint URL/)).not.toBeInTheDocument(); expect(screen.queryByLabelText("API Key")).not.toBeInTheDocument(); expect(screen.queryByRole("radio", { name: /Identity-based/ })).not.toBeInTheDocument(); - expect(screen.getByRole("button", { name: "Create Target" })).toBeDisabled(); + expect(getSubmitButton()).toBeDisabled(); expect(screen.getByLabelText("GitHub Token", { selector: "input" })).toHaveAttribute("type", "password"); await user.type(screen.getByRole("textbox", { name: /Model \/ Deployment Name/ }), "gpt-4.1"); await user.type(screen.getByLabelText("GitHub Token", { selector: "input" }), "test-github-token"); - await user.click(screen.getByRole("button", { name: "Create Target" })); + await user.click(getSubmitButton()); await waitFor(() => { expect(mockedTargetsApi.createTarget).toHaveBeenCalledWith({ + name: "github_copilot_gpt_4_1", + persist: false, type: "GitHubCopilotTarget", params: { model_name: "gpt-4.1", github_token: "test-github-token" }, }); @@ -1967,10 +2007,12 @@ describe("CreateTargetDialog", () => { await selectTargetType("OpenAIChatTarget"); expect(screen.queryByText(/Model IDs are not validated when creating a target/)).not.toBeInTheDocument(); await user.type(screen.getByRole("textbox", { name: /Endpoint URL/ }), "https://api.openai.com"); - await user.click(screen.getByRole("button", { name: "Create Target" })); + await user.click(getSubmitButton()); await waitFor(() => { expect(mockedTargetsApi.createTarget).toHaveBeenCalledWith({ + name: "openai_chat", + persist: true, type: "OpenAIChatTarget", params: { endpoint: "https://api.openai.com" }, }); @@ -1997,7 +2039,7 @@ describe("CreateTargetDialog", () => { await user.click(screen.getByRole("radio", { name: /Identity-based/ })); - expect(screen.getByText("Create Target").closest("button")).toBeEnabled(); + expect(getSubmitButton().closest("button")).toBeEnabled(); }); it("should mask the sas_token parameter and drop it once identity auth is selected", async () => { @@ -2030,10 +2072,12 @@ describe("CreateTargetDialog", () => { expect(sasTokenField).toBeDisabled(); expect(sasTokenField).toHaveValue(""); - await user.click(screen.getByText("Create Target")); + await user.click(getSubmitButton()); await waitFor(() => { expect(mockedTargetsApi.createTarget).toHaveBeenCalledWith({ + name: "azure_blob_storage", + persist: true, type: "AzureBlobStorageTarget", params: {}, auth_mode: "identity", @@ -2104,10 +2148,12 @@ describe("CreateTargetDialog", () => { screen.queryByPlaceholderText("API key (stored in memory only)") ).not.toBeInTheDocument(); - await user.click(screen.getByText("Create Target")); + await user.click(getSubmitButton()); await waitFor(() => { expect(mockedTargetsApi.createTarget).toHaveBeenCalledWith({ + name: "openai_chat", + persist: true, type: "OpenAIChatTarget", params: { endpoint: "https://my-resource.openai.azure.com/", @@ -2151,7 +2197,7 @@ describe("CreateTargetDialog", () => { screen.getByRole("radio", { name: /Identity-based/ }) ); - await user.click(screen.getByText("Create Target")); + await user.click(getSubmitButton()); await waitFor(() => { const call = mockedTargetsApi.createTarget.mock.calls[0][0]; @@ -2188,10 +2234,12 @@ describe("CreateTargetDialog", () => { expect(screen.queryByPlaceholderText("API key (stored in memory only)")).not.toBeInTheDocument(); - await user.click(screen.getByText("Create Target")); + await user.click(getSubmitButton()); await waitFor(() => { expect(mockedTargetsApi.createTarget).toHaveBeenCalledWith({ + name: "openai_chat", + persist: true, type: "OpenAIChatTarget", params: { endpoint: "http://localhost:11434/v1", @@ -2297,7 +2345,7 @@ describe("CreateTargetDialog", () => { ).not.toBeInTheDocument(); }); - it("should disable Create Target and skip API call for identity + non-Azure OpenAI endpoint", async () => { + it("should disable the submit button and skip API call for identity + non-Azure OpenAI endpoint", async () => { const user = userEvent.setup(); render( @@ -2317,10 +2365,10 @@ describe("CreateTargetDialog", () => { screen.getByRole("radio", { name: /Identity-based/ }) ); - const createButton = screen.getByText("Create Target").closest("button"); + const createButton = getSubmitButton().closest("button"); expect(createButton).toBeDisabled(); - await user.click(screen.getByText("Create Target")); + await user.click(getSubmitButton()); expect(mockedTargetsApi.createTarget).not.toHaveBeenCalled(); }); @@ -2383,7 +2431,7 @@ describe("CreateTargetDialog", () => { ).not.toBeInTheDocument(); }); - it("should disable Create Target and skip API call for identity + non-AML endpoint on AzureMLChatTarget", async () => { + it("should disable the submit button and skip API call for identity + non-AML endpoint on AzureMLChatTarget", async () => { const user = userEvent.setup(); render( @@ -2405,10 +2453,10 @@ describe("CreateTargetDialog", () => { screen.getByRole("radio", { name: /Identity-based/ }) ); - const createButton = screen.getByText("Create Target").closest("button"); + const createButton = getSubmitButton().closest("button"); expect(createButton).toBeDisabled(); - await user.click(screen.getByText("Create Target")); + await user.click(getSubmitButton()); expect(mockedTargetsApi.createTarget).not.toHaveBeenCalled(); }); @@ -2467,7 +2515,7 @@ describe("CreateTargetDialog", () => { await selectTargetType("RoundRobinTarget"); - const createButton = screen.getByText("Create Target").closest("button"); + const createButton = getSubmitButton().closest("button"); expect(createButton).toBeDisabled(); }); @@ -2671,7 +2719,7 @@ describe("CreateTargetDialog", () => { await user.selectOptions(select, "a"); await user.selectOptions(select, "b"); - await user.click(screen.getByText("Create Target")); + await user.click(getSubmitButton()); await waitFor(() => { // The backend's detail (the actual validation message) should be shown to @@ -2759,11 +2807,11 @@ describe("CreateTargetDialog", () => { ); expect(screen.getByText("Weight must be a whole number")).toBeInTheDocument(); - expect(screen.getByText("Create Target").closest("button")).toBeDisabled(); + expect(getSubmitButton().closest("button")).toBeDisabled(); // Pressing Enter inside the weight input must not bypass the disabled // button and submit the form. - await user.click(screen.getByText("Create Target")); + await user.click(getSubmitButton()); expect(mockedTargetsApi.createTarget).not.toHaveBeenCalled(); }, 30000); @@ -2784,12 +2832,12 @@ describe("CreateTargetDialog", () => { const inputs = screen.getAllByLabelText(/Weight for /) as HTMLInputElement[]; expect(inputs[0].value).toBe("7"); expect(inputs[1].value).toBe("42"); - expect(screen.getByText("Create Target").closest("button")).not.toBeDisabled(); + expect(getSubmitButton().closest("button")).not.toBeDisabled(); }, { timeout: 10000 }, ); - await user.click(screen.getByText("Create Target")); + await user.click(getSubmitButton()); await waitFor( () => { @@ -2818,7 +2866,7 @@ describe("CreateTargetDialog", () => { { timeout: 10000 }, ); // With only one inner target left, Create is disabled (needs >= 2). - expect(screen.getByText("Create Target").closest("button")).toBeDisabled(); + expect(getSubmitButton().closest("button")).toBeDisabled(); }, 30000); it("submit-time guards reject invalid weights even if the disabled button is bypassed", async () => { @@ -2830,7 +2878,7 @@ describe("CreateTargetDialog", () => { fireEvent.change(weightInputs[0], { target: { value: "2.5" } }); await waitFor( () => { - expect(screen.getByText("Create Target").closest("button")).toBeDisabled(); + expect(getSubmitButton().closest("button")).toBeDisabled(); }, { timeout: 10000 }, ); @@ -2876,3 +2924,843 @@ describe("CreateTargetDialog", () => { expect(mockedTargetsApi.createTarget).not.toHaveBeenCalled(); }, 30000); }); + +const GITHUB_COPILOT_NOT_SAVEABLE_REASON = + "'GitHubCopilotTarget' needs a credential in 'github_token', which saved entries can't hold or take from this " + + "server yet."; + +// What the backend reports about saving each type, so the dialog can tell before submitting. +const SAVEABILITY: Record> = { + OpenAIChatTarget: { saveable_auth_modes: ["identity", "none"], not_saveable_reason: null }, + GitHubCopilotTarget: { saveable_auth_modes: [], not_saveable_reason: GITHUB_COPILOT_NOT_SAVEABLE_REASON }, + RoundRobinTarget: { saveable_auth_modes: ["api_key"], not_saveable_reason: null }, +}; + +const SAVEABLE_TARGET_TYPES: TargetTypeListResponse = { + items: TARGET_TYPES.items.map((entry) => ({ ...entry, ...SAVEABILITY[entry.target_type] })), +}; + +// A2ATarget saves without credentials, but its optional token and key can't be saved. +const A2A_TARGET_TYPE: TargetTypeEntry = { + target_type: "A2ATarget", + parameters: [ + { name: "endpoint", type_name: "str", required: true, default: null, description: "The A2A agent endpoint." }, + { name: "auth_token", type_name: "str", required: false, default: null, sensitive: true }, + { name: "api_key", type_name: "str", required: false, default: null, sensitive: true }, + ], + supported_auth_modes: ["api_key"], + saveable_auth_modes: ["api_key"], + not_saveable_reason: null, +}; + +const AZURE_ENDPOINT = "https://contoso.openai.azure.com/openai/v1"; + +function axiosError(status: number, data: Record): Error { + return Object.assign(new Error("Request failed"), { isAxiosError: true, response: { status, data } }); +} + +// Pasting fills a field in one input event, which keeps these tests well inside the timeout on a busy machine. +async function pasteInto(user: ReturnType, field: HTMLElement, text: string): Promise { + await user.click(field); + await user.paste(text); +} + +describe("CreateTargetDialog saving", () => { + const defaultProps = { + open: true, + onClose: jest.fn(), + onCreated: jest.fn(), + }; + + let dialogAccessibilityObserver: MutationObserver; + + beforeEach(() => { + dialogAccessibilityObserver = watchDialogAccessibility(); + jest.clearAllMocks(); + mockedTargetsApi.listTargetTypes.mockResolvedValue(SAVEABLE_TARGET_TYPES); + mockedTargetsApi.listTargets.mockResolvedValue({ + items: [], + pagination: { limit: 200, has_more: false, next_cursor: null, prev_cursor: null }, + } as unknown as Awaited>); + mockedTargetsApi.createTarget.mockResolvedValue(makeTarget({ target_registry_name: "openai_chat" })); + }); + + afterEach(() => { + dialogAccessibilityObserver.disconnect(); + }); + + async function fillAzureOpenAiChat(user: ReturnType): Promise { + await selectTargetType("OpenAIChatTarget"); + await pasteInto(user, screen.getByPlaceholderText("https://your-resource.openai.azure.com/"), AZURE_ENDPOINT); + await pasteInto(user, screen.getByRole("textbox", { name: /Model \/ Deployment Name/ }), "gpt-4o"); + } + + it("should suggest a registry name from the target type that no target has", async () => { + render( + + + + ); + + await selectTargetType("OpenAIChatTarget"); + + expect(screen.getByRole("textbox", { name: /Registry Name/ })).toHaveValue("openai_chat_2"); + }); + + it("should follow the model in the suggested name until the name is edited", async () => { + const user = userEvent.setup(); + render( + + + + ); + + await selectTargetType("OpenAIChatTarget"); + const nameInput = screen.getByRole("textbox", { name: /Registry Name/ }); + expect(nameInput).toHaveValue("openai_chat"); + const modelInput = screen.getByRole("textbox", { name: /Model \/ Deployment Name/ }); + await pasteInto(user, modelInput, "gpt-4o"); + // A startup initializer registers `openai_chat`, which would stop a saved target of that name restoring. + expect(nameInput).toHaveValue("openai_chat_gpt_4o"); + + await user.type(nameInput, "_eval"); + await pasteInto(user, modelInput, "-mini"); + expect(nameInput).toHaveValue("openai_chat_gpt_4o_eval"); + }); + + it("should keep a name the user typed when the target type changes", async () => { + const user = userEvent.setup(); + render( + + + + ); + + await selectTargetType("OpenAIChatTarget"); + const nameInput = screen.getByRole("textbox", { name: /Registry Name/ }); + await user.clear(nameInput); + await user.type(nameInput, "contoso_gpt4o"); + await selectTargetType("GitHubCopilotTarget"); + + expect(screen.getByRole("textbox", { name: /Registry Name/ })).toHaveValue("contoso_gpt4o"); + }); + + it("should show the naming rule and keep the button disabled for an invalid name", async () => { + const user = userEvent.setup(); + render( + + + + ); + + await fillAzureOpenAiChat(user); + const nameInput = screen.getByRole("textbox", { name: /Registry Name/ }); + await user.clear(nameInput); + await user.type(nameInput, "Contoso-GPT"); + + expect( + screen.getByText("Use lowercase letters, digits and underscores, starting with a letter (max 64 characters)."), + ).toBeInTheDocument(); + expect(getSubmitButton()).toBeDisabled(); + }); + + it("should not send an invalid name when the form is submitted", async () => { + const user = userEvent.setup(); + render( + + + + ); + + await fillAzureOpenAiChat(user); + const nameInput = screen.getByRole("textbox", { name: /Registry Name/ }); + await user.clear(nameInput); + await user.type(nameInput, "Contoso-GPT"); + fireEvent.submit(screen.getByTestId("create-target-form")); + + expect(mockedTargetsApi.createTarget).not.toHaveBeenCalled(); + expect(nameInput).toHaveAccessibleDescription(expect.stringContaining("Use lowercase letters")); + }); + + it("should start with Microsoft Entra ID and save the target", async () => { + const user = userEvent.setup(); + render( + + + + ); + + await fillAzureOpenAiChat(user); + + expect(screen.getByRole("radio", { name: /Identity-based/ })).toBeChecked(); + await user.click(screen.getByRole("button", { name: "Save Target" })); + + await waitFor(() => expect(defaultProps.onCreated).toHaveBeenCalledTimes(1)); + expect(mockedTargetsApi.createTarget).toHaveBeenCalledWith({ + name: "openai_chat_gpt_4o", + type: "OpenAIChatTarget", + params: { endpoint: AZURE_ENDPOINT, model_name: "gpt-4o" }, + auth_mode: "identity", + persist: true, + }); + }); + + async function renderWithPendingTargetTypes(): Promise<(targetTypes: TargetTypeListResponse) => void> { + let resolveTargetTypes: ((targetTypes: TargetTypeListResponse) => void) | null = null; + mockedTargetsApi.listTargetTypes.mockReturnValue( + new Promise((resolve) => { + resolveTargetTypes = resolve; + }), + ); + render( + + + + ); + await selectTargetType("OpenAIChatTarget"); + if (resolveTargetTypes === null) { + throw new Error("Target type resolver was not initialized"); + } + return resolveTargetTypes; + } + + it("should switch to a saveable default when the target details arrive after the type is chosen", async () => { + const user = userEvent.setup(); + const resolveTargetTypes = await renderWithPendingTargetTypes(); + expect(screen.getByRole("radio", { name: "API Key" })).toBeChecked(); + + await act(async () => { + resolveTargetTypes(SAVEABLE_TARGET_TYPES); + }); + + expect(screen.getByRole("radio", { name: /Identity-based/ })).toBeChecked(); + await pasteInto(user, screen.getByPlaceholderText("https://your-resource.openai.azure.com/"), AZURE_ENDPOINT); + await pasteInto(user, screen.getByRole("textbox", { name: /Model \/ Deployment Name/ }), "gpt-4o"); + expect(screen.getByRole("button", { name: "Save Target" })).toBeEnabled(); + }); + + it("should keep a key typed before the target details arrive and explain why it can't be saved", async () => { + const user = userEvent.setup(); + const resolveTargetTypes = await renderWithPendingTargetTypes(); + await user.type(screen.getByPlaceholderText("API key (stored in memory only)"), "test-key"); + + await act(async () => { + resolveTargetTypes(SAVEABLE_TARGET_TYPES); + }); + + expect(screen.getByRole("radio", { name: "API Key" })).toBeChecked(); + expect(screen.getByPlaceholderText("API key (stored in memory only)")).toHaveValue("test-key"); + expect(screen.getByText(/Saved targets can't use an API key yet\./)).toBeInTheDocument(); + expect(screen.getByRole("button", { name: "Create as Temporary" })).toBeInTheDocument(); + }); + + it("should go back to the saveable default when the target type changes", async () => { + const user = userEvent.setup(); + render( + + + + ); + + await selectTargetType("OpenAIChatTarget"); + await user.click(screen.getByRole("radio", { name: "API Key" })); + await selectTargetType("GitHubCopilotTarget"); + await selectTargetType("OpenAIChatTarget"); + + expect(screen.getByRole("radio", { name: /Identity-based/ })).toBeChecked(); + }); + + it("should keep the key and sign-in when the selected type is picked again", async () => { + const user = userEvent.setup(); + render( + + + + ); + + await selectTargetType("OpenAIChatTarget"); + await user.click(screen.getByRole("radio", { name: "API Key" })); + await user.type(screen.getByPlaceholderText("API key (stored in memory only)"), "test-key"); + await selectTargetType("OpenAIChatTarget"); + + expect(screen.getByRole("radio", { name: "API Key" })).toBeChecked(); + expect(screen.getByPlaceholderText("API key (stored in memory only)")).toHaveValue("test-key"); + }); + + it("should point a local endpoint that fails the Entra ID check to No key once the field is left", async () => { + const user = userEvent.setup(); + render( + + + + ); + + await selectTargetType("OpenAIChatTarget"); + await pasteInto(user, screen.getByRole("textbox", { name: /Model \/ Deployment Name/ }), "gpt-4o"); + await pasteInto(user, screen.getByPlaceholderText("https://your-resource.openai.azure.com/"), "http://localhost:11434/v1"); + + // No error while the address is still being typed, but saving already waits for one that works. + expect(screen.queryByText(/Identity-based auth only works/)).not.toBeInTheDocument(); + expect(screen.getByRole("button", { name: "Save Target" })).toBeDisabled(); + await user.tab(); + expect(screen.getAllByRole("status").some((region) => ( + region.textContent?.includes("For a server that needs no key, such as a local one, choose No key.") + ))).toBe(true); + await user.click(screen.getByRole("radio", { name: /^No key/ })); + expect(screen.queryByText(/Identity-based auth only works/)).not.toBeInTheDocument(); + expect(screen.getByRole("button", { name: "Save Target" })).toBeEnabled(); + }); + + it("should ask for a model, and suggest a name with it, when an OpenAI type's details fail to load", async () => { + mockedTargetsApi.listTargetTypes.mockRejectedValueOnce(new Error("registry unavailable")); + const user = userEvent.setup(); + render( + + + + ); + + expect(await screen.findByText(/Target details could not be loaded/)).toBeInTheDocument(); + await selectTargetType("OpenAIChatTarget"); + await pasteInto(user, screen.getByPlaceholderText("https://your-resource.openai.azure.com/"), AZURE_ENDPOINT); + expect(screen.getByRole("button", { name: "Save Target" })).toBeDisabled(); + await pasteInto(user, screen.getByRole("textbox", { name: /Model \/ Deployment Name/ }), "gpt-4o"); + + expect(screen.getByRole("textbox", { name: /Registry Name/ })).toHaveValue("openai_chat_gpt_4o"); + expect(screen.getByRole("button", { name: "Save Target" })).toBeEnabled(); + }); + + it("should not carry a typed key over to another target type", async () => { + const user = userEvent.setup(); + mockedTargetsApi.listTargetTypes.mockResolvedValue({ items: [...SAVEABLE_TARGET_TYPES.items, A2A_TARGET_TYPE] }); + render( + + + + ); + + await selectTargetType("OpenAIChatTarget"); + await user.click(screen.getByRole("radio", { name: "API Key" })); + await user.type(screen.getByPlaceholderText("API key (stored in memory only)"), "test-key"); + await selectTargetType("A2ATarget"); + + expect(screen.getByPlaceholderText("API key (stored in memory only)")).toHaveValue(""); + await pasteInto(user, screen.getByPlaceholderText("https://example.com/"), "https://agent.example.com"); + await user.click(screen.getByRole("button", { name: "Save Target" })); + + await waitFor(() => expect(defaultProps.onCreated).toHaveBeenCalledTimes(1)); + expect(mockedTargetsApi.createTarget).toHaveBeenCalledWith({ + name: "a2a", + type: "A2ATarget", + params: { endpoint: "https://agent.example.com" }, + persist: true, + }); + }); + + it("should offer Temporary with the reason when the target uses an API key", async () => { + const user = userEvent.setup(); + render( + + + + ); + + await fillAzureOpenAiChat(user); + await user.click(screen.getByRole("radio", { name: "API Key" })); + await user.type(screen.getByPlaceholderText("API key (stored in memory only)"), "test-key"); + + // A live region, so screen readers hear why the button changed. + const warning = screen.getByRole("status", { name: "This target can't be saved" }); + expect(warning).toHaveTextContent( + /Saved targets can't use an API key yet\. Choose Microsoft Entra ID or No key to save this one\./, + ); + expect(warning).toHaveTextContent("A Temporary target is gone after the backend restarts or is reinitialized."); + // Once when it appeared, not again for each typed character. + expect(jest.mocked(Element.prototype.scrollIntoView).mock.contexts.filter((context) => context === warning)) + .toHaveLength(1); + await user.click(screen.getByRole("button", { name: "Create as Temporary" })); + + await waitFor(() => expect(defaultProps.onCreated).toHaveBeenCalledTimes(1)); + expect(mockedTargetsApi.createTarget).toHaveBeenCalledWith({ + name: "openai_chat_gpt_4o", + type: "OpenAIChatTarget", + params: { endpoint: AZURE_ENDPOINT, model_name: "gpt-4o", api_key: "test-key" }, + persist: false, + }); + }); + + it("should explain before submitting why a target type can't be saved", async () => { + render( + + + + ); + + await selectTargetType("GitHubCopilotTarget"); + + expect(screen.getByText(new RegExp(GITHUB_COPILOT_NOT_SAVEABLE_REASON.replace(/[.']/g, ".")))).toBeInTheDocument(); + expect(screen.getByRole("button", { name: "Create as Temporary" })).toBeInTheDocument(); + }); + + it("should offer Temporary only while a credential is typed for a target that saves without one", async () => { + const user = userEvent.setup(); + mockedTargetsApi.listTargetTypes.mockResolvedValue({ items: [...SAVEABLE_TARGET_TYPES.items, A2A_TARGET_TYPE] }); + render( + + + + ); + + await selectTargetType("A2ATarget"); + await pasteInto(user, screen.getByPlaceholderText("https://example.com/"), "https://agent.example.com"); + expect(screen.getByRole("button", { name: "Save Target" })).toBeEnabled(); + + await user.click(screen.getByText("Advanced settings")); + const tokenField = screen.getByLabelText(/Auth Token/, { selector: "input" }); + await pasteInto(user, tokenField, "secret-token"); + + expect(screen.getByText(/Saved targets can't hold credentials \('auth_token'\) yet\./)).toBeInTheDocument(); + expect(screen.getByRole("button", { name: "Create as Temporary" })).toBeInTheDocument(); + + await user.clear(tokenField); + + expect(screen.queryByText("This target can't be saved")).not.toBeInTheDocument(); + await user.click(screen.getByRole("button", { name: "Save Target" })); + await waitFor(() => expect(defaultProps.onCreated).toHaveBeenCalledTimes(1)); + expect(mockedTargetsApi.createTarget).toHaveBeenCalledWith({ + name: "a2a", + type: "A2ATarget", + params: { endpoint: "https://agent.example.com" }, + persist: true, + }); + }); + + it("should offer Temporary after the server refuses to save, only for the values it refused", async () => { + const user = userEvent.setup(); + const detail = "'endpoint' can carry a credential in a URL, which saved entries can't hold. " + + "It can still be created without saving it."; + mockedTargetsApi.createTarget.mockRejectedValueOnce(axiosError(400, { type: "/errors/not-saveable", detail })); + render( + + + + ); + + await fillAzureOpenAiChat(user); + await user.click(screen.getByRole("button", { name: "Save Target" })); + + expect(await screen.findByText(new RegExp("can carry a credential in a URL"))).toBeInTheDocument(); + await user.click(screen.getByRole("button", { name: "Create as Temporary" })); + + await waitFor(() => expect(defaultProps.onCreated).toHaveBeenCalledTimes(1)); + expect(mockedTargetsApi.createTarget.mock.calls.map(([request]) => request.persist)).toEqual([true, false]); + expect(mockedTargetsApi.createTarget.mock.calls[1][0]).toEqual({ + ...mockedTargetsApi.createTarget.mock.calls[0][0], + persist: false, + }); + }); + + it("should go back to saving once the refused values change", async () => { + const user = userEvent.setup(); + mockedTargetsApi.createTarget.mockRejectedValueOnce(axiosError(400, { + type: "/errors/not-saveable", + detail: "Refused. It can still be created without saving it.", + })); + render( + + + + ); + + await fillAzureOpenAiChat(user); + await user.click(screen.getByRole("button", { name: "Save Target" })); + expect(await screen.findByRole("button", { name: "Create as Temporary" })).toBeInTheDocument(); + + await user.type(screen.getByRole("textbox", { name: /Model \/ Deployment Name/ }), "-mini"); + + expect(screen.getByRole("button", { name: "Save Target" })).toBeInTheDocument(); + expect(screen.queryByText("This target can't be saved")).not.toBeInTheDocument(); + }); + + it("should go back to saving once an advanced value the server refused changes", async () => { + const user = userEvent.setup(); + mockedTargetsApi.createTarget.mockRejectedValueOnce(axiosError(400, { + type: "/errors/not-saveable", + detail: "Refused. It can still be created without saving it.", + })); + render( + + + + ); + + await fillAzureOpenAiChat(user); + await user.click(screen.getByRole("button", { name: "Save Target" })); + expect(await screen.findByRole("button", { name: "Create as Temporary" })).toBeInTheDocument(); + + await user.click(screen.getByText("Advanced settings")); + await pasteInto(user, screen.getByLabelText(/Temperature/, { selector: "input" }), "0.5"); + + expect(screen.getByRole("button", { name: "Save Target" })).toBeInTheDocument(); + }); + + it.each([ + ["an ordinary bad request", 400, { detail: "Endpoint is not reachable." }], + ["a validation error", 422, { detail: "Registry name is invalid." }], + ["a forbidden request", 403, { detail: "You can't create targets." }], + ["unavailable storage", 503, { detail: "Saved targets can't be written right now." }], + ])("should not offer Temporary for %s", async (_, status, data) => { + const user = userEvent.setup(); + mockedTargetsApi.createTarget.mockRejectedValueOnce(axiosError(status, data)); + render( + + + + ); + + await fillAzureOpenAiChat(user); + await user.click(screen.getByRole("button", { name: "Save Target" })); + + expect(await screen.findByText(data.detail)).toBeInTheDocument(); + expect(screen.getByRole("button", { name: "Save Target" })).toBeInTheDocument(); + expect(screen.queryByRole("button", { name: "Create as Temporary" })).not.toBeInTheDocument(); + }); + + it("should start over with saving after the dialog is closed and reopened", async () => { + const user = userEvent.setup(); + mockedTargetsApi.createTarget.mockRejectedValueOnce(axiosError(400, { + type: "/errors/not-saveable", + detail: "Refused. It can still be created without saving it.", + })); + const { rerender } = render( + + + + ); + + await fillAzureOpenAiChat(user); + await user.type(screen.getByRole("textbox", { name: /Registry Name/ }), "_custom"); + await user.click(screen.getByRole("button", { name: "Save Target" })); + expect(await screen.findByRole("button", { name: "Create as Temporary" })).toBeInTheDocument(); + await user.click(screen.getByRole("button", { name: "Cancel" })); + rerender( + + + + ); + rerender( + + + + ); + + expect(screen.getByRole("textbox", { name: /Registry Name/ })).toHaveValue(""); + await fillAzureOpenAiChat(user); + expect(screen.getByRole("textbox", { name: /Registry Name/ })).toHaveValue("openai_chat_gpt_4o"); + expect(screen.getByRole("button", { name: "Save Target" })).toBeInTheDocument(); + expect(screen.queryByText("This target can't be saved")).not.toBeInTheDocument(); + }); + + it("should ask for a target type when Enter is pressed in a filled-in name first, until one is chosen", async () => { + const user = userEvent.setup(); + render( + + + + ); + + // Until a type is chosen the name is the form's only input, so Enter submits once it holds a name. + await user.type(screen.getByRole("textbox", { name: /Registry Name/ }), "my_chat{Enter}"); + + expect(await screen.findByText("Please select a target type")).toBeInTheDocument(); + await fillAzureOpenAiChat(user); + expect(screen.queryByText("Please select a target type")).not.toBeInTheDocument(); + expect(screen.getByRole("textbox", { name: /Registry Name/ })).toHaveValue("my_chat"); + expect(mockedTargetsApi.createTarget).not.toHaveBeenCalled(); + }); + + it("should show the Entra ID endpoint error after the dialog was closed from the endpoint field", async () => { + const user = userEvent.setup(); + const { rerender } = render( + + + + ); + + await selectTargetType("OpenAIChatTarget"); + await pasteInto(user, screen.getByPlaceholderText("https://your-resource.openai.azure.com/"), "http://localhost:11434/v1"); + await user.keyboard("{Escape}"); + expect(defaultProps.onClose).toHaveBeenCalledTimes(1); + rerender( + + + + ); + rerender( + + + + ); + + await selectTargetType("OpenAIChatTarget"); + fireEvent.change(screen.getByPlaceholderText("https://your-resource.openai.azure.com/"), { + target: { value: "http://localhost:11434/v1" }, + }); + expect(screen.getByText(/Identity-based auth only works with Azure OpenAI/)).toBeInTheDocument(); + }); + + it("should say a name is taken before submitting when the dialog already lists it", async () => { + const user = userEvent.setup(); + render( + + + + ); + + await fillAzureOpenAiChat(user); + const nameInput = screen.getByRole("textbox", { name: /Registry Name/ }); + await user.clear(nameInput); + await user.type(nameInput, "contoso_chat"); + + expect(nameInput).toHaveAccessibleDescription( + expect.stringContaining("A target named 'contoso_chat' already exists."), + ); + expect(screen.getByRole("button", { name: "Save Target" })).toBeDisabled(); + await user.type(nameInput, "_2"); + expect(screen.getByRole("button", { name: "Save Target" })).toBeEnabled(); + }); + + it("should not hold a name that the refreshed target list no longer has", async () => { + const user = userEvent.setup(); + const temporaryChat = makeTarget({ + target_registry_name: "temp_chat", + target_type: "OpenAIChatTarget", + model_name: "gpt-4o", + origin: "temporary", + }); + const { rerender } = render( + + + + ); + + await selectTargetType("RoundRobinTarget"); + await fillAzureOpenAiChat(user); + rerender( + + + + ); + const nameInput = screen.getByRole("textbox", { name: /Registry Name/ }); + await user.clear(nameInput); + await pasteInto(user, nameInput, "temp_chat"); + + expect(nameInput).not.toHaveAccessibleDescription(expect.stringContaining("already exists")); + expect(screen.getByRole("button", { name: "Save Target" })).toBeEnabled(); + }); + + it("should keep a conflict that arrives late on the name it was reported for", async () => { + const user = userEvent.setup(); + const detail = "Instance 'openai_chat_gpt_4o' already exists"; + let rejectCreate: ((error: Error) => void) | null = null; + mockedTargetsApi.createTarget.mockImplementationOnce(() => new Promise((_, reject) => { + rejectCreate = reject; + })); + render( + + + + ); + + await fillAzureOpenAiChat(user); + await user.click(screen.getByRole("button", { name: "Save Target" })); + const nameInput = screen.getByRole("textbox", { name: /Registry Name/ }); + await user.type(nameInput, "_2"); + const modelInput = screen.getByRole("textbox", { name: /Model \/ Deployment Name/ }); + await user.click(modelInput); + if (rejectCreate === null) { + throw new Error("The create request was not sent"); + } + await act(async () => { + rejectCreate(axiosError(409, { detail })); + }); + + expect(modelInput).toHaveFocus(); + expect(nameInput).not.toHaveAccessibleDescription(expect.stringContaining(detail)); + expect(screen.getByRole("button", { name: "Save Target" })).toBeEnabled(); + await user.clear(nameInput); + await pasteInto(user, nameInput, "openai_chat_gpt_4o"); + expect(nameInput).toHaveAccessibleDescription(expect.stringContaining(detail)); + expect(screen.getByRole("button", { name: "Save Target" })).toBeDisabled(); + }); + + it("should let the user save again when a refusal arrives after the form was edited", async () => { + const user = userEvent.setup(); + let rejectCreate: ((error: Error) => void) | null = null; + mockedTargetsApi.createTarget.mockImplementationOnce(() => new Promise((_, reject) => { + rejectCreate = reject; + })); + render( + + + + ); + + await fillAzureOpenAiChat(user); + await user.click(screen.getByRole("button", { name: "Save Target" })); + await user.type(screen.getByRole("textbox", { name: /Model \/ Deployment Name/ }), "-mini"); + if (rejectCreate === null) { + throw new Error("The create request was not sent"); + } + await act(async () => { + rejectCreate(axiosError(400, { + type: "/errors/not-saveable", + detail: "Refused. It can still be created without saving it.", + })); + }); + + expect(screen.queryByText("This target can't be saved")).not.toBeInTheDocument(); + expect(screen.getByRole("button", { name: "Save Target" })).toBeEnabled(); + }); + + it("should show a taken name on the name field and hold saving until it is edited", async () => { + const user = userEvent.setup(); + const detail = "A saved target named 'openai_chat_gpt_4o' already exists."; + mockedTargetsApi.createTarget.mockRejectedValueOnce(axiosError(409, { detail })); + render( + + + + ); + + await fillAzureOpenAiChat(user); + await user.click(screen.getByRole("button", { name: "Save Target" })); + + const nameInput = screen.getByRole("textbox", { name: /Registry Name/ }); + await waitFor(() => expect(nameInput).toHaveAccessibleDescription(expect.stringContaining(detail))); + expect(nameInput).toHaveFocus(); + expect(screen.getByRole("button", { name: "Save Target" })).toBeDisabled(); + fireEvent.submit(screen.getByTestId("create-target-form")); + expect(mockedTargetsApi.createTarget).toHaveBeenCalledTimes(1); + await user.type(nameInput, "_2"); + + expect(screen.queryByText(detail)).not.toBeInTheDocument(); + expect(screen.getByRole("button", { name: "Save Target" })).toBeEnabled(); + }); + + it("should keep a typed conflict such as a backend update out of the name field", async () => { + const user = userEvent.setup(); + const detail = "The backend was updated. Reload the page to continue."; + mockedTargetsApi.createTarget.mockRejectedValueOnce( + axiosError(409, { type: "urn:pyrit:compatibility:mismatch", detail }), + ); + render( + + + + ); + + await fillAzureOpenAiChat(user); + await user.click(screen.getByRole("button", { name: "Save Target" })); + + expect(await screen.findByText(detail)).toBeInTheDocument(); + const nameInput = screen.getByRole("textbox", { name: /Registry Name/ }); + expect(nameInput).not.toHaveAccessibleDescription(expect.stringContaining(detail)); + expect(nameInput).not.toHaveFocus(); + }); + + it("should save a round robin over targets that come back after a restart", async () => { + const user = userEvent.setup(); + render( + + + + ); + + await selectTargetType("RoundRobinTarget"); + const select = screen.getByText("Select a target to add...").closest("select")!; + await user.selectOptions(select, "saved_chat"); + await user.selectOptions(select, "startup_chat"); + await user.click(screen.getByRole("button", { name: "Save Target" })); + + await waitFor(() => expect(defaultProps.onCreated).toHaveBeenCalledTimes(1)); + expect(mockedTargetsApi.createTarget).toHaveBeenCalledWith({ + name: "weighted_round_robin", + type: "RoundRobinTarget", + params: { targets: ["saved_chat", "startup_chat"], weights: [1, 1] }, + persist: true, + }); + }); + + it("should mark Temporary round-robin members and offer Temporary for the round robin", async () => { + const user = userEvent.setup(); + render( + + + + ); + + await selectTargetType("RoundRobinTarget"); + const select = screen.getByText("Select a target to add...").closest("select")!; + expect(screen.getByRole("option", { name: "temp_chat — OpenAIChatTarget (gpt-4o) · Temporary" })).toBeInTheDocument(); + await user.selectOptions(select, "saved_chat"); + await user.selectOptions(select, "temp_chat"); + + expect(screen.getByLabelText("Selected target: saved_chat (gpt-4o)")).toBeInTheDocument(); + expect(screen.getByLabelText("Selected target: temp_chat (gpt-4o) · Temporary")).toBeInTheDocument(); + expect( + await screen.findByText(/Uses temporary target 'temp_chat', which won't exist after a restart\./), + ).toBeInTheDocument(); + await user.click(screen.getByRole("button", { name: "Create as Temporary" })); + + await waitFor(() => expect(defaultProps.onCreated).toHaveBeenCalledTimes(1)); + expect(mockedTargetsApi.createTarget).toHaveBeenCalledWith({ + name: "weighted_round_robin", + type: "RoundRobinTarget", + params: { targets: ["saved_chat", "temp_chat"], weights: [1, 1] }, + persist: false, + }); + }); +}); diff --git a/frontend/src/components/Config/CreateTargetDialog.tsx b/frontend/src/components/Config/CreateTargetDialog.tsx index 9298b7ee4f..cb462c9293 100644 --- a/frontend/src/components/Config/CreateTargetDialog.tsx +++ b/frontend/src/components/Config/CreateTargetDialog.tsx @@ -22,6 +22,7 @@ import { Field, MessageBar, MessageBarBody, + MessageBarTitle, Tooltip, } from '@fluentui/react-components' import { DeleteRegular } from '@fluentui/react-icons' @@ -32,9 +33,10 @@ import { type ParameterFormValue, } from '@/components/Parameters/parameterForm' import { targetsApi } from '@/services/api' -import { toApiError } from '@/services/errors' +import { isNotSaveableError, toApiError } from '@/services/errors' import { listRegisteredTargets } from '@/services/targetRegistry' import type { TargetAuthMode, TargetInstance, TargetTypeEntry } from '@/types' +import { suggestRegistryName, validateRegistryName } from '@/utils/registryName' import { isGuiTarget } from '@/utils/targetVisibility' import { targetIdentifierHash, @@ -42,6 +44,7 @@ import { targetType as getTargetType, targetUnderlyingModelName, } from '@/utils/targetIdentity' +import { TARGET_ORIGIN_LABELS } from '@/utils/targetOrigin' import { useCreateTargetDialogStyles } from './CreateTargetDialog.styles' import { canConfigureTargetType, @@ -141,10 +144,20 @@ const AUTH_MODE_LABELS: Record = { none: 'No key', } +// A new target starts with the first of these that a saved target of its type can use. +const SAVEABLE_AUTH_MODE_PREFERENCE: readonly TargetAuthMode[] = ['identity', 'none'] + +const TEMPORARY_TARGET_NOTE = 'A Temporary target is gone after the backend restarts or is reinitialized.' + function getTargetDisplayName(targetType: string): string { return TARGET_DISPLAY_NAMES[targetType] ?? targetType } +/** The text a suggested registry name is built from: the type's display name, or its class name without "Target". */ +function getTargetNameLabel(targetType: string): string { + return TARGET_DISPLAY_NAMES[targetType] ?? targetType.replace(/Target$/, '') +} + function getAuthDescription(entry: TargetTypeEntry, registryMetadataLoaded: boolean): string | null { if (entry.target_type === 'GitHubCopilotTarget') { return 'Supported authentication: GitHub token' @@ -240,6 +253,18 @@ interface SelectedInnerTarget { weightInput: string } +/** A save the backend refused, for the form values it refused. */ +interface SaveRefusal { + readonly formKey: string + readonly reason: string +} + +/** A name the backend said is taken. */ +interface NameConflict { + readonly name: string + readonly detail: string +} + /** * Resolve the effective underlying model for compatibility checks. * @@ -282,10 +307,13 @@ export default function CreateTargetDialog({ open, onClose, onCreated, existingT const styles = useCreateTargetDialogStyles() const [targetType, setTargetType] = useState('') const [endpoint, setEndpoint] = useState('') + // The Entra ID endpoint error waits until the endpoint field is left, so it doesn't flash while a host is typed. + const [endpointFocused, setEndpointFocused] = useState(false) const [modelName, setModelName] = useState('') const [hasDifferentUnderlying, setHasDifferentUnderlying] = useState(false) const [underlyingModel, setUnderlyingModel] = useState('') - const [authMode, setAuthMode] = useState('api_key') + // The auth mode the user picked; until then the dialog uses the default for the selected type. + const [chosenAuthMode, setChosenAuthMode] = useState(null) const [apiKey, setApiKey] = useState('') const [parameterValues, setParameterValues] = useState>({}) const [submitting, setSubmitting] = useState(false) @@ -300,6 +328,12 @@ export default function CreateTargetDialog({ open, onClose, onCreated, existingT underlyingModel?: string apiKey?: string }>({}) + const [registryName, setRegistryName] = useState('') + // Until the user edits the name, the field shows a suggestion instead. + const [nameEdited, setNameEdited] = useState(false) + const [nameConflict, setNameConflict] = useState(null) + const [saveRefusal, setSaveRefusal] = useState(null) + const nameInputRef = useRef(null) useEffect(() => { if (httpRequestError) { @@ -420,16 +454,31 @@ export default function CreateTargetDialog({ open, onClose, onCreated, existingT ? targetTypeEntry.supported_auth_modes.includes('none') : defaultSupportsNoKey(targetType) const showAuthField = targetType !== '' && (supportsIdentity || supportsNoKey) + // Default to a mode a saved target of this type can use. It is derived, so it also applies when the type's + // details arrive after the type was chosen. + const defaultAuthMode = SAVEABLE_AUTH_MODE_PREFERENCE.find( + (mode) => targetTypeEntry?.saveable_auth_modes?.includes(mode), + ) ?? 'api_key' + const chosenAuthModeSupported = chosenAuthMode === 'api_key' + || (chosenAuthMode === 'identity' && supportsIdentity) + || (chosenAuthMode === 'none' && supportsNoKey) + const authMode = chosenAuthMode !== null && chosenAuthModeSupported ? chosenAuthMode : defaultAuthMode const isIdentity = showAuthField && authMode === 'identity' const isNoKey = showAuthField && authMode === 'none' const endpointRequired = Boolean(endpointParameter?.required) || (metadataUnavailableForSelection && hasEndpointField) - const modelNameRequired = Boolean(modelNameParameter?.required) + // Without the type's details, still require the model every OpenAI type needs. The suggested name then includes + // it, which keeps it off names such as `openai_chat` that startup initializers use. + const modelNameRequired = modelNameParameter + ? Boolean(modelNameParameter.required) + : metadataUnavailableForSelection && isOpenAi const underlyingModelRequired = Boolean(underlyingModelParameter?.required) const apiKeyRequired = Boolean(apiKeyParameter?.required) && !isIdentity && !isNoKey const identityEndpointError: string | null = (() => { if (!isIdentity || endpoint === '') return null if (isOpenAi && !isAzureOpenAiEndpoint(endpoint)) { + const noKeyHint = supportsNoKey ? ' For a server that needs no key, such as a local one, choose No key.' : '' return 'Identity-based auth only works with Azure OpenAI / AI Foundry endpoints (for example, *.openai.azure.com or *.ai.azure.com).' + + noKeyHint } if (isAzureML && !isAzureMlEndpoint(endpoint)) { return 'Identity-based auth for AzureMLChatTarget only works with Azure ML managed online endpoints (for example, *.inference.ml.azure.com).' @@ -437,6 +486,74 @@ export default function CreateTargetDialog({ open, onClose, onCreated, existingT return null })() const showIdentityEndpointError = identityEndpointError !== null + // The parent's list is current; the round-robin list is a snapshot from when it was fetched. + const takenNames = useMemo( + () => new Set((existingTargets ?? availableTargets).map((t) => t.target_registry_name)), + [existingTargets, availableTargets], + ) + // Until edited, the name follows the type and model. That tells saved targets apart and leaves names such as + // `openai_chat` to the startup initializers, whose target would otherwise stop a saved one of that name restoring. + const suggestedName = targetType + ? suggestRegistryName(getTargetNameLabel(targetType), takenNames, hasModelNameField ? modelName : '') + : '' + const nameValue = nameEdited ? registryName : suggestedName + // The server stays authoritative; this only catches names already in the list the dialog has. + const nameTaken = takenNames.has(nameValue) + // A conflict applies to the name it was reported for, even if it arrives after the name was edited. + const nameConflictDetail = nameConflict?.name === nameValue ? nameConflict.detail : null + const nameMessage = nameConflictDetail + ?? (nameTaken ? `A target named '${nameValue}' already exists.` : null) + ?? (nameEdited ? validateRegistryName(nameValue) : null) + const nameBlocked = nameConflictDetail !== null || nameTaken || validateRegistryName(nameValue) !== null + const predictedTemporaryReason: string | null = (() => { + if (!targetType) return null + if (isRoundRobin) { + const temporaryMembers = selectedInnerTargets + .map((selected) => selected.registryName) + .filter((name) => availableTargets.find((t) => t.target_registry_name === name)?.origin === 'temporary') + if (temporaryMembers.length === 0) return null + return `Uses temporary ${temporaryMembers.length === 1 ? 'target' : 'targets'} ${ + temporaryMembers.map((name) => `'${name}'`).join(', ') + }, which won't exist after a restart.` + } + if (targetTypeEntry?.not_saveable_reason) return targetTypeEntry.not_saveable_reason + const saveableAuthModes = targetTypeEntry?.saveable_auth_modes + if (showAuthField && saveableAuthModes && !saveableAuthModes.includes(authMode)) { + const choices = saveableAuthModes.map((mode) => AUTH_MODE_LABELS[mode]).join(' or ') + const current = authMode === 'api_key' ? 'an API key' : AUTH_MODE_LABELS[authMode] + return `Saved targets can't use ${current} yet. Choose ${choices} to save this one.` + } + const typedCredential = hasApiKeyField && !isIdentity && !isNoKey && apiKey !== '' + ? 'api_key' + : metadataDrivenParameters.find((parameter) => ( + parameter.sensitive + && isParameterValueSet(parameterValues[parameter.name]) + && !(isIdentity && parameter.identity_conflicting) + ))?.name + if (typedCredential) return `Saved targets can't hold credentials ('${typedCredential}') yet.` + return null + })() + // A refused save only applies to the values it was refused for, so any edit lets the user try saving again. + const formKey = JSON.stringify([ + targetType, + endpoint, + modelName, + hasDifferentUnderlying, + underlyingModel, + authMode, + apiKey, + parameterValues, + selectedInnerTargets, + ]) + const refusedReason = saveRefusal?.formKey === formKey ? saveRefusal.reason : null + const temporaryReason = predictedTemporaryReason ?? refusedReason + const submitLabel = submitting ? 'Creating...' : (temporaryReason === null ? 'Save Target' : 'Create as Temporary') + const temporaryWarningRef = useRef(null) + + // The reason sits below the fields, so bring it into view when it appears or changes. + useEffect(() => { + if (temporaryReason) temporaryWarningRef.current?.scrollIntoView({ block: 'nearest' }) + }, [temporaryReason]) // Fetch the available targets when the dialog opens with RoundRobin selected. // If the parent already passed targets, derive availableTargets from them @@ -528,13 +645,18 @@ export default function CreateTargetDialog({ open, onClose, onCreated, existingT setModelName('') setHasDifferentUnderlying(false) setUnderlyingModel('') - setAuthMode('api_key') + setChosenAuthMode(null) setApiKey('') setParameterValues({}) setError(null) setHttpRequestError(null) setFieldErrors({}) setSelectedInnerTargets([]) + setEndpointFocused(false) + setRegistryName('') + setNameEdited(false) + setNameConflict(null) + setSaveRefusal(null) } const handleClose = () => { @@ -543,7 +665,36 @@ export default function CreateTargetDialog({ open, onClose, onCreated, existingT onClose() } + const handleCreateError = (err: unknown, persist: boolean) => { + const apiError = toApiError(err) + if (persist && isNotSaveableError(apiError)) { + setSaveRefusal({ formKey, reason: apiError.detail }) + return + } + // A taken name is a 409 without a problem type; a typed 409 is the build-identity mismatch the client handles. + if (apiError.status === 409 && apiError.type === undefined) { + setNameConflict({ name: nameValue, detail: apiError.detail }) + // A conflict for a name that was edited away stays hidden, so it doesn't take focus either. + if (nameInputRef.current?.value === nameValue) nameInputRef.current.focus() + return + } + // Surface the backend's RFC 7807 `detail` (e.g. RoundRobinTarget validation + // messages) rather than the generic axios "Request failed with status code 400". + setError(apiError.detail) + setHttpRequestError( + targetType === 'HTTPTarget' && apiError.status === 400 ? apiError.detail : null, + ) + } + const handleSubmit = async () => { + if (nameBlocked) { + // Keep the name shown, a suggestion included, so its message stays with it. + setRegistryName(nameValue) + setNameEdited(true) + return + } + const persist = temporaryReason === null + // For RoundRobinTarget, validation is different: we need ≥2 selected targets, not endpoint if (isRoundRobin) { if (selectedInnerTargets.length < 2) { @@ -568,18 +719,18 @@ export default function CreateTargetDialog({ open, onClose, onCreated, existingT try { await targetsApi.createTarget({ + name: nameValue, type: 'RoundRobinTarget', params: { targets: selectedInnerTargets.map((t) => t.registryName), weights: parsedWeights, }, + persist, }) resetForm() onCreated() } catch (err) { - // Surface the backend's RFC 7807 `detail` (e.g. RoundRobinTarget validation - // messages) rather than the generic axios "Request failed with status code 400". - setError(toApiError(err).detail) + handleCreateError(err, persist) } finally { setSubmitting(false) } @@ -640,22 +791,18 @@ export default function CreateTargetDialog({ open, onClose, onCreated, existingT } await targetsApi.createTarget({ + name: nameValue, type: targetType, params, ...(isIdentity ? { auth_mode: 'identity' as const } : {}), ...(isNoKey ? { auth_mode: 'none' as const } : {}), + persist, }) resetForm() onCreated() } catch (err) { - // Surface the backend's RFC 7807 `detail` (e.g. RoundRobinTarget validation - // messages) rather than the generic axios "Request failed with status code 400". - const apiError = toApiError(err) - setError(apiError.detail) - setHttpRequestError( - targetType === 'HTTPTarget' && apiError.status === 400 ? apiError.detail : null, - ) + handleCreateError(err, persist) } finally { setSubmitting(false) } @@ -712,26 +859,17 @@ export default function CreateTargetDialog({ open, onClose, onCreated, existingT value={targetType ? selectedTargetDisplayName : ''} onOptionSelect={(_, data) => { const next = data.optionValue - if (!next) return + // The picker also reports the type that is already selected, which must not clear what was typed. + if (!next || next === targetType) return setTargetType(next) + setFieldErrors((previous) => ({ ...previous, targetType: undefined })) setParameterValues({}) if (httpRequestError !== null) { setHttpRequestError(null) setError(null) } - const nextEntry = targetTypeByName.get(next) - const nextSupportsIdentity = nextEntry - ? nextEntry.supported_auth_modes.includes('identity') - : defaultSupportsIdentity(next) - const nextSupportsNoKey = nextEntry - ? nextEntry.supported_auth_modes.includes('none') - : defaultSupportsNoKey(next) - if ( - (authMode === 'identity' && !nextSupportsIdentity) - || (authMode === 'none' && !nextSupportsNoKey) - ) { - setAuthMode('api_key') - } + setChosenAuthMode(null) + setApiKey('') }} > {targetTypeOptions.map((entry) => { @@ -800,6 +938,24 @@ export default function CreateTargetDialog({ open, onClose, onCreated, existingT )} + + { + setRegistryName(data.value) + setNameEdited(true) + }} + /> + + {isGitHubCopilot && ( <> @@ -838,6 +994,7 @@ export default function CreateTargetDialog({ open, onClose, onCreated, existingT ))} @@ -860,7 +1017,7 @@ export default function CreateTargetDialog({ open, onClose, onCreated, existingT ) const selectedTargetLabel = `${target?.target_registry_name ?? sel.registryName}${ target && targetModelName(target) ? ` (${targetModelName(target)})` : '' - }` + }${target?.origin === 'temporary' ? ` · ${TARGET_ORIGIN_LABELS.temporary}` : ''}` const weightParse = parseWeight(sel.weightInput) const weightError = weightParse.ok ? null : weightParse.error return ( @@ -942,6 +1099,8 @@ export default function CreateTargetDialog({ open, onClose, onCreated, existingT : 'https://example.com/'} value={endpoint} onChange={(_, data) => setEndpoint(data.value)} + onFocus={() => setEndpointFocused(true)} + onBlur={() => setEndpointFocused(false)} /> )} @@ -1042,7 +1201,7 @@ export default function CreateTargetDialog({ open, onClose, onCreated, existingT value={authMode} onChange={(_, data) => { const next = data.value as TargetAuthMode - setAuthMode(next) + setChosenAuthMode(next) if (next === 'none') { setApiKey('') } @@ -1069,8 +1228,8 @@ export default function CreateTargetDialog({ open, onClose, onCreated, existingT )} - {showIdentityEndpointError && ( - + {showIdentityEndpointError && !endpointFocused && ( + {identityEndpointError} @@ -1089,7 +1248,10 @@ export default function CreateTargetDialog({ open, onClose, onCreated, existingT type="password" placeholder="API key (stored in memory only)" value={apiKey} - onChange={(_, data) => setApiKey(data.value)} + onChange={(_, data) => { + setApiKey(data.value) + setChosenAuthMode('api_key') + }} /> )} @@ -1137,6 +1299,15 @@ export default function CreateTargetDialog({ open, onClose, onCreated, existingT )} + {temporaryReason && ( + + + This target can't be saved + {temporaryReason} {TEMPORARY_TARGET_NOTE} + + + )} + {!isRoundRobin && (