From 8089c802f046d1b8f60ad17806d25c94cb769c65 Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Fri, 19 Jun 2026 10:43:28 +0700 Subject: [PATCH] Fix Paseo Agent PR review issues --- .../paseo-agent-settings-sheet-model.test.ts | 60 ++++ .../paseo-agent-settings-sheet-model.ts | 41 +++ .../paseo-agent-settings-sheet.test.tsx | 258 ------------------ .../components/paseo-agent-settings-sheet.tsx | 42 +-- .../providers/paseo-agent/config-service.ts | 37 +-- .../agent/providers/paseo-agent/config.ts | 14 +- packages/server/src/server/session.ts | 4 +- 7 files changed, 124 insertions(+), 332 deletions(-) create mode 100644 packages/app/src/components/paseo-agent-settings-sheet-model.test.ts create mode 100644 packages/app/src/components/paseo-agent-settings-sheet-model.ts delete mode 100644 packages/app/src/components/paseo-agent-settings-sheet.test.tsx diff --git a/packages/app/src/components/paseo-agent-settings-sheet-model.test.ts b/packages/app/src/components/paseo-agent-settings-sheet-model.test.ts new file mode 100644 index 000000000..b9ac6da70 --- /dev/null +++ b/packages/app/src/components/paseo-agent-settings-sheet-model.test.ts @@ -0,0 +1,60 @@ +import { describe, expect, it } from "vitest"; + +import { + createOpenRouterProviderInput, + parsePaseoAgentModelIds, + paseoAgentAuthLabel, +} from "./paseo-agent-settings-sheet-model"; + +describe("paseo-agent-settings-sheet-model", () => { + it("parses model ids from comma and newline separated input", () => { + expect( + parsePaseoAgentModelIds(` + anthropic/claude-3.7-sonnet, openai/gpt-4o + anthropic/claude-3.7-sonnet + openai/gpt-4o-mini + `), + ).toEqual(["anthropic/claude-3.7-sonnet", "openai/gpt-4o", "openai/gpt-4o-mini"]); + }); + + it("builds the OpenRouter provider payload without an empty api key", () => { + expect( + createOpenRouterProviderInput({ + name: " openrouter-main ", + apiKey: " ", + modelIds: ["openai/gpt-4o-mini"], + }), + ).toEqual({ + name: "openrouter-main", + providerType: "openrouter", + options: { + models: [{ id: "openai/gpt-4o-mini" }], + }, + }); + }); + + it("builds the OpenRouter provider payload with a trimmed api key", () => { + expect( + createOpenRouterProviderInput({ + name: "openrouter-main", + apiKey: " sk-or-secret ", + modelIds: ["openai/gpt-4o-mini", "anthropic/claude-3.7-sonnet"], + }), + ).toEqual({ + name: "openrouter-main", + providerType: "openrouter", + options: { + apiKey: "sk-or-secret", + models: [{ id: "openai/gpt-4o-mini" }, { id: "anthropic/claude-3.7-sonnet" }], + }, + }); + }); + + it("describes the provider auth state", () => { + expect(paseoAgentAuthLabel({ kind: "api_key", configured: true })).toBe("API key configured"); + expect(paseoAgentAuthLabel({ kind: "api_key", configured: false })).toBe("API key required"); + expect(paseoAgentAuthLabel({ kind: "oauth", configured: true })).toBe("ChatGPT login stored"); + expect(paseoAgentAuthLabel({ kind: "oauth", configured: false })).toBe("Login required"); + expect(paseoAgentAuthLabel({ kind: "none", configured: false })).toBe("No auth"); + }); +}); diff --git a/packages/app/src/components/paseo-agent-settings-sheet-model.ts b/packages/app/src/components/paseo-agent-settings-sheet-model.ts new file mode 100644 index 000000000..cb73d5056 --- /dev/null +++ b/packages/app/src/components/paseo-agent-settings-sheet-model.ts @@ -0,0 +1,41 @@ +import type { RedactedPaseoAgentProviderConfig } from "@getpaseo/protocol/messages"; +import type { PaseoAgentSetProviderInput } from "@/hooks/use-paseo-agent-providers"; + +export function paseoAgentAuthLabel(auth: RedactedPaseoAgentProviderConfig["auth"]): string { + if (auth.kind === "oauth") { + return auth.configured ? "ChatGPT login stored" : "Login required"; + } + if (auth.kind === "none") { + return "No auth"; + } + return auth.configured ? "API key configured" : "API key required"; +} + +export function parsePaseoAgentModelIds(raw: string): string[] { + const seen = new Set(); + const ids: string[] = []; + for (const part of raw.split(/[\n,]/)) { + const id = part.trim(); + if (id.length > 0 && !seen.has(id)) { + seen.add(id); + ids.push(id); + } + } + return ids; +} + +export function createOpenRouterProviderInput(input: { + name: string; + apiKey: string; + modelIds: string[]; +}): PaseoAgentSetProviderInput { + const trimmedKey = input.apiKey.trim(); + return { + name: input.name.trim(), + providerType: "openrouter", + options: { + models: input.modelIds.map((id) => ({ id })), + ...(trimmedKey.length > 0 ? { apiKey: trimmedKey } : {}), + }, + }; +} diff --git a/packages/app/src/components/paseo-agent-settings-sheet.test.tsx b/packages/app/src/components/paseo-agent-settings-sheet.test.tsx deleted file mode 100644 index 32093efd3..000000000 --- a/packages/app/src/components/paseo-agent-settings-sheet.test.tsx +++ /dev/null @@ -1,258 +0,0 @@ -/** - * @vitest-environment jsdom - */ -import React, { act } from "react"; -import { createRoot, type Root } from "react-dom/client"; -import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import type { RedactedPaseoAgentProviderConfig } from "@getpaseo/protocol/messages"; - -const { theme, hookState, setProviderMock } = vi.hoisted(() => ({ - theme: { - spacing: { 1: 4, 2: 8, 3: 12, 4: 16 }, - fontSize: { xs: 11, sm: 13 }, - fontWeight: { medium: "500" }, - borderRadius: { lg: 8 }, - colors: { - surface1: "#111", - surface2: "#222", - foreground: "#fff", - foregroundMuted: "#aaa", - border: "#555", - destructive: "#f00", - statusSuccess: "#0f0", - }, - }, - hookState: { - supported: true, - providers: [] as RedactedPaseoAgentProviderConfig[], - isLoading: false, - error: null as string | null, - }, - setProviderMock: vi.fn(async () => null), -})); - -vi.mock("react-native", () => ({ - View: ({ children, testID }: { children?: React.ReactNode; testID?: string }) => - React.createElement("div", { "data-testid": testID }, children), - Text: ({ children, testID }: { children?: React.ReactNode; testID?: string }) => - React.createElement("span", { "data-testid": testID }, children), -})); - -vi.mock("react-native-unistyles", () => ({ - StyleSheet: { - create: (factory: unknown) => - typeof factory === "function" ? (factory as (t: typeof theme) => unknown)(theme) : factory, - }, -})); - -vi.mock("lucide-react-native", () => ({ - Plus: () => React.createElement("span", { "data-icon": "Plus" }), -})); - -vi.mock("@/constants/platform", () => ({ isWeb: true })); - -vi.mock("@/components/adaptive-modal-sheet", () => ({ - AdaptiveModalSheet: ({ - children, - footer, - visible, - testID, - }: { - children?: React.ReactNode; - footer?: React.ReactNode; - visible?: boolean; - testID?: string; - }) => (visible ? React.createElement("div", { "data-testid": testID }, children, footer) : null), - AdaptiveTextInput: ({ - onChangeText, - accessibilityLabel, - testID, - }: { - onChangeText?: (value: string) => void; - accessibilityLabel?: string; - testID?: string; - }) => - React.createElement("input", { - "data-testid": testID, - "aria-label": accessibilityLabel, - onChange: (event: React.ChangeEvent) => onChangeText?.(event.target.value), - }), -})); - -vi.mock("@/components/ui/button", () => ({ - Button: ({ - children, - onPress, - disabled, - testID, - }: { - children?: React.ReactNode; - onPress?: () => void; - disabled?: boolean; - testID?: string; - }) => - React.createElement( - "button", - { - type: "button", - "data-testid": testID, - disabled, - onClick: disabled ? undefined : onPress, - }, - children, - ), -})); - -vi.mock("@/hooks/use-paseo-agent-providers", () => ({ - usePaseoAgentProviders: () => ({ - supported: hookState.supported, - providers: hookState.providers, - defaultModel: null, - isLoading: hookState.isLoading, - error: hookState.error, - refresh: vi.fn(async () => {}), - setProvider: setProviderMock, - }), -})); - -import { PaseoAgentSettingsSheet } from "./paseo-agent-settings-sheet"; - -function openRouterProvider(): RedactedPaseoAgentProviderConfig { - return { - name: "openrouter-main", - providerType: "openrouter", - models: [{ id: "anthropic/claude-3.7-sonnet" }], - auth: { kind: "api_key", configured: true, source: "literal" }, - available: true, - error: null, - }; -} - -describe("PaseoAgentSettingsSheet", () => { - let root: Root | null = null; - let container: HTMLElement | null = null; - - beforeEach(() => { - vi.stubGlobal("React", React); - vi.stubGlobal("IS_REACT_ACT_ENVIRONMENT", true); - container = document.createElement("div"); - document.body.appendChild(container); - root = createRoot(container); - - hookState.supported = true; - hookState.providers = []; - hookState.isLoading = false; - hookState.error = null; - setProviderMock.mockReset(); - setProviderMock.mockResolvedValue(null); - }); - - afterEach(() => { - if (root) { - act(() => root?.unmount()); - } - root = null; - container?.remove(); - container = null; - vi.unstubAllGlobals(); - }); - - function render(): void { - act(() => { - root?.render(); - }); - } - - function type(testID: string, value: string): void { - const input = container?.querySelector(`[data-testid="${testID}"]`); - if (!input) throw new Error(`No input ${testID}`); - const setValue = Object.getOwnPropertyDescriptor( - window.HTMLInputElement.prototype, - "value", - )?.set; - act(() => { - setValue?.call(input, value); - input.dispatchEvent(new window.Event("input", { bubbles: true })); - }); - } - - function click(testID: string): void { - const el = container?.querySelector(`[data-testid="${testID}"]`); - if (!el) throw new Error(`No element ${testID}`); - act(() => { - el.dispatchEvent(new window.MouseEvent("click", { bubbles: true })); - }); - } - - it("shows the update-host message and hides the add button when unsupported", () => { - hookState.supported = false; - render(); - - expect( - container?.querySelector('[data-testid="paseo-agent-unsupported"]')?.textContent, - ).toContain("Update the host to configure Paseo Agent."); - expect(container?.querySelector('[data-testid="paseo-agent-add-openrouter"]')).toBeNull(); - }); - - it("shows the error message instead of the empty state when the fetch fails", () => { - hookState.error = "Host is not connected"; - render(); - - const text = container?.textContent ?? ""; - expect(text).toContain("Host is not connected"); - expect(text).not.toContain("No inference providers configured yet."); - }); - - it("lists configured providers with type, model count, and auth state", () => { - hookState.providers = [openRouterProvider()]; - render(); - - const text = container?.textContent ?? ""; - expect(text).toContain("openrouter-main"); - expect(text).toContain("openrouter"); - expect(text).toContain("1 model"); - expect(text).toContain("API key configured"); - }); - - it("submits OpenRouter setup with name, api key, and parsed models", async () => { - render(); - - click("paseo-agent-add-openrouter"); - type("paseo-openrouter-name", "my-router"); - type("paseo-openrouter-api-key", "sk-or-secret"); - type("paseo-openrouter-models", "anthropic/claude-3.7-sonnet, openai/gpt-4o"); - - await act(async () => { - click("paseo-openrouter-submit"); - }); - - expect(setProviderMock).toHaveBeenCalledTimes(1); - expect(setProviderMock).toHaveBeenCalledWith({ - name: "my-router", - providerType: "openrouter", - options: { - apiKey: "sk-or-secret", - models: [{ id: "anthropic/claude-3.7-sonnet" }, { id: "openai/gpt-4o" }], - }, - }); - }); - - it("omits api key from the payload when left blank", async () => { - render(); - - click("paseo-agent-add-openrouter"); - type("paseo-openrouter-models", "anthropic/claude-3.7-sonnet"); - - await act(async () => { - click("paseo-openrouter-submit"); - }); - - expect(setProviderMock).toHaveBeenCalledWith({ - name: "openrouter", - providerType: "openrouter", - options: { - models: [{ id: "anthropic/claude-3.7-sonnet" }], - }, - }); - }); -}); diff --git a/packages/app/src/components/paseo-agent-settings-sheet.tsx b/packages/app/src/components/paseo-agent-settings-sheet.tsx index 9f3009bad..f599ae84b 100644 --- a/packages/app/src/components/paseo-agent-settings-sheet.tsx +++ b/packages/app/src/components/paseo-agent-settings-sheet.tsx @@ -11,6 +11,11 @@ import { import { Button } from "@/components/ui/button"; import { isWeb } from "@/constants/platform"; import { usePaseoAgentProviders } from "@/hooks/use-paseo-agent-providers"; +import { + createOpenRouterProviderInput, + parsePaseoAgentModelIds, + paseoAgentAuthLabel, +} from "./paseo-agent-settings-sheet-model"; interface PaseoAgentSettingsSheetProps { serverId: string; @@ -24,33 +29,10 @@ const HEADER: SheetHeader = { title: "Paseo Agent" }; const ADD_HEADER: SheetHeader = { title: "Add OpenRouter provider" }; const DEFAULT_PROVIDER_NAME = "openrouter"; -function authLabel(auth: RedactedPaseoAgentProviderConfig["auth"]): string { - if (auth.kind === "oauth") { - return auth.configured ? "ChatGPT login stored" : "Login required"; - } - if (auth.kind === "none") { - return "No auth"; - } - return auth.configured ? "API key configured" : "API key required"; -} - -function parseModelIds(raw: string): string[] { - const seen = new Set(); - const ids: string[] = []; - for (const part of raw.split(/[\n,]/)) { - const id = part.trim(); - if (id.length > 0 && !seen.has(id)) { - seen.add(id); - ids.push(id); - } - } - return ids; -} - function ProviderRow({ provider }: { provider: RedactedPaseoAgentProviderConfig }) { const modelCount = provider.models.length; const modelLabel = modelCount === 1 ? "1 model" : `${modelCount} models`; - const auth = authLabel(provider.auth); + const auth = paseoAgentAuthLabel(provider.auth); return ( parseModelIds(models), [models]); + const modelIds = useMemo(() => parsePaseoAgentModelIds(models), [models]); const canSubmit = trimmedName.length > 0 && modelIds.length > 0 && !saving; const handleSubmit = useCallback(() => { if (!canSubmit) return; setError(null); setSaving(true); - const trimmedKey = apiKey.trim(); - void setProvider({ - name: trimmedName, - providerType: "openrouter", - options: { - models: modelIds.map((id) => ({ id })), - ...(trimmedKey.length > 0 ? { apiKey: trimmedKey } : {}), - }, - }) + void setProvider(createOpenRouterProviderInput({ name: trimmedName, apiKey, modelIds })) .then(() => { setApiKey(""); onClose(); diff --git a/packages/server/src/server/agent/providers/paseo-agent/config-service.ts b/packages/server/src/server/agent/providers/paseo-agent/config-service.ts index acd3d41a1..4c12c1b84 100644 --- a/packages/server/src/server/agent/providers/paseo-agent/config-service.ts +++ b/packages/server/src/server/agent/providers/paseo-agent/config-service.ts @@ -14,6 +14,7 @@ import { PaseoAgentConfigSchema, type PaseoAgentConfig, type PaseoAgentProviderType, + resolvePaseoAgentProviderTypeDefaults, } from "./config.js"; import { hasStoredOAuthCredential, storeCodexOAuthCredential } from "./oauth-store.js"; import { isRefreshTokenExpressionConfigured } from "./oauth-credentials.js"; @@ -45,40 +46,6 @@ interface SetProviderInput { }; } -const PROVIDER_DEFAULTS: Record< - PaseoAgentProviderType | "openai-codex", - { baseUrl?: string; api?: string; envVar?: string } -> = { - openrouter: { - baseUrl: "https://openrouter.ai/api/v1", - api: "openai-completions", - envVar: "OPENROUTER_API_KEY", - }, - openai: { - baseUrl: "https://api.openai.com/v1", - api: "openai-responses", - envVar: "OPENAI_API_KEY", - }, - anthropic: { - baseUrl: "https://api.anthropic.com", - api: "anthropic-messages", - envVar: "ANTHROPIC_API_KEY", - }, - opencode: { - baseUrl: "https://opencode.ai/zen/v1", - api: "openai-completions", - envVar: "OPENCODE_API_KEY", - }, - "openai-compatible": { - api: "openai-completions", - }, - custom: {}, - "openai-codex": { - baseUrl: "https://chatgpt.com/backend-api", - api: "openai-codex-responses", - }, -}; - const ENV_REFERENCE_PATTERN = /\$\{?([A-Za-z_][A-Za-z0-9_]*)\}?/g; function resolveEnv(paseoHome: string, env?: NodeJS.ProcessEnv): NodeJS.ProcessEnv { @@ -125,7 +92,7 @@ function redactedProviders( env: NodeJS.ProcessEnv, ): RedactedPaseoAgentProviderConfig[] { return Object.entries(config.providers ?? {}).map(([name, entry]) => { - const defaults = PROVIDER_DEFAULTS[entry.type]; + const defaults = resolvePaseoAgentProviderTypeDefaults(entry.type); let auth: PaseoAgentProviderAuthState; if (entry.type === "openai-codex") { const hasRefreshToken = diff --git a/packages/server/src/server/agent/providers/paseo-agent/config.ts b/packages/server/src/server/agent/providers/paseo-agent/config.ts index 9a2cdf414..42ba6f6bd 100644 --- a/packages/server/src/server/agent/providers/paseo-agent/config.ts +++ b/packages/server/src/server/agent/providers/paseo-agent/config.ts @@ -43,7 +43,7 @@ const PROVIDER_TYPES = [ export type PaseoAgentProviderType = (typeof PROVIDER_TYPES)[number]; -interface ProviderTypeDefault { +export interface PaseoAgentProviderTypeDefault { /** Pi wire protocol. `undefined` for `custom`, where the user must pick one. */ api?: string; /** Default base URL. `undefined` means the user must supply `options.baseUrl`. */ @@ -55,7 +55,7 @@ interface ProviderTypeDefault { // Defaults mirror Pi's built-in provider definitions (packages/ai models). Pi adds // its own attribution headers for openrouter/opencode based on the base URL, so we // deliberately do not inject provider headers here. -const PROVIDER_TYPE_DEFAULTS: Record = { +const PROVIDER_TYPE_DEFAULTS: Record = { openrouter: { api: "openai-completions", baseUrl: "https://openrouter.ai/api/v1", @@ -98,6 +98,12 @@ const PROVIDER_TYPE_DEFAULTS: Record { - const defaults = PROVIDER_TYPE_DEFAULTS[entry.type]; + const defaults = resolvePaseoAgentProviderTypeDefaults(entry.type); if (!defaults.baseUrl && !entry.options.baseUrl) { ctx.addIssue({ code: z.ZodIssueCode.custom, @@ -195,7 +201,7 @@ function entries(config: PaseoAgentConfig): [string, PaseoAgentInferenceProvider function resolveProviderSettings( entry: PaseoAgentInferenceProviderEntry, ): ResolvedProviderSettings { - const defaults = PROVIDER_TYPE_DEFAULTS[entry.type]; + const defaults = resolvePaseoAgentProviderTypeDefaults(entry.type); const apiKey = entry.options.apiKey ?? (defaults.envVar ? `$${defaults.envVar}` : undefined); return { baseUrl: entry.options.baseUrl ?? defaults.baseUrl, diff --git a/packages/server/src/server/session.ts b/packages/server/src/server/session.ts index 0e12b2bf6..8010fb85c 100644 --- a/packages/server/src/server/session.ts +++ b/packages/server/src/server/session.ts @@ -570,6 +570,7 @@ export class Session { } | null = null; private readonly terminalManager: TerminalManager | null; private readonly providerSnapshotManager: ProviderSnapshotManager; + private paseoAgentConfigService: PaseoAgentConfigService | null = null; private readonly serviceProxy: ServiceProxySubsystem | null; private readonly scriptRuntimeStore: WorkspaceScriptRuntimeStore | null; private readonly getDaemonTcpPort: (() => number | null) | null; @@ -1677,7 +1678,7 @@ export class Session { } private createPaseoAgentConfigService(): PaseoAgentConfigService { - return new PaseoAgentConfigService({ + this.paseoAgentConfigService ??= new PaseoAgentConfigService({ paseoHome: this.paseoHome, logger: this.sessionLogger, onConfigChanged: (config) => { @@ -1685,6 +1686,7 @@ export class Session { this.agentManager.updateProviderRegistry(state); }, }); + return this.paseoAgentConfigService; } private async refreshPaseoAgentRuntimeSnapshot(): Promise {