From fe1da1ca25f20627d3b7f924c17936ec1bb0c1de Mon Sep 17 00:00:00 2001 From: Dotta Date: Tue, 6 Oct 2026 10:39:50 -0500 Subject: [PATCH] fix(connections): recover browser logins and isolate Hermes gateway keys Allow a personal gateway to be saved before a new agent exists. Update the new-agent connection selector regression. Co-Authored-By: Paperclip --- server/src/__tests__/ai-connections.test.ts | 16 ++++++++++++ .../src/__tests__/ai-provider-routing.test.ts | 9 +++++++ server/src/services/ai-provider-routing.ts | 5 ++-- .../services/local-ai-browser-login.test.ts | 10 ++++---- server/src/services/local-ai-login.ts | 6 ++++- .../ai-connections/AiProviderSetup.test.tsx | 25 +++++++++++++++++++ .../ai-connections/AiProviderSetup.tsx | 2 +- ui/src/pages/NewAgent.test.tsx | 17 ++++++++++--- 8 files changed, 78 insertions(+), 12 deletions(-) diff --git a/server/src/__tests__/ai-connections.test.ts b/server/src/__tests__/ai-connections.test.ts index 16e6de92737..13d9b17bf1b 100644 --- a/server/src/__tests__/ai-connections.test.ts +++ b/server/src/__tests__/ai-connections.test.ts @@ -1,6 +1,7 @@ import { connectionIntentService } from "../services/connection-intents.js"; import { connectionIntentDeliveryService } from "../services/connection-intent-delivery.js"; import { issueRecoveryActionService } from "../services/issue-recovery-actions.js"; +import { localAiLoginService } from "../services/local-ai-login.js"; import * as localCredentials from "../services/local-ai-credentials.js"; import { afterAll, beforeAll, describe, expect, it, vi } from "vitest"; import { randomUUID } from "node:crypto"; @@ -793,6 +794,21 @@ console.log(JSON.stringify({ type: "result", subtype: "success", result: "hello" expect(network).not.toHaveBeenCalled(); } finally { network.mockRestore(); } }); + it.each(["anthropic", "openai"] as const)("reports a missing %s browser process after a server restart", async provider => { + const [environment] = await db.select().from(environments).where(eq(environments.driver, "local")).limit(1); + const id = randomUUID(); + const owner = `restart-${provider}`; + const intent = { provider, method: "subscription", ownership: "personal", name: "Interrupted sign-in", allAgents: false, agentIds: [] } as const; + await db.insert(adapterAuthSessions).values({ id, publicSessionId: id, companyId, environmentId: environment.id, startedByUserId: owner, adapterType: provider === "anthropic" ? "claude_local" : "codex_local", aiConnection: { ...intent, agentIds: [] }, connectionMethod: "local_subscription", status: "waiting_for_user", expiresAt: new Date(Date.now() + 60_000) }); + const reader = vi.spyOn(localCredentials, "readVerifiedLocalAiCredential").mockRejectedValue(new Error("No credential yet")); + try { + const login = localAiLoginService(db); + await expect(login.check(companyId, owner, { ...intent, agentIds: [] }, id)).resolves.toEqual({ status: "sign_in_required", error: "The server restarted during sign-in. Start sign-in again." }); + await expect(login.check(companyId, "bob", { ...intent, agentIds: [] }, id)).rejects.toThrow("not found"); + reader.mockResolvedValue("completed-before-restart"); + await expect(login.check(companyId, owner, { ...intent, agentIds: [] }, id)).resolves.toEqual({ status: "ready" }); + } finally { reader.mockRestore(); await db.delete(adapterAuthSessions).where(eq(adapterAuthSessions.id, id)); } + }); it("requires an owned browser sign-in attempt for local subscriptions", async () => { const reader = vi.spyOn(localCredentials, "readVerifiedLocalAiCredential").mockResolvedValue("fixture-local-token"); const app = express(); diff --git a/server/src/__tests__/ai-provider-routing.test.ts b/server/src/__tests__/ai-provider-routing.test.ts index d67dcd8e1ea..4e46b0f80d3 100644 --- a/server/src/__tests__/ai-provider-routing.test.ts +++ b/server/src/__tests__/ai-provider-routing.test.ts @@ -84,6 +84,15 @@ describe("provider routing", () => { ), ).toBe(false); }); + it.each(["gateway", "openrouter"] as const)("uses a terminal-filtered secret variable for Hermes %s authentication", kind => { + const route = aiProviderRoutingSchema.parse({ kind, protocol: "chat", auth: "bearer", ...(kind === "gateway" ? { baseUrl: "https://gateway.example/v1" } : {}) }); + const projected = managedProviderRouting(route, "hermes_local", "selected-key", "fixture-model"); + expect(projected.env.OPENAI_API_KEY).toBe("selected-key"); + expect(projected.env.PAPERCLIP_AI_PROVIDER_KEY).toBeUndefined(); + expect(projected.hermesConfig).toContain('api_key: "${OPENAI_API_KEY}"'); + expect(projected.hermesConfig).not.toContain("selected-key"); + expect(Object.entries(projected.env).filter(([, value]) => value === "selected-key").map(([name]) => name)).toEqual(kind === "openrouter" ? ["OPENAI_API_KEY", "OPENROUTER_API_KEY"] : ["OPENAI_API_KEY"]); + }); it("routes OpenRouter through each harness's protocol without putting secrets in Codex config", () => { const routing = aiProviderRoutingSchema.parse({ kind: "openrouter", diff --git a/server/src/services/ai-provider-routing.ts b/server/src/services/ai-provider-routing.ts index 5ce3e759eb8..33ceffb3366 100644 --- a/server/src/services/ai-provider-routing.ts +++ b/server/src/services/ai-provider-routing.ts @@ -68,8 +68,9 @@ export function managedProviderRouting( env.OPENROUTER_API_KEY = route.kind === "openrouter" ? credential : ""; config.provider = route.kind === "openrouter" ? "openrouter" : "auto"; // Hermes now reads custom endpoint routing from config.yaml, not OPENAI_BASE_URL. - env.PAPERCLIP_AI_PROVIDER_KEY = credential; - hermesConfig = `model:\n provider: ${route.kind === "openrouter" ? '"openrouter"' : '"custom"'}\n default: ${JSON.stringify(model)}\n base_url: ${JSON.stringify(baseUrl)}\n api_mode: "chat_completions"\n${route.auth === "none" ? "" : ' api_key: "${PAPERCLIP_AI_PROVIDER_KEY}"\n'}`; + // Hermes strips OPENAI_API_KEY from terminal children. A custom variable + // would expose the reusable gateway key to task-controlled shell commands. + hermesConfig = `model:\n provider: ${route.kind === "openrouter" ? '"openrouter"' : '"custom"'}\n default: ${JSON.stringify(model)}\n base_url: ${JSON.stringify(baseUrl)}\n api_mode: "chat_completions"\n${route.auth === "none" ? "" : ' api_key: "${OPENAI_API_KEY}"\n'}`; } return { env, config, codexConfig, hermesConfig }; } diff --git a/server/src/services/local-ai-browser-login.test.ts b/server/src/services/local-ai-browser-login.test.ts index 0f8085314c7..410cc02d0ee 100644 --- a/server/src/services/local-ai-browser-login.test.ts +++ b/server/src/services/local-ai-browser-login.test.ts @@ -61,8 +61,8 @@ describe.skipIf(process.platform === "win32")("local browser subscription login" let pid = 0; await vi.waitFor(async () => { pid = Number(await readFile(path.join(home, "login.pid"), "utf8")); expect(pid).toBeGreaterThan(0); }); login.abort(); - await vi.waitFor(() => expect(login.outcome).toBe("failure")); - await vi.waitFor(() => expect(() => process.kill(pid, 0)).toThrow()); + await vi.waitFor(() => expect(login.outcome).toBe("failure"), { timeout: 5000 }); + await vi.waitFor(() => expect(() => process.kill(pid, 0)).toThrow(), { timeout: 5000 }); }); it("cancels Claude while waiting for its browser code and terminates the provider", async () => { @@ -77,15 +77,15 @@ describe.skipIf(process.platform === "win32")("local browser subscription login" await vi.waitFor(() => expect(login.authorizationUrl).toBeDefined()); const pid = Number(await readFile(path.join(home, "login.pid"), "utf8")); login.abort(); - await vi.waitFor(() => expect(login.outcome).toBe("failure")); - await vi.waitFor(() => expect(() => process.kill(pid, 0)).toThrow()); + await vi.waitFor(() => expect(login.outcome).toBe("failure"), { timeout: 5000 }); + await vi.waitFor(() => expect(() => process.kill(pid, 0)).toThrow(), { timeout: 5000 }); await expect(readFile(path.join(home, ".credentials.json"))).rejects.toThrow(); }); it("returns a fixed failure without exposing provider error output", async () => { const home = await fakeCli("codex", 'echo "private-provider-error" >&2\nexit 1'); const login = startLocalBrowserLogin("openai", home); - await vi.waitFor(() => expect(login.outcome).toBe("failure")); + await vi.waitFor(() => expect(login.outcome).toBe("failure"), { timeout: 5000 }); expect(JSON.stringify(login)).not.toContain("private-provider-error"); expect(login.authorizationUrl).toBeUndefined(); }); diff --git a/server/src/services/local-ai-login.ts b/server/src/services/local-ai-login.ts index 140d64480f6..3492d47896a 100644 --- a/server/src/services/local-ai-login.ts +++ b/server/src/services/local-ai-login.ts @@ -168,7 +168,11 @@ export function localAiLoginService(db: Db) { } catch { const login = id ? browserLogins.get(id) : undefined; return { status: "sign_in_required", authorizationUrl: login?.authorizationUrl, code: login?.code, - ...(login?.outcome === "failure" ? { error: "Sign-in ended before the account was connected. Start sign-in again." } : {}) }; + ...(!login && intent.provider !== "xai" + ? { error: "The server restarted during sign-in. Start sign-in again." } + : login?.outcome === "failure" + ? { error: "Sign-in ended before the account was connected. Start sign-in again." } + : {}) }; } } diff --git a/ui/src/components/ai-connections/AiProviderSetup.test.tsx b/ui/src/components/ai-connections/AiProviderSetup.test.tsx index eb60c665dad..96e04233935 100644 --- a/ui/src/components/ai-connections/AiProviderSetup.test.tsx +++ b/ui/src/components/ai-connections/AiProviderSetup.test.tsx @@ -97,3 +97,28 @@ it("saves the default organization grant and all-agent installation from the dir await act(async () => root.unmount()); client.clear(); } }); + + +it("allows an ordinary member to save a personal gateway before the agent exists", async () => { + const client = new QueryClient({ defaultOptions: { queries: { retry: false, staleTime: Infinity } } }); + client.setQueryData(["ai-connections", "company", undefined], { canManageConnections: false, connections: [] }); + client.setQueryData(["agents", "company", "provider-access"], []); + const container = document.createElement("div"); + const root = createRoot(container); + const onComplete = vi.fn(); + try { + await act(async () => root.render( {}} onComplete={onComplete} />)); + const input = container.querySelector('input[aria-label="API key"]')!; + await act(async () => { + Object.getOwnPropertyDescriptor(HTMLInputElement.prototype, "value")!.set!.call(input, "member-key"); + input.dispatchEvent(new Event("input", { bubbles: true })); + }); + const connect = container.querySelector('button[type="submit"]') as HTMLButtonElement; + expect(connect.disabled).toBe(false); + await act(async () => container.querySelector("form")!.dispatchEvent(new Event("submit", { bubbles: true, cancelable: true }))); + expect(aiConnectionsApi.create).toHaveBeenCalledWith("company", expect.objectContaining({ ownership: "personal", allAgents: false, agentIds: [], apiKey: "member-key" })); + expect(onComplete).toHaveBeenCalledWith(expect.objectContaining({ mode: "delegated", connectionId: "connection", grantId: "grant" })); + } finally { + await act(async () => root.unmount()); client.clear(); + } +}); diff --git a/ui/src/components/ai-connections/AiProviderSetup.tsx b/ui/src/components/ai-connections/AiProviderSetup.tsx index 76dfc0388d5..13de861d187 100644 --- a/ui/src/components/ai-connections/AiProviderSetup.tsx +++ b/ui/src/components/ai-connections/AiProviderSetup.tsx @@ -450,7 +450,7 @@ export function AiProviderSetup({ type="submit" disabled={ save.isPending || accounts.isPending || accounts.isError || - (!reconnect && !allAgents && agentIds.size === 0) || + (!reconnect && ownership === "shared" && !allAgents && agentIds.size === 0) || (auth !== "none" && !apiKey.trim()) } > diff --git a/ui/src/pages/NewAgent.test.tsx b/ui/src/pages/NewAgent.test.tsx index c1b3eccb56e..32b4906036f 100644 --- a/ui/src/pages/NewAgent.test.tsx +++ b/ui/src/pages/NewAgent.test.tsx @@ -528,11 +528,22 @@ describe("New agent setup", () => { await render("opencode_local"); const model = "openrouter/anthropic/claude-sonnet-4.6"; await fill("Model", model); - await click("Connect another account"); + const connectionSelect = container.querySelector('[role="combobox"][aria-label="Connection"]')!; + await act(async () => connectionSelect.dispatchEvent(new KeyboardEvent("keydown", { key: "ArrowDown", bubbles: true }))); + await settle(); + const connectOption = [...document.querySelectorAll('[role="option"]')].find(option => option.textContent?.includes("Connect an account"))!; + expect(connectOption).toBeTruthy(); + await act(async () => (connectOption as HTMLElement).click()); + await settle(); const dialog = document.querySelector('[role="dialog"]')!; expect(dialog).toBeTruthy(); expect(api.hire).not.toHaveBeenCalled(); expect(api.testEnvironment).not.toHaveBeenCalled(); + const advanced = [...dialog.querySelectorAll("summary")].find(node => node.textContent?.includes("Advanced providers"))!; + await act(async () => advanced.click()); + const openrouter = [...dialog.querySelectorAll("button")].find(node => node.textContent?.includes("OpenRouter"))!; + await act(async () => openrouter.click()); + await settle(); const input = dialog.querySelector('[aria-label="API key"]') as HTMLInputElement; expect(input).toBeTruthy(); await act(async () => { @@ -544,11 +555,11 @@ describe("New agent setup", () => { await act(async () => connectButton.click()); await settle(); expect(document.querySelector('[role="dialog"]')).toBeNull(); - expect(managedApi.setDefault).toHaveBeenCalledWith("company-1", "managed-grant"); + expect(managedApi.setDefault).not.toHaveBeenCalled(); expect(managedApi.create).toHaveBeenCalledWith("company-1", expect.objectContaining({ provider: "openrouter", method: "api_key", apiKey: "example-test-secret", })); - const binding = { provider: "openrouter", method: "api_key", mode: "responsible_user" }; + const binding = { provider: "openrouter", method: "api_key", mode: "delegated", connectionId: "managed-connection", grantId: "managed-grant" }; await click("Run test"); expect(api.testEnvironment.mock.calls[0][2]).toEqual(expect.objectContaining({ aiConnection: binding, testCredentials: {},