chore: promote dev to main

src/api/routers/agentDefinitions.ts

@@ -17,7 +17,7 @@ import {
 import { getRawTemplate, validateTemplate } from '../../agents/prompts/index.js';
 import {
 	deleteAgentDefinition,
-	getAgentDefinition,
+	getAgentDefinitionMetadata,
 	listAgentDefinitions,
 	upsertAgentDefinition,
 } from '../../db/repositories/agentDefinitionsRepository.js';
@@ -145,9 +145,8 @@ export const agentDefinitionsRouter = router({
 			}),
 		)
 		.mutation(async ({ input }) => {
-			// Validate agentType doesn't already exist in DB
-			// getAgentDefinition returns null for not-found, throws for DB errors
-			const existing = await getAgentDefinition(input.agentType);
+			// Validate agentType doesn't already exist in DB without parsing the stored body.
+			const existing = await getAgentDefinitionMetadata(input.agentType);
 			if (existing !== null) {
 				throw new TRPCError({
 					code: 'CONFLICT',
@@ -192,9 +191,8 @@ export const agentDefinitionsRouter = router({
 	delete: superAdminProcedure
 		.input(z.object({ agentType: z.string().min(1) }))
 		.mutation(async ({ input }) => {
-			// Verify the definition exists in DB
-			// getAgentDefinition returns null for not-found, throws for DB errors
-			const dbRow = await getAgentDefinition(input.agentType);
+			// Verify the definition exists in DB without parsing the stored body.
+			const dbRow = await getAgentDefinitionMetadata(input.agentType);
 			if (dbRow === null) {
 				throw new TRPCError({
 					code: 'NOT_FOUND',

src/db/repositories/agentDefinitionsRepository.ts

@@ -1,6 +1,7 @@
 import { eq } from 'drizzle-orm';
 import type { AgentDefinition } from '../../agents/definitions/schema.js';
 import { AgentDefinitionSchema } from '../../agents/definitions/schema.js';
+import { logger } from '../../utils/logging.js';
 import { getDb } from '../client.js';
 import { agentDefinitions } from '../schema/index.js';
 
@@ -23,11 +24,42 @@ export async function listAgentDefinitions(): Promise<
 > {
 	const db = getDb();
 	const rows = await db.select().from(agentDefinitions);
-	return rows.map((row) => ({
-		agentType: row.agentType,
-		definition: AgentDefinitionSchema.parse(row.definition),
-		isBuiltin: row.isBuiltin ?? false,
-	}));
+	const validRows: Array<{ agentType: string; definition: AgentDefinition; isBuiltin: boolean }> =
+		[];
+
+	for (const row of rows) {
+		const result = AgentDefinitionSchema.safeParse(row.definition);
+		if (!result.success) {
+			logger.warn('Skipping invalid agent definition from DB', {
+				agentType: row.agentType,
+				issues: result.error.issues.map((issue) => ({
+					path: issue.path.length > 0 ? issue.path.join('.') : '<root>',
+					message: issue.message,
+				})),
+			});
+			continue;
+		}
+
+		validRows.push({
+			agentType: row.agentType,
+			definition: result.data,
+			isBuiltin: row.isBuiltin ?? false,
+		});
+	}
+
+	return validRows;
+}
+
+export async function getAgentDefinitionMetadata(
+	agentType: string,
+): Promise<{ agentType: string; isBuiltin: boolean } | null> {
+	const db = getDb();
+	const [row] = await db
+		.select({ agentType: agentDefinitions.agentType, isBuiltin: agentDefinitions.isBuiltin })
+		.from(agentDefinitions)
+		.where(eq(agentDefinitions.agentType, agentType));
+	if (!row) return null;
+	return { agentType: row.agentType, isBuiltin: row.isBuiltin ?? false };
 }
 
 export async function upsertAgentDefinition(

tests/unit/api/routers/agentDefinitions.test.ts

@@ -15,7 +15,7 @@ const {
 	mockResolveAgentDefinition,
 	mockResolveKnownAgentTypes,
 	mockListAgentDefinitions,
-	mockGetAgentDefinition,
+	mockGetAgentDefinitionMetadata,
 	mockUpsertAgentDefinition,
 	mockDeleteAgentDefinition,
 	mockGetRawTemplate,
@@ -29,7 +29,7 @@ const {
 	mockResolveAgentDefinition: vi.fn<(agentType: string) => Promise<AgentDefinition>>(),
 	mockResolveKnownAgentTypes: vi.fn<() => Promise<string[]>>(),
 	mockListAgentDefinitions: vi.fn(),
-	mockGetAgentDefinition: vi.fn(),
+	mockGetAgentDefinitionMetadata: vi.fn(),
 	mockUpsertAgentDefinition: vi.fn(),
 	mockDeleteAgentDefinition: vi.fn(),
 	mockGetRawTemplate: vi.fn<(agentType: string) => string>(),
@@ -48,7 +48,7 @@ vi.mock('../../../../src/agents/definitions/loader.js', () => ({
 
 vi.mock('../../../../src/db/repositories/agentDefinitionsRepository.js', () => ({
 	listAgentDefinitions: mockListAgentDefinitions,
-	getAgentDefinition: mockGetAgentDefinition,
+	getAgentDefinitionMetadata: mockGetAgentDefinitionMetadata,
 	upsertAgentDefinition: mockUpsertAgentDefinition,
 	deleteAgentDefinition: mockDeleteAgentDefinition,
 }));
@@ -246,7 +246,7 @@ describe('agentDefinitionsRouter', () => {
 	// =====================================================================
 	describe('create', () => {
 		it('creates a new definition (superadmin)', async () => {
-			mockGetAgentDefinition.mockResolvedValue(null);
+			mockGetAgentDefinitionMetadata.mockResolvedValue(null);
 			mockUpsertAgentDefinition.mockResolvedValue(undefined);
 			const def = createMockDefinition();
 
@@ -259,7 +259,7 @@ describe('agentDefinitionsRouter', () => {
 		});
 
 		it('marks YAML-backed type as builtin on create', async () => {
-			mockGetAgentDefinition.mockResolvedValue(null);
+			mockGetAgentDefinitionMetadata.mockResolvedValue(null);
 			mockUpsertAgentDefinition.mockResolvedValue(undefined);
 			const def = createMockDefinition();
 
@@ -270,7 +270,7 @@ describe('agentDefinitionsRouter', () => {
 		});
 
 		it('throws CONFLICT when agent type already exists', async () => {
-			mockGetAgentDefinition.mockResolvedValue(createMockDefinition());
+			mockGetAgentDefinitionMetadata.mockResolvedValue({ agentType: 'existing', isBuiltin: false });
 			const def = createMockDefinition();
 
 			const caller = createCaller({ user: mockSuperAdmin, effectiveOrgId: mockSuperAdmin.orgId });
@@ -348,8 +348,10 @@ describe('agentDefinitionsRouter', () => {
 	// =====================================================================
 	describe('delete', () => {
 		it('deletes a non-builtin definition (superadmin)', async () => {
-			mockGetAgentDefinition.mockResolvedValue(createMockDefinition());
-			mockGetBuiltinAgentTypes.mockReturnValue(['implementation', 'review']); // custom-agent is NOT in this list
+			mockGetAgentDefinitionMetadata.mockResolvedValue({
+				agentType: 'custom-agent',
+				isBuiltin: false,
+			});
 			mockDeleteAgentDefinition.mockResolvedValue(undefined);
 
 			const caller = createCaller({ user: mockSuperAdmin, effectiveOrgId: mockSuperAdmin.orgId });
@@ -360,8 +362,22 @@ describe('agentDefinitionsRouter', () => {
 			expect(mockInvalidateDefinitionCache).toHaveBeenCalled();
 		});
 
+		it('deletes an invalid DB-only definition without parsing it', async () => {
+			mockGetAgentDefinitionMetadata.mockResolvedValue({
+				agentType: 'email-joke',
+				isBuiltin: true,
+			});
+			mockDeleteAgentDefinition.mockResolvedValue(undefined);
+
+			const caller = createCaller({ user: mockSuperAdmin, effectiveOrgId: mockSuperAdmin.orgId });
+			const result = await caller.delete({ agentType: 'email-joke' });
+
+			expect(result).toEqual({ agentType: 'email-joke' });
+			expect(mockDeleteAgentDefinition).toHaveBeenCalledWith('email-joke');
+		});
+
 		it('throws NOT_FOUND when definition not in DB', async () => {
-			mockGetAgentDefinition.mockResolvedValue(null);
+			mockGetAgentDefinitionMetadata.mockResolvedValue(null);
 
 			const caller = createCaller({ user: mockSuperAdmin, effectiveOrgId: mockSuperAdmin.orgId });
 			await expect(caller.delete({ agentType: 'missing' })).rejects.toMatchObject({
@@ -370,7 +386,10 @@ describe('agentDefinitionsRouter', () => {
 		});
 
 		it('throws FORBIDDEN when trying to delete a builtin (YAML-backed) type', async () => {
-			mockGetAgentDefinition.mockResolvedValue(createMockDefinition());
+			mockGetAgentDefinitionMetadata.mockResolvedValue({
+				agentType: 'implementation',
+				isBuiltin: true,
+			});
 
 			const caller = createCaller({ user: mockSuperAdmin, effectiveOrgId: mockSuperAdmin.orgId });
 			await expect(caller.delete({ agentType: 'implementation' })).rejects.toMatchObject({

tests/unit/db/repositories/agentDefinitionsRepository.test.ts

@@ -4,10 +4,19 @@ import { mockDbClientModule } from '../../../helpers/sharedMocks.js';
 
 vi.mock('../../../../src/db/client.js', () => mockDbClientModule);
 
+const { mockLoggerWarn } = vi.hoisted(() => ({
+	mockLoggerWarn: vi.fn(),
+}));
+
+vi.mock('../../../../src/utils/logging.js', () => ({
+	logger: { warn: mockLoggerWarn },
+}));
+
 import type { AgentDefinition } from '../../../../src/agents/definitions/schema.js';
 import {
 	deleteAgentDefinition,
 	getAgentDefinition,
+	getAgentDefinitionMetadata,
 	listAgentDefinitions,
 	upsertAgentDefinition,
 } from '../../../../src/db/repositories/agentDefinitionsRepository.js';
@@ -43,6 +52,7 @@ describe('agentDefinitionsRepository', () => {
 
 	beforeEach(() => {
 		mockDb = createMockDbWithGetDb({ withUpsert: true });
+		mockLoggerWarn.mockClear();
 	});
 
 	describe('getAgentDefinition', () => {
@@ -103,6 +113,53 @@ describe('agentDefinitionsRepository', () => {
 			const result = await listAgentDefinitions();
 			expect(result[0].isBuiltin).toBe(false);
 		});
+
+		it('skips invalid definitions and keeps valid rows', async () => {
+			mockDb.chain.from.mockResolvedValueOnce([
+				{ agentType: 'implementation', definition: mockDefinition, isBuiltin: true },
+				{ agentType: 'email-joke', definition: { invalid: 'data' }, isBuiltin: true },
+				{ agentType: 'review', definition: mockDefinition, isBuiltin: false },
+			]);
+
+			const result = await listAgentDefinitions();
+
+			expect(result.map((row) => row.agentType)).toEqual(['implementation', 'review']);
+			expect(mockLoggerWarn).toHaveBeenCalledWith(
+				'Skipping invalid agent definition from DB',
+				expect.objectContaining({
+					agentType: 'email-joke',
+					issues: expect.any(Array),
+				}),
+			);
+		});
+	});
+
+	describe('getAgentDefinitionMetadata', () => {
+		it('returns metadata without parsing the definition body', async () => {
+			mockDb.chain.where.mockResolvedValueOnce([
+				{ agentType: 'email-joke', isBuiltin: true, definition: { invalid: 'data' } },
+			]);
+
+			const result = await getAgentDefinitionMetadata('email-joke');
+
+			expect(result).toEqual({ agentType: 'email-joke', isBuiltin: true });
+		});
+
+		it('returns null when not found', async () => {
+			mockDb.chain.where.mockResolvedValueOnce([]);
+
+			const result = await getAgentDefinitionMetadata('missing');
+
+			expect(result).toBeNull();
+		});
+
+		it('defaults isBuiltin to false when null', async () => {
+			mockDb.chain.where.mockResolvedValueOnce([{ agentType: 'custom', isBuiltin: null }]);
+
+			const result = await getAgentDefinitionMetadata('custom');
+
+			expect(result).toEqual({ agentType: 'custom', isBuiltin: false });
+		});
 	});
 
 	describe('upsertAgentDefinition', () => {