diff --git a/apps/server/src/core/ai-chat/roles/ai-agent-roles.controller.spec.ts b/apps/server/src/core/ai-chat/roles/ai-agent-roles.controller.spec.ts index fd01a509..9076b396 100644 --- a/apps/server/src/core/ai-chat/roles/ai-agent-roles.controller.spec.ts +++ b/apps/server/src/core/ai-chat/roles/ai-agent-roles.controller.spec.ts @@ -39,6 +39,10 @@ describe('AiAgentRolesController admin gate', () => { create: jest.fn().mockResolvedValue({ id: 'r1' }), update: jest.fn().mockResolvedValue({ id: 'r1' }), remove: jest.fn().mockResolvedValue({ success: true }), + getCatalog: jest.fn().mockResolvedValue({ languages: [], bundles: [] }), + getCatalogBundle: jest.fn().mockResolvedValue({ roles: [] }), + importFromCatalog: jest.fn().mockResolvedValue({ created: 0 }), + updateFromCatalog: jest.fn().mockResolvedValue({ updated: false }), }; const controller = new AiAgentRolesController( rolesService as never, @@ -109,6 +113,90 @@ describe('AiAgentRolesController admin gate', () => { }); }); + // Catalog routes (browse + import) are ALL admin-only: a non-admin caller must + // get ForbiddenException with the service untouched; an admin delegates with + // the right arguments (import/update-from-catalog carry workspace.id). + describe('catalog routes admin gate', () => { + const catalogDto = { language: 'en' } as never; + const bundleDto = { bundleId: 'general', language: 'en' } as never; + const importDto = { + bundleId: 'general', + language: 'en', + conflict: 'skip', + } as never; + const updateDto = { id: 'r1' } as never; + + describe('non-admin is rejected and the service is NOT called', () => { + it('catalog', async () => { + const { controller, rolesService } = makeController(false); + await expect( + controller.catalog(catalogDto, user, workspace), + ).rejects.toBeInstanceOf(ForbiddenException); + expect(rolesService.getCatalog).not.toHaveBeenCalled(); + }); + + it('catalog/bundle', async () => { + const { controller, rolesService } = makeController(false); + await expect( + controller.catalogBundle(bundleDto, user, workspace), + ).rejects.toBeInstanceOf(ForbiddenException); + expect(rolesService.getCatalogBundle).not.toHaveBeenCalled(); + }); + + it('import', async () => { + const { controller, rolesService } = makeController(false); + await expect( + controller.import(importDto, user, workspace), + ).rejects.toBeInstanceOf(ForbiddenException); + expect(rolesService.importFromCatalog).not.toHaveBeenCalled(); + }); + + it('update-from-catalog', async () => { + const { controller, rolesService } = makeController(false); + await expect( + controller.updateFromCatalog(updateDto, user, workspace), + ).rejects.toBeInstanceOf(ForbiddenException); + expect(rolesService.updateFromCatalog).not.toHaveBeenCalled(); + }); + }); + + describe('admin delegates to the service', () => { + it('catalog passes the requested language', async () => { + const { controller, rolesService } = makeController(true); + await controller.catalog(catalogDto, user, workspace); + expect(rolesService.getCatalog).toHaveBeenCalledWith('en'); + }); + + it('catalog/bundle passes bundleId + language', async () => { + const { controller, rolesService } = makeController(true); + await controller.catalogBundle(bundleDto, user, workspace); + expect(rolesService.getCatalogBundle).toHaveBeenCalledWith( + 'general', + 'en', + ); + }); + + it('import passes workspace.id + user.id + dto', async () => { + const { controller, rolesService } = makeController(true); + await controller.import(importDto, user, workspace); + expect(rolesService.importFromCatalog).toHaveBeenCalledWith( + 'ws-1', + 'u1', + importDto, + ); + }); + + it('update-from-catalog passes workspace.id + dto', async () => { + const { controller, rolesService } = makeController(true); + await controller.updateFromCatalog(updateDto, user, workspace); + expect(rolesService.updateFromCatalog).toHaveBeenCalledWith( + 'ws-1', + updateDto, + ); + }); + }); + }); + describe('list (member-reachable)', () => { it('non-admin reaches list and the service is asked for the picker view (isAdmin=false)', async () => { const { controller, rolesService } = makeController(false); diff --git a/apps/server/src/core/ai-chat/roles/ai-agent-roles.service.spec.ts b/apps/server/src/core/ai-chat/roles/ai-agent-roles.service.spec.ts index dc99733a..c4c0468c 100644 --- a/apps/server/src/core/ai-chat/roles/ai-agent-roles.service.spec.ts +++ b/apps/server/src/core/ai-chat/roles/ai-agent-roles.service.spec.ts @@ -1,4 +1,8 @@ -import { BadRequestException, ConflictException } from '@nestjs/common'; +import { + BadGatewayException, + BadRequestException, + ConflictException, +} from '@nestjs/common'; import { AiAgentRolesService } from './ai-agent-roles.service'; import type { AiAgentRole } from '@docmost/db/types/entity.types'; import type { @@ -786,4 +790,220 @@ describe('AiAgentRolesService guards', () => { expect(repo.update.mock.calls[0][2].name).toBe('Researcher'); }); }); + + // --------------------------------------------------------------------------- + // Catalog browse (getCatalog / getCatalogBundle) against a MOCKED provider. + // Covers the localized() three-tier fallback (requested lang -> en -> first -> + // null), the sorted union of bundle languages, the missing-bundle BadGateway, + // and the role-version default. + // --------------------------------------------------------------------------- + describe('getCatalog', () => { + function makeBrowseService(index: unknown) { + const repo = { + findById: jest.fn(), + insert: jest.fn(), + update: jest.fn(), + softDelete: jest.fn(), + listByWorkspace: jest.fn(), + }; + const catalog = { + fetchIndex: jest.fn().mockResolvedValue(index), + fetchBundle: jest.fn(), + }; + const service = new AiAgentRolesService(repo as never, catalog as never); + return { service, catalog }; + } + + it('returns the sorted union of every bundle language', async () => { + const { service } = makeBrowseService({ + schemaVersion: 1, + bundles: [ + { + id: 'a', + name: { en: 'A' }, + languages: ['ru', 'en'], + roles: [], + }, + { + id: 'b', + name: { en: 'B' }, + languages: ['en', 'de'], + roles: [], + }, + ], + }); + const res = await service.getCatalog('en'); + expect(res.languages).toEqual(['de', 'en', 'ru']); + }); + + it('localized name uses the requested language when present', async () => { + const { service } = makeBrowseService({ + schemaVersion: 1, + bundles: [ + { + id: 'a', + name: { en: 'General', ru: 'Общие' }, + description: { en: 'desc-en', ru: 'desc-ru' }, + languages: ['en', 'ru'], + roles: [{ slug: 'researcher', version: 2 }], + }, + ], + }); + const res = await service.getCatalog('ru'); + expect(res.bundles[0]).toMatchObject({ + id: 'a', + name: 'Общие', + description: 'desc-ru', + languages: ['en', 'ru'], + roles: [{ slug: 'researcher', version: 2 }], + }); + }); + + it('localized name falls back to en when the requested language is missing', async () => { + const { service } = makeBrowseService({ + schemaVersion: 1, + bundles: [ + { + id: 'a', + name: { en: 'General', ru: 'Общие' }, + languages: ['en', 'ru'], + roles: [], + }, + ], + }); + const res = await service.getCatalog('fr'); + expect(res.bundles[0].name).toBe('General'); + }); + + it('localized name falls back to the first available locale when en is absent', async () => { + const { service } = makeBrowseService({ + schemaVersion: 1, + bundles: [ + { + id: 'a', + name: { ru: 'Общие', de: 'Allgemein' }, + languages: ['ru', 'de'], + roles: [], + }, + ], + }); + const res = await service.getCatalog('fr'); + // Neither 'fr' nor 'en' is present -> first available value. + expect(res.bundles[0].name).toBe('Общие'); + }); + + it('empty name map => falls back to the bundle id; absent description => null', async () => { + const { service } = makeBrowseService({ + schemaVersion: 1, + bundles: [ + { + id: 'a', + name: {}, + languages: ['en'], + roles: [], + }, + ], + }); + const res = await service.getCatalog('en'); + expect(res.bundles[0].name).toBe('a'); + expect(res.bundles[0].description).toBeNull(); + }); + }); + + describe('getCatalogBundle', () => { + function makeBundleService(opts: { + index: unknown; + bundle: unknown; + }) { + const repo = { + findById: jest.fn(), + insert: jest.fn(), + update: jest.fn(), + softDelete: jest.fn(), + listByWorkspace: jest.fn(), + }; + const catalog = { + fetchIndex: jest.fn().mockResolvedValue(opts.index), + fetchBundle: jest.fn().mockResolvedValue(opts.bundle), + }; + const service = new AiAgentRolesService(repo as never, catalog as never); + return { service, catalog }; + } + + const index = { + schemaVersion: 1, + bundles: [ + { + id: 'general', + name: { en: 'General' }, + languages: ['en'], + roles: [{ slug: 'researcher', version: 4 }], + }, + ], + }; + + it('missing bundle in the index => BadGateway', async () => { + const { service, catalog } = makeBundleService({ + index, + bundle: { schemaVersion: 1, language: 'en', roles: [] }, + }); + await expect( + service.getCatalogBundle('ghost', 'en'), + ).rejects.toBeInstanceOf(BadGatewayException); + expect(catalog.fetchBundle).not.toHaveBeenCalled(); + }); + + it('maps role content with the version taken from the index', async () => { + const { service } = makeBundleService({ + index, + bundle: { + schemaVersion: 1, + language: 'en', + roles: [ + { + slug: 'researcher', + name: 'Researcher', + instructions: 'be a researcher', + emoji: '🔬', + autoStart: false, + launchMessage: 'go', + }, + ], + }, + }); + const res = await service.getCatalogBundle('general', 'en'); + expect(res).toMatchObject({ bundleId: 'general', language: 'en' }); + expect(res.roles[0]).toEqual({ + slug: 'researcher', + emoji: '🔬', + name: 'Researcher', + description: null, + instructions: 'be a researcher', + autoStart: false, + launchMessage: 'go', + version: 4, + }); + }); + + it('role absent from the index meta => version defaults to 1; autoStart defaults to true', async () => { + const { service } = makeBundleService({ + index, + bundle: { + schemaVersion: 1, + language: 'en', + roles: [ + { slug: 'newcomer', name: 'Newcomer', instructions: 'hi' }, + ], + }, + }); + const res = await service.getCatalogBundle('general', 'en'); + expect(res.roles[0]).toMatchObject({ + slug: 'newcomer', + version: 1, + autoStart: true, + emoji: null, + launchMessage: null, + }); + }); + }); }); diff --git a/apps/server/src/core/ai-chat/roles/ai-agent-roles.service.ts b/apps/server/src/core/ai-chat/roles/ai-agent-roles.service.ts index 276c1118..c33200f5 100644 --- a/apps/server/src/core/ai-chat/roles/ai-agent-roles.service.ts +++ b/apps/server/src/core/ai-chat/roles/ai-agent-roles.service.ts @@ -509,11 +509,7 @@ export class AiAgentRolesService { * failures keep surfacing as 500s. */ function rethrowDuplicateName(err: unknown, name: string): never { - if ( - err && - typeof err === 'object' && - (err as { code?: unknown }).code === '23505' - ) { + if (isUniqueViolation(err)) { throw new ConflictException( `A role named "${name}" already exists in this workspace.`, ); @@ -521,6 +517,15 @@ function rethrowDuplicateName(err: unknown, name: string): never { throw err; } +/** Whether `err` is a Postgres unique-violation (SQLSTATE 23505). */ +function isUniqueViolation(err: unknown): boolean { + return ( + !!err && + typeof err === 'object' && + (err as { code?: unknown }).code === '23505' + ); +} + /** '' / whitespace-only / undefined / null => null; otherwise the trimmed value. */ function emptyToNull(value: string | null | undefined): string | null { if (value === undefined || value === null) return null; @@ -564,23 +569,20 @@ function localized( * `taken` after a successful insert. */ function freeName(base: string, taken: Set): string { - let n = 2; - // Cap the search defensively; the loop always terminates well before this. - while (n < 1000) { + // `taken` is finite, so within `taken.size + 2` iterations a candidate index + // is guaranteed free; the 1000 cap is a defensive upper bound far above any + // realistic per-name collision count. The throw below is therefore + // unreachable in practice and only satisfies the return-type checker. + for (let n = 2; n < 1000; n++) { const candidate = `${base} (${n})`; if (!taken.has(candidate.toLowerCase())) return candidate; - n++; } - return `${base} (${Date.now()})`; + throw new BadRequestException(`Too many roles named "${base}"`); } /** A short, safe message for an import insert failure (409 vs other). */ function importErrorMessage(err: unknown): string { - if ( - err && - typeof err === 'object' && - (err as { code?: unknown }).code === '23505' - ) { + if (isUniqueViolation(err)) { return 'A role with this name already exists'; } return 'Failed to import role'; diff --git a/apps/server/src/core/ai-chat/roles/catalog/ai-agent-roles-catalog.provider.spec.ts b/apps/server/src/core/ai-chat/roles/catalog/ai-agent-roles-catalog.provider.spec.ts index ce5ec55f..23d53a78 100644 --- a/apps/server/src/core/ai-chat/roles/catalog/ai-agent-roles-catalog.provider.spec.ts +++ b/apps/server/src/core/ai-chat/roles/catalog/ai-agent-roles-catalog.provider.spec.ts @@ -174,6 +174,24 @@ describe('AiAgentRolesCatalogProvider (local fixtures)', () => { ); }); + it('fetch rejects (network failure) => BadGateway (unavailable)', async () => { + global.fetch = jest + .fn() + .mockRejectedValue(new Error('ECONNREFUSED')) as never; + const provider = makeProvider('https://catalog.example.com'); + await expect(provider.fetchIndex()).rejects.toBeInstanceOf( + BadGatewayException, + ); + }); + + it('non-ok response (503) => BadGateway carrying the status', async () => { + global.fetch = jest.fn().mockResolvedValue( + mockResponse({ ok: false, status: 503, body: null }), + ) as never; + const provider = makeProvider('https://catalog.example.com'); + await expect(provider.fetchIndex()).rejects.toThrow(/503/); + }); + it('small streamed body parses normally (cap not hit)', async () => { const json = JSON.stringify({ schemaVersion: 1, diff --git a/apps/server/src/core/ai-chat/roles/catalog/ai-agent-roles-catalog.provider.ts b/apps/server/src/core/ai-chat/roles/catalog/ai-agent-roles-catalog.provider.ts index 9eda82c7..83895cc8 100644 --- a/apps/server/src/core/ai-chat/roles/catalog/ai-agent-roles-catalog.provider.ts +++ b/apps/server/src/core/ai-chat/roles/catalog/ai-agent-roles-catalog.provider.ts @@ -4,6 +4,7 @@ import { BadGatewayException, BadRequestException, Injectable, + Logger, } from '@nestjs/common'; import { EnvironmentService } from '../../../../integrations/environment/environment.service'; import { @@ -35,6 +36,8 @@ const MAX_BYTES = 1_000_000; */ @Injectable() export class AiAgentRolesCatalogProvider { + private readonly logger = new Logger(AiAgentRolesCatalogProvider.name); + constructor(private readonly environmentService: EnvironmentService) {} /** Read + validate the top-level index (`index.json`). */ @@ -79,9 +82,11 @@ export class AiAgentRolesCatalogProvider { private parseJson(raw: string, rel: string): unknown { try { return JSON.parse(raw); - } catch { + } catch (err) { + const reason = shortError(err); + this.logger.error(`Agent roles catalog JSON parse failed (${rel}): ${reason}`); throw new BadGatewayException( - `Agent roles catalog file is not valid JSON (${rel})`, + `Agent roles catalog file is not valid JSON (${rel}): ${reason}`, ); } } @@ -102,8 +107,14 @@ export class AiAgentRolesCatalogProvider { private async readLocal(dir: string, rel: string): Promise { try { return await fs.readFile(path.join(dir, rel), 'utf8'); - } catch { - throw new BadGatewayException('Agent roles catalog is unavailable'); + } catch (err) { + const reason = shortError(err); + this.logger.error( + `Agent roles catalog local read failed (${path.join(dir, rel)}): ${reason}`, + ); + throw new BadGatewayException( + `Agent roles catalog is unavailable: ${reason}`, + ); } } @@ -122,10 +133,19 @@ export class AiAgentRolesCatalogProvider { let response: Response; try { response = await fetch(url, { signal: controller.signal }); - } catch { - throw new BadGatewayException('Agent roles catalog is unavailable'); + } catch (err) { + const reason = shortError(err); + this.logger.error( + `Agent roles catalog remote fetch failed (${rel}): ${reason}`, + ); + throw new BadGatewayException( + `Agent roles catalog is unavailable: ${reason}`, + ); } if (!response.ok) { + this.logger.error( + `Agent roles catalog remote returned ${response.status} (${rel})`, + ); throw new BadGatewayException( `Agent roles catalog returned ${response.status}`, ); @@ -179,13 +199,28 @@ async function readStreamCapped( // Release the stream on both the normal and the too-large/abort paths. await reader.cancel().catch(() => undefined); } - const merged = new Uint8Array(total); - let offset = 0; - for (const chunk of chunks) { - merged.set(chunk, offset); - offset += chunk.length; + return Buffer.concat(chunks).toString('utf8'); +} + +/** + * A short, non-sensitive error string for logging/propagation: only the first + * line of the message head is kept (upstream bodies / URLs are discarded). + */ +function shortError(err: unknown): string { + let message = ''; + if (typeof err === 'string') { + message = err; + } else if ( + err && + typeof err === 'object' && + typeof (err as { message?: unknown }).message === 'string' + ) { + // Read `.message` directly (works for Error instances and the realm-shifted + // Error-likes jest can hand back, where `instanceof Error` is false). + message = (err as { message: string }).message; } - return new TextDecoder('utf-8').decode(merged); + const head = (message || 'unknown error').split('\n')[0]; + return head.length > 200 ? `${head.slice(0, 200)}…` : head; } // --------------------------------------------------------------------------- diff --git a/apps/server/src/database/migrations/20260626T120000-ai-agent-roles-catalog-source.ts b/apps/server/src/database/migrations/20260626T150000-ai-agent-roles-catalog-source.ts similarity index 100% rename from apps/server/src/database/migrations/20260626T120000-ai-agent-roles-catalog-source.ts rename to apps/server/src/database/migrations/20260626T150000-ai-agent-roles-catalog-source.ts