From 4c41e57eda6f38d08ddc30e14c5d0b4aa19d251d Mon Sep 17 00:00:00 2001 From: Dotta Date: Fri, 2 Oct 2026 15:25:05 -0500 Subject: [PATCH] Harden provider reconnect and preserve native account recovery Compare routed destinations structurally after JSONB persistence. Preserve native default selection and reconnect, respect provider access permissions, and normalize absent verification artifacts at the provider boundary. Co-Authored-By: Paperclip --- doc/connections/AI-CONNECTIONS.md | 5 ++++ .../src/contracts/completion-result.test.ts | 13 ++++++++++ .../src/contracts/completion-result.ts | 5 +++- server/src/__tests__/ai-connections.test.ts | 18 +++++++++++++ .../src/services/ai-connection-usage.test.ts | 2 +- server/src/services/ai-connection-usage.ts | 2 +- server/src/services/ai-connections.ts | 3 ++- .../ai-connections/AiConnectionField.test.tsx | 26 +++++++++++++------ .../ai-connections/AiConnectionField.tsx | 4 +-- .../ai-connections/AiConnectionSelect.tsx | 4 +++ .../ai-connections/AiProviderSetup.tsx | 11 +++++++- ui/src/pages/NewAgent.test.tsx | 2 +- ...provider-routing-06-production.stories.tsx | 14 +++++++++- 13 files changed, 92 insertions(+), 17 deletions(-) diff --git a/doc/connections/AI-CONNECTIONS.md b/doc/connections/AI-CONNECTIONS.md index 4a648d0beb2..8c24a3d06de 100644 --- a/doc/connections/AI-CONNECTIONS.md +++ b/doc/connections/AI-CONNECTIONS.md @@ -588,5 +588,10 @@ probe isolation, and new-runner home/environment boundaries. The isolated local test-drive exercised live OpenRouter requests using the Codex and Claude CLI probes, then completed real tasks using Codex, Claude, and OpenCode New Runner. The app walkthrough verified connection selection, saving, and completed tasks. +A follow-up Codex/OpenRouter acceptance test ran a shell calculation, completed +the task, then resumed from a new user message and completed a second shell +calculation with the prior context. Reconnect coverage round-trips routing +through PostgreSQL JSONB and verifies that credential rotation retains identity +and agent access. Bedrock and private gateway credentials were not available for live verification; their mapping and validation are covered by deterministic tests. diff --git a/packages/paperclip-runner/src/contracts/completion-result.test.ts b/packages/paperclip-runner/src/contracts/completion-result.test.ts index 7fd02435bac..e5a5a082378 100644 --- a/packages/paperclip-runner/src/contracts/completion-result.test.ts +++ b/packages/paperclip-runner/src/contracts/completion-result.test.ts @@ -1,3 +1,4 @@ +import { normalizeLegacyPrpStructuredRunResult } from "../protocol/result-normalization.js"; import Ajv2020 from "ajv/dist/2020.js"; import { describe, expect, it } from "vitest"; import { @@ -186,6 +187,18 @@ describe("provider-neutral completion result schema", () => { expect(providerValidate(providerResult)).toBe(true); }); + it.each([undefined, null, "", " ", "artifact:verified-result"])("normalizes optional verification artifact metadata (%s)", (artifactRef) => { + const providerValidate = new Ajv2020({ strict: false }).compile(PRP_COMPLETION_RESULT_PROVIDER_INPUT_SCHEMA); + const input = { ...structuredClone(baseResult), verification: [{ commandOrCheck: "Check answer", status: "passed", artifactRef }] }; + expect(providerValidate(input)).toBe(true); + const normalized = normalizeLegacyPrpStructuredRunResult(input); + expect(validate(normalized)).toBe(true); + expect(normalized).toMatchObject({ verification: [{ commandOrCheck: "Check answer", status: "passed" }] }); + if (artifactRef?.trim()) expect(normalized).toHaveProperty("verification.0.artifactRef", artifactRef); + else expect(normalized).not.toHaveProperty("verification.0.artifactRef"); + expect(providerValidate({ ...input, verification: [{ ...input.verification[0], artifactRef: 42 }] })).toBe(false); + }); + it("requires a reason code for verification that was not run", () => { const result = structuredClone(baseResult); result.verification = [{ commandOrCheck: "Run tests", status: "not_run" } as never]; diff --git a/packages/paperclip-runner/src/contracts/completion-result.ts b/packages/paperclip-runner/src/contracts/completion-result.ts index 6beca921f2b..89342a5f0a9 100644 --- a/packages/paperclip-runner/src/contracts/completion-result.ts +++ b/packages/paperclip-runner/src/contracts/completion-result.ts @@ -268,7 +268,10 @@ const providerVerificationCompatibilitySchema = { detail: { type: "string" }, result: { type: "string" }, cwd: { type: "string" }, - artifactRef: { type: "string", minLength: 1 }, + artifactRef: { + type: ["string", "null"], + description: "Reference to a real verification artifact. Omit or use null when no artifact exists; empty values are normalized away.", + }, }, }, } as const; diff --git a/server/src/__tests__/ai-connections.test.ts b/server/src/__tests__/ai-connections.test.ts index 72e98b9f6fd..4cda599af6e 100644 --- a/server/src/__tests__/ai-connections.test.ts +++ b/server/src/__tests__/ai-connections.test.ts @@ -144,6 +144,24 @@ describe("managed AI connections", () => { await db.update(connectionGrants).set({ status: "revoked" }).where(eq(connectionGrants.id, saved.grantId)); await expect(service.select({ ...input, binding: selected, userId: "alice", adapterType: "codex_local", model: "openai/gpt-5.4" })).rejects.toThrow("Reconnect"); }); + it("reconnects JSONB routing without changing its identity or access", async () => { + const routing = { kind: "gateway", protocol: "responses", auth: "bearer", baseUrl: "https://gateway.example/v1", models: [{ id: "gateway-model" }] } as const; + const input = { provider: "openai", method: "api_key", name: "Reconnect gateway", ownership: "personal", allAgents: false, agentIds: [agentId], routing: { ...routing, models: [...routing.models] } } as const; + const saved = await service.save(companyId, "alice", { ...input, agentIds: [...input.agentIds] }, "old-gateway-credential"); + const [stored] = await db.select().from(toolConnections).where(eq(toolConnections.id, saved.connectionId)); + expect(stored!.config.ai).toMatchObject({ routing }); + // PostgreSQL JSONB reorders object keys; compare values, not serialization. + const reordered = { models: [...routing.models], auth: routing.auth, baseUrl: routing.baseUrl, protocol: routing.protocol, kind: routing.kind }; + expect(await service.save(companyId, "alice", { ...input, agentIds: [], allAgents: true, connectionId: saved.connectionId, routing: reordered }, "new-gateway-credential")).toEqual(saved); + const selection = { companyId, agentId, userId: "alice", adapterType: "codex_local", binding: { provider: "openai", method: "api_key", mode: "delegated", ...saved } } as const; + const selected = await service.select(selection); + expect(await service.credential(selected)).toBe("new-gateway-credential"); + const installs = await db.select().from(toolConnectionInstalls).where(eq(toolConnectionInstalls.connectionId, saved.connectionId)); + expect(installs).toEqual([expect.objectContaining({ targetType: "agent", targetId: agentId })]); + await expect(service.save(companyId, "alice", { ...input, agentIds: [agentId], connectionId: saved.connectionId, routing: { ...reordered, baseUrl: "https://different.example/v1" } }, "rejected-credential")).rejects.toThrow("retain"); + expect(await service.credential(await service.select(selection))).toBe("new-gateway-credential"); + }); + it("saves no-auth endpoints without a secret and refuses protocol mismatches", async () => { const routing = { kind: "local", protocol: "chat", auth: "none", baseUrl: "http://localhost:11434/v1", models: [] } as const; const saved = await service.save(companyId, "alice", { provider: "openai", method: "api_key", name: "Local", ownership: "personal", allAgents: true, agentIds: [], routing: { ...routing, models: [] } }, ""); diff --git a/server/src/services/ai-connection-usage.test.ts b/server/src/services/ai-connection-usage.test.ts index cd4900d42e1..07862376afe 100644 --- a/server/src/services/ai-connection-usage.test.ts +++ b/server/src/services/ai-connection-usage.test.ts @@ -214,7 +214,7 @@ describe("connection usage probes", () => { expect(unlimited).toMatchObject({ status: "ok", overage: null, limits: [{ used: 20, limit: null, usedPercent: null, limitReached: null }] }); }); - it.each(["openai", "anthropic", "xai"] as const)("makes unsupported %s API probes explicit without a request", async provider => { + it.each(["openai", "anthropic", "xai", "google"] as const)("makes unsupported %s API probes explicit without a request", async provider => { const request = fixture({}); expect(await probeAiConnectionUsage({ provider, method: "api_key" }, "key", { request })).toMatchObject({ status: "unsupported", limits: [], overage: null }); expect(request).not.toHaveBeenCalled(); diff --git a/server/src/services/ai-connection-usage.ts b/server/src/services/ai-connection-usage.ts index 74619ef2fe2..c827a05e51a 100644 --- a/server/src/services/ai-connection-usage.ts +++ b/server/src/services/ai-connection-usage.ts @@ -332,7 +332,7 @@ export async function probeAiConnectionUsage( options: { request?: typeof fetch } = {}, ): Promise> { const base = { ...metadata, checkedAt: new Date().toISOString(), source: null, planType: null, limits: [], overage: null }; - if (!supportsAiConnectionUsage(metadata.provider, metadata.method)) { + if (metadata.provider === "google" || !supportsAiConnectionUsage(metadata.provider, metadata.method)) { return { ...base, status: "unsupported", errorCode: "unsupported", message: messages.unsupported }; } try { diff --git a/server/src/services/ai-connections.ts b/server/src/services/ai-connections.ts index d8098d6cd9a..68487354ccb 100644 --- a/server/src/services/ai-connections.ts +++ b/server/src/services/ai-connections.ts @@ -1,5 +1,6 @@ import { syncConnectionCredentialBindings } from "./connection-credential-bindings.js"; import { createHash, randomUUID } from "node:crypto"; +import { isDeepStrictEqual } from "node:util"; import { and, eq, inArray, or } from "drizzle-orm"; import { type Db, @@ -488,7 +489,7 @@ export function aiConnectionService(db: Db) { input.provider ) throw unprocessable("Reconnect cannot change providers"); - if (reconnect && JSON.stringify((reconnect.connection.config.ai as AiConnectionMetadata).routing) !== JSON.stringify(routing)) + if (reconnect && !isDeepStrictEqual((reconnect.connection.config.ai as AiConnectionMetadata).routing, routing)) throw unprocessable("Reconnect must retain this connection’s routing. Create another connection to change its destination."); if ( reconnect && diff --git a/ui/src/components/ai-connections/AiConnectionField.test.tsx b/ui/src/components/ai-connections/AiConnectionField.test.tsx index 7ab32e4afe0..d223009f054 100644 --- a/ui/src/components/ai-connections/AiConnectionField.test.tsx +++ b/ui/src/components/ai-connections/AiConnectionField.test.tsx @@ -44,7 +44,17 @@ async function mount(connections: AiManagedConnectionSummary[], canManageConnect )); await settle(); } -function click(label: string) { +async function click(label: string) { + if (label === "Connect another account") { + const trigger = document.querySelector('[role="combobox"][aria-label="Connection"]')!; + flushSync(() => trigger.dispatchEvent(new KeyboardEvent("keydown", { key: "ArrowDown", bubbles: true }))); + await settle(); + const option = Array.from(document.querySelectorAll('[role="option"]')).find(item => item.textContent?.includes("Connect an account")); + expect(option).toBeDefined(); + flushSync(() => option!.click()); + await settle(); + return; + } const button = Array.from(document.querySelectorAll("button")).find(item => item.textContent === label); expect(button, `Missing button: ${label}`).toBeDefined(); flushSync(() => button!.click()); @@ -62,7 +72,7 @@ afterEach(() => { flushSync(() => root.unmount()); client.clear(); container.rem it("reconnects the unavailable personal default in place", async () => { await mount([account()]); - click("Reconnect account"); + await click("Reconnect account"); expect(credentialProps).toMatchObject({ connectionId: "old-connection", initialMethod: "subscription", fixedMethod: true }); credentialProps!.onComplete({ connectionId: "old-connection", grantId: "old-grant", method: "subscription" }); await settle(); @@ -72,7 +82,7 @@ it("reconnects the unavailable personal default in place", async () => { it("selects the returned new grant and actual method before adopting the personal default", async () => { await mount([account()]); - click("Connect another account"); + await click("Connect another account"); expect(document.body.textContent).toContain("default"); expect(credentialProps).toMatchObject({ connectionId: undefined, allAgents: true }); let resolveDefault!: () => void; @@ -88,7 +98,7 @@ it("selects the returned new grant and actual method before adopting the persona it("lets the owner limit a new personal connection to this agent", async () => { await mount([]); - click("Connect another account"); + await click("Connect another account"); const checkbox = document.querySelector('[role="checkbox"]')!; expect(checkbox).not.toBeNull(); expect(checkbox.getAttribute("aria-checked")).toBe("true"); @@ -98,13 +108,13 @@ it("lets the owner limit a new personal connection to this agent", async () => { it("keeps default-update failures visible and retries without another provider login", async () => { await mount([account()]); - click("Connect another account"); + await click("Connect another account"); mocks.setDefault.mockRejectedValueOnce(new Error("Default update failed")); credentialProps!.onComplete({ connectionId: "new-connection", grantId: "new-grant", method: "api_key" }); await settle(); expect(document.body.textContent).toContain("Default update failed"); expect(onChange).not.toHaveBeenCalled(); - click("Retry default selection"); + await click("Retry default selection"); await settle(); expect(mocks.setDefault).toHaveBeenCalledTimes(2); expect(onChange).toHaveBeenCalledWith({ provider: "anthropic", method: "api_key", mode: "responsible_user" }); @@ -117,12 +127,12 @@ it("does not offer reconnection for a healthy default or another owner's account it("keeps an ordinary member's new connection scoped to the current agent by default", async () => { await mount([], false); - click("Connect another account"); + await click("Connect another account"); expect(credentialProps).toMatchObject({ allAgents: false, agentIds: ["agent"] }); }); it("uses the server's connection-manager permission for company-wide access", async () => { await mount([], true); - click("Connect another account"); + await click("Connect another account"); expect(credentialProps).toMatchObject({ allAgents: true }); }); diff --git a/ui/src/components/ai-connections/AiConnectionField.tsx b/ui/src/components/ai-connections/AiConnectionField.tsx index cc524512d86..7835b2bc4a6 100644 --- a/ui/src/components/ai-connections/AiConnectionField.tsx +++ b/ui/src/components/ai-connections/AiConnectionField.tsx @@ -97,7 +97,7 @@ export function AiConnectionField({ const openConnection = (reconnect?: AiManagedConnectionSummary) => { returnFocus.current = document.activeElement as HTMLElement; setReconnecting(reconnect); - setAdvancedSetup(false); + setAdvancedSetup(!reconnect && provider === "openrouter"); setAllAgents(accounts.data?.canManageConnections ?? false); setSavedAccount(undefined); selectDefault.reset(); @@ -211,7 +211,7 @@ export function AiConnectionField({ initialMethod={reconnecting?.method ?? method} fixedMethod={Boolean(reconnecting)} connectionId={reconnecting?.id} - name={reconnecting?.name ?? `My ${provider === "anthropic" ? "Claude" : provider === "openai" ? "OpenAI" : provider === "xai" ? "Grok" : "OpenRouter"} ${method === "subscription" ? "subscription" : "API"}`} + name={reconnecting?.name ?? `My ${provider === "anthropic" ? "Claude" : provider === "openai" ? "OpenAI" : provider === "xai" ? "Grok" : provider === "google" ? "Gemini" : "OpenRouter"} ${method === "subscription" ? "subscription" : "API"}`} ownership="personal" agentIds={agentId ? [agentId] : []} allAgents={allAgents} diff --git a/ui/src/components/ai-connections/AiConnectionSelect.tsx b/ui/src/components/ai-connections/AiConnectionSelect.tsx index bb418fb2c16..8bbfc080949 100644 --- a/ui/src/components/ai-connections/AiConnectionSelect.tsx +++ b/ui/src/components/ai-connections/AiConnectionSelect.tsx @@ -26,6 +26,7 @@ export function AiConnectionSelect({ error, onRetry, onConnect, + onReconnect, onChange, adapterType, }: AiConnectionPickerProps & { adapterType: string }) { @@ -148,6 +149,9 @@ export function AiConnectionSelect({ Connect an account… + {!readOnly && onReconnect && !loading && !error && ( + + )} {error && (

diff --git a/ui/src/components/ai-connections/AiProviderSetup.tsx b/ui/src/components/ai-connections/AiProviderSetup.tsx index 6e2874af8bc..e925d46e95f 100644 --- a/ui/src/components/ai-connections/AiProviderSetup.tsx +++ b/ui/src/components/ai-connections/AiProviderSetup.tsx @@ -99,7 +99,7 @@ export function AiProviderSetup({ const [ownership, setOwnership] = useState<"personal" | "shared">( reconnect?.ownership ?? "personal", ); - const [allAgents, setAllAgents] = useState(!agentId); + const [allAgentsChoice, setAllAgents] = useState(); const [agentIds, setAgentIds] = useState(new Set(agentId ? [agentId] : [])); const [protocol, setProtocol] = useState( reconnect?.routing?.protocol ?? "responses", @@ -119,6 +119,12 @@ export function AiProviderSetup({ const [secretAccessKey, setSecretAccessKey] = useState(""); const [sessionToken, setSessionToken] = useState(""); const client = useQueryClient(); + const accounts = useQuery({ + queryKey: ["ai-connections", companyId, agentId], + queryFn: () => aiConnectionsApi.list(companyId, agentId), + }); + const canManageConnections = accounts.data?.canManageConnections ?? false; + const allAgents = allAgentsChoice ?? (!agentId && canManageConnections); const agents = useQuery({ queryKey: ["agents", companyId, "provider-access"], queryFn: () => agentsApi.list(companyId), @@ -270,6 +276,7 @@ export function AiProviderSetup({ ) : step === "access" ? ( <> + {accounts.error &&

Could not load connection permissions.

} {agents.error && (

Could not load agents. Try again. @@ -277,6 +284,8 @@ export function AiProviderSetup({ )} ({ openNewIssue: vi.fn(), })); const managedApi = vi.hoisted(() => ({ - list: vi.fn(async () => ({ currentUserId: "user-1", connections: [] })), + list: vi.fn(async () => ({ currentUserId: "user-1", canManageConnections: true, connections: [] })), create: vi.fn(async () => ({ connectionId: "managed-connection", grantId: "managed-grant" })), setDefault: vi.fn(async () => ({})), })); diff --git a/ui/storybook/stories/provider-routing-06-production.stories.tsx b/ui/storybook/stories/provider-routing-06-production.stories.tsx index 7e71643c3a5..72c1cf1fcc4 100644 --- a/ui/storybook/stories/provider-routing-06-production.stories.tsx +++ b/ui/storybook/stories/provider-routing-06-production.stories.tsx @@ -54,7 +54,7 @@ const accounts: AiManagedConnectionSummary[] = [ status: "connected", }, ]; -function Providers() { +function Providers({ canManageConnections = true }: { canManageConnections?: boolean }) { const [client] = useState(() => { const query = new QueryClient({ defaultOptions: { queries: { retry: false, staleTime: Infinity } }, @@ -63,12 +63,14 @@ function Providers() { ["agents", companyId, "provider-access"], [{ id: "nova", name: "Nova" }], ); + query.setQueryData(["ai-connections", companyId, "nova"], { currentUserId: "you", connections: accounts, canManageConnections }); return query; }); return ( {}} onComplete={() => {}} /> @@ -151,3 +153,13 @@ export const CompatibleConnectionDropdown: Story = { ).toHaveTextContent("Company OpenRouter"); }, }; + +export const MemberProviderAccess: Story = { + render: () => , + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + await userEvent.click(canvas.getByRole("button", { name: "OpenAI" })); + await expect(canvas.getByText("Which agents can use this connection?", { exact: true })).toBeVisible(); + await expect(canvas.getByRole("button", { name: "Continue", exact: true })).toBeEnabled(); + }, +};