diff --git a/apps/client/src/features/workspace/components/settings/components/ai-agent-role-form.drivers.test.ts b/apps/client/src/features/workspace/components/settings/components/ai-agent-role-form.drivers.test.ts new file mode 100644 index 00000000..9ff4f30b --- /dev/null +++ b/apps/client/src/features/workspace/components/settings/components/ai-agent-role-form.drivers.test.ts @@ -0,0 +1,53 @@ +import { readFileSync } from "node:fs"; +import { fileURLToPath } from "node:url"; +import path from "node:path"; +import { describe, expect, it } from "vitest"; +import { + AI_DRIVER_VALUES, + DRIVER_OPTIONS, +} from "./ai-agent-role-form"; + +/** + * Drift guard: the client's hardcoded driver list must stay in sync with the + * server `AI_DRIVERS`. Client and server are separate build targets and Vite + * refuses to import a module from outside the client root, so instead of an + * `import` we read the server `ai.types.ts` source and parse out the AI_DRIVERS + * literal. This contract test fails loudly if the two lists ever diverge + * (order-independent). + */ +function readServerAiDrivers(): string[] { + const here = path.dirname(fileURLToPath(import.meta.url)); + // apps/client/src/.../components -> repo apps/server/src/integrations/ai + const serverTypesPath = path.resolve( + here, + "../../../../../../../server/src/integrations/ai/ai.types.ts", + ); + const source = readFileSync(serverTypesPath, "utf8"); + const match = source.match(/AI_DRIVERS\s*:\s*AiDriver\[\]\s*=\s*\[([^\]]*)\]/); + if (!match) { + throw new Error( + `Could not locate the AI_DRIVERS literal in ${serverTypesPath}`, + ); + } + return match[1] + .split(",") + .map((s) => s.trim().replace(/^['"]|['"]$/g, "")) + .filter((s) => s.length > 0); +} + +describe("ai-agent-role-form driver drift guard", () => { + it("mirrors the server AI_DRIVERS list exactly", () => { + const serverDrivers = readServerAiDrivers(); + expect([...AI_DRIVER_VALUES].sort()).toEqual([...serverDrivers].sort()); + }); + + it("exposes one Select option per server driver plus a workspace-default", () => { + const serverDrivers = readServerAiDrivers(); + const driverOptionValues = DRIVER_OPTIONS.map((o) => o.value).filter( + (v) => v !== "", + ); + expect(driverOptionValues.sort()).toEqual([...serverDrivers].sort()); + // Exactly one empty-value option for the "Workspace default" choice. + expect(DRIVER_OPTIONS.filter((o) => o.value === "")).toHaveLength(1); + }); +}); diff --git a/apps/client/src/features/workspace/components/settings/components/ai-agent-role-form.tsx b/apps/client/src/features/workspace/components/settings/components/ai-agent-role-form.tsx index f3e7a930..afbb4f59 100644 --- a/apps/client/src/features/workspace/components/settings/components/ai-agent-role-form.tsx +++ b/apps/client/src/features/workspace/components/settings/components/ai-agent-role-form.tsx @@ -23,13 +23,25 @@ import { IAiRoleUpdate, } from "@/features/ai-chat/types/ai-chat.types.ts"; -// Supported drivers for the optional model override (mirrors server AI_DRIVERS). -// "" => use the workspace default driver/model. -const DRIVER_OPTIONS = [ +// Source of truth: the server `AI_DRIVERS` list in +// apps/server/src/integrations/ai/ai.types.ts. The client cannot import that +// constant at build time (separate build target), so it is mirrored here and a +// drift contract test (ai-agent-role-form.drivers.test.ts) fails if the two +// lists diverge. Keep this in sync when adding/removing a server driver. +export const AI_DRIVER_VALUES = ["openai", "gemini", "ollama"] as const; +export type AiDriverValue = (typeof AI_DRIVER_VALUES)[number]; + +const DRIVER_LABELS: Record = { + openai: "OpenAI", + gemini: "Gemini", + ollama: "Ollama", +}; + +// Select options for the optional model override. "" => use the workspace +// default driver/model. +export const DRIVER_OPTIONS = [ { value: "", label: "Workspace default" }, - { value: "openai", label: "OpenAI" }, - { value: "gemini", label: "Gemini" }, - { value: "ollama", label: "Ollama" }, + ...AI_DRIVER_VALUES.map((value) => ({ value, label: DRIVER_LABELS[value] })), ]; const formSchema = z.object({ @@ -38,7 +50,7 @@ const formSchema = z.object({ description: z.string(), instructions: z.string().min(1), // "" => no driver override (use the workspace driver). - driver: z.enum(["", "openai", "gemini", "ollama"]), + driver: z.enum(["", ...AI_DRIVER_VALUES]), chatModel: z.string(), enabled: z.boolean(), }); diff --git a/apps/server/src/core/ai-chat/roles/dto/agent-role.dto.spec.ts b/apps/server/src/core/ai-chat/roles/dto/agent-role.dto.spec.ts new file mode 100644 index 00000000..bb063dc1 --- /dev/null +++ b/apps/server/src/core/ai-chat/roles/dto/agent-role.dto.spec.ts @@ -0,0 +1,81 @@ +import 'reflect-metadata'; +import { plainToInstance } from 'class-transformer'; +import { validateSync } from 'class-validator'; +import { CreateAgentRoleDto, RoleModelConfigDto } from './agent-role.dto'; + +/** + * API-boundary validation for the role model override. The key invariants: + * - `driver`, when present, must be a supported server driver (AI_DRIVERS); + * - `chatModel`, when present, must be a non-empty, trimmed, bounded string — + * empty/whitespace-only garbage is rejected here, not at provider runtime. + */ +describe('RoleModelConfigDto validation', () => { + function validateConfig(config: unknown) { + const dto = plainToInstance(RoleModelConfigDto, config); + return validateSync(dto as object); + } + + it('accepts a supported driver + non-empty chatModel', () => { + expect(validateConfig({ driver: 'openai', chatModel: 'gpt-4o' })).toHaveLength( + 0, + ); + }); + + it('accepts an empty object (omitted override => workspace default)', () => { + expect(validateConfig({})).toHaveLength(0); + }); + + it('rejects an unknown driver', () => { + const errors = validateConfig({ driver: 'anthropic', chatModel: 'x' }); + expect(errors.some((e) => e.property === 'driver')).toBe(true); + }); + + it('rejects an empty chatModel string', () => { + const errors = validateConfig({ chatModel: '' }); + expect(errors.some((e) => e.property === 'chatModel')).toBe(true); + }); + + it('rejects a whitespace-only chatModel (trimmed to empty)', () => { + const errors = validateConfig({ chatModel: ' ' }); + expect(errors.some((e) => e.property === 'chatModel')).toBe(true); + }); + + it('trims surrounding whitespace from chatModel', () => { + const dto = plainToInstance(RoleModelConfigDto, { + chatModel: ' gpt-4o-mini ', + }); + expect(validateSync(dto as object)).toHaveLength(0); + expect(dto.chatModel).toBe('gpt-4o-mini'); + }); + + it('rejects a chatModel longer than 200 chars', () => { + const errors = validateConfig({ chatModel: 'a'.repeat(201) }); + expect(errors.some((e) => e.property === 'chatModel')).toBe(true); + }); +}); + +describe('CreateAgentRoleDto with nested modelConfig', () => { + function validateCreate(payload: unknown) { + const dto = plainToInstance(CreateAgentRoleDto, payload); + return validateSync(dto as object); + } + + const base = { name: 'Researcher', instructions: 'Do research.' }; + + it('accepts a valid create payload with a model override', () => { + expect( + validateCreate({ + ...base, + modelConfig: { driver: 'gemini', chatModel: 'gemini-2.0-flash' }, + }), + ).toHaveLength(0); + }); + + it('rejects a create payload whose nested chatModel is blank', () => { + const errors = validateCreate({ + ...base, + modelConfig: { chatModel: ' ' }, + }); + expect(errors.length).toBeGreaterThan(0); + }); +}); diff --git a/apps/server/src/core/ai-chat/roles/dto/agent-role.dto.ts b/apps/server/src/core/ai-chat/roles/dto/agent-role.dto.ts index 02ff840e..9aff4c36 100644 --- a/apps/server/src/core/ai-chat/roles/dto/agent-role.dto.ts +++ b/apps/server/src/core/ai-chat/roles/dto/agent-role.dto.ts @@ -5,9 +5,10 @@ import { IsOptional, IsString, MaxLength, + MinLength, ValidateNested, } from 'class-validator'; -import { Type } from 'class-transformer'; +import { Transform, TransformFnParams, Type } from 'class-transformer'; import { AI_DRIVERS, AiDriver } from '../../../../integrations/ai/ai.types'; /** @@ -20,8 +21,16 @@ export class RoleModelConfigDto { @IsIn(AI_DRIVERS) driver?: AiDriver; + // Free-form provider model id (providers add models constantly, so we don't + // pin an allow-list). We still reject empty/whitespace-only garbage at the API + // boundary: trim first, then require a non-empty, bounded string. An invalid + // model still surfaces as a clear provider 503 at resolve time, not here. @IsOptional() + @Transform(({ value }: TransformFnParams) => + typeof value === 'string' ? value.trim() : value, + ) @IsString() + @MinLength(1) @MaxLength(200) chatModel?: string; }