mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-21 08:22:20 +02:00
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 <noreply@paperclip.ing>
This commit is contained in:
1 parent
88fbc2c702
commit
4c41e57eda
13 files changed
+92
-17
No files matched your search
@@ -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.
|
||||
@@ -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];
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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: [] } }, "");
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -332,7 +332,7 @@ export async function probeAiConnectionUsage(
|
||||
options: { request?: typeof fetch } = {},
|
||||
): Promise<Omit<AiConnectionUsage, "connectionId" | "grantId">> {
|
||||
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 {
|
||||
|
||||
@@ -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 &&
|
||||
|
||||
@@ -44,7 +44,17 @@ async function mount(connections: AiManagedConnectionSummary[], canManageConnect
|
||||
</QueryClientProvider>));
|
||||
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<HTMLElement>('[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<HTMLButtonElement>('[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 });
|
||||
});
|
||||
@@ -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}
|
||||
|
||||
@@ -26,6 +26,7 @@ export function AiConnectionSelect({
|
||||
error,
|
||||
onRetry,
|
||||
onConnect,
|
||||
onReconnect,
|
||||
onChange,
|
||||
adapterType,
|
||||
}: AiConnectionPickerProps & { adapterType: string }) {
|
||||
@@ -148,6 +149,9 @@ export function AiConnectionSelect({
|
||||
<SelectItem value="connect">Connect an account…</SelectItem>
|
||||
</SelectContent>
|
||||
</Select>
|
||||
{!readOnly && onReconnect && !loading && !error && (
|
||||
<Button type="button" variant="outline" onClick={onReconnect}>Reconnect account</Button>
|
||||
)}
|
||||
{error && (
|
||||
<div className="space-y-2">
|
||||
<p role="alert" className="text-sm text-destructive">
|
||||
|
||||
@@ -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<boolean>();
|
||||
const [agentIds, setAgentIds] = useState(new Set(agentId ? [agentId] : []));
|
||||
const [protocol, setProtocol] = useState<AiProviderRouting["protocol"]>(
|
||||
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 && <p role="alert" className="text-sm text-destructive">Could not load connection permissions. <Button type="button" variant="ghost" onClick={() => void accounts.refetch()}>Retry</Button></p>}
|
||||
{agents.error && (
|
||||
<p role="alert" className="text-sm text-destructive">
|
||||
Could not load agents. Try again.
|
||||
@@ -277,6 +284,8 @@ export function AiProviderSetup({
|
||||
)}
|
||||
<AccessStepContent
|
||||
agents={agents.data ?? []}
|
||||
pending={accounts.isPending || Boolean(accounts.error)}
|
||||
capabilities={{ canCreateOrganizationGrant: canManageConnections, canSetCompanyInstall: canManageConnections }}
|
||||
authKind="api_key"
|
||||
grantKind={ownership === "shared" ? "organization" : "user"}
|
||||
grantKinds={["user", "organization"]}
|
||||
|
||||
@@ -40,7 +40,7 @@ const state = vi.hoisted(() => ({
|
||||
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 () => ({})),
|
||||
}));
|
||||
|
||||
@@ -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 (
|
||||
<QueryClientProvider client={client}>
|
||||
<AiProviderSetup
|
||||
companyId={companyId}
|
||||
agentId="nova"
|
||||
onCancel={() => {}}
|
||||
onComplete={() => {}}
|
||||
/>
|
||||
@@ -151,3 +153,13 @@ export const CompatibleConnectionDropdown: Story = {
|
||||
).toHaveTextContent("Company OpenRouter");
|
||||
},
|
||||
};
|
||||
|
||||
export const MemberProviderAccess: Story = {
|
||||
render: () => <Providers canManageConnections={false} />,
|
||||
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();
|
||||
},
|
||||
};
|
||||
Reference in new issue
Block a user