diff --git a/apps/api/src/openapi-docs.spec.ts b/apps/api/src/openapi-docs.spec.ts index 74f95f62d4..d9d42d89ae 100644 --- a/apps/api/src/openapi-docs.spec.ts +++ b/apps/api/src/openapi-docs.spec.ts @@ -113,6 +113,7 @@ import { AppModule } from './app.module'; import { applyPublicOpenApiMetadata, PUBLIC_OPENAPI_DESCRIPTION, + PUBLIC_OPENAPI_TIMEOUT_MS, PUBLIC_OPENAPI_TITLE, PUBLIC_SERVER_URL, } from './openapi/public-docs-metadata'; @@ -156,6 +157,14 @@ describe('OpenAPI document', () => { ]); }); + it('bakes a finite default request timeout into the generated SDK + MCP server', () => { + // Without x-speakeasy-timeout the generated request funcs use -1 ("no + // timeout") and a hung upstream wedges the MCP connection forever. + expect( + (document as { 'x-speakeasy-timeout'?: number })['x-speakeasy-timeout'], + ).toBe(PUBLIC_OPENAPI_TIMEOUT_MS); + }); + it('keeps the public spec complete, SEO-ready, and free of private surfaces', () => { const issues = collectPublicOpenApiIssues(document); diff --git a/apps/api/src/openapi/public-docs-metadata.ts b/apps/api/src/openapi/public-docs-metadata.ts index d2c8b9ad91..b3454be8d8 100644 --- a/apps/api/src/openapi/public-docs-metadata.ts +++ b/apps/api/src/openapi/public-docs-metadata.ts @@ -26,6 +26,31 @@ export const PUBLIC_OPENAPI_DESCRIPTION = export const PUBLIC_SERVER_URL = 'https://api.trycomp.ai'; +/** + * Default request timeout (ms) baked into the generated SDK + MCP server via the + * `x-speakeasy-timeout` document-root extension. + * + * Speakeasy-generated request functions resolve their timeout to + * `operationTimeoutMs || clientTimeoutMs || -1`, and `-1` means "no timeout". + * Without this, a slow/hung upstream call leaves the MCP server's fetch dangling + * forever; the MCP client eventually gives up and marks the whole connection + * unhealthy (customer-reported wedging). A finite timeout makes the SDK abort + * the request and return a clean error instead, keeping the connection alive. + * + * 120s comfortably covers our slowest endpoints while staying under the ALB's + * 300s idle timeout (comp-private infra/modules/loadbalancer.ts). + */ +export const PUBLIC_OPENAPI_TIMEOUT_MS = 120_000; + +/** + * OpenAPIObject (from @nestjs/swagger) has no index signature for `x-*` + * extensions at the document root, so we widen it locally instead of reaching + * for `as any`. + */ +type OpenApiDocumentWithExtensions = OpenAPIObject & { + 'x-speakeasy-timeout'?: number; +}; + function getVisibilityForOperation( operation: OpenApiOperation, metadata?: PublicOperationMetadata, @@ -284,6 +309,12 @@ export function applyPublicOpenApiMetadata(document: OpenAPIObject): void { }, ]; + // Bake a finite default request timeout into the generated SDK + MCP server + // so a hung upstream call can never wedge the MCP connection. See + // PUBLIC_OPENAPI_TIMEOUT_MS for the full rationale. + (document as OpenApiDocumentWithExtensions)['x-speakeasy-timeout'] = + PUBLIC_OPENAPI_TIMEOUT_MS; + const paths = document.paths as Record< string, Record diff --git a/apps/api/src/tasks/automations/automations.controller.ts b/apps/api/src/tasks/automations/automations.controller.ts index 05ce95b6cc..c07c3f0434 100644 --- a/apps/api/src/tasks/automations/automations.controller.ts +++ b/apps/api/src/tasks/automations/automations.controller.ts @@ -10,6 +10,7 @@ import { UseGuards, } from '@nestjs/common'; import { + ApiBody, ApiOperation, ApiParam, ApiResponse, @@ -22,6 +23,7 @@ import { PermissionGuard } from '../../auth/permission.guard'; import { RequirePermission } from '../../auth/require-permission.decorator'; import { TasksService } from '../tasks.service'; import { AutomationsService } from './automations.service'; +import { CreateVersionDto } from './dto/create-version.dto'; import { UpdateAutomationDto } from './dto/update-automation.dto'; import { AUTOMATION_OPERATIONS } from './schemas/automation-operations'; import { CREATE_AUTOMATION_RESPONSES } from './schemas/create-automation.responses'; @@ -255,11 +257,12 @@ export class AutomationsController { }) @ApiParam({ name: 'taskId', description: 'Task ID' }) @ApiParam({ name: 'automationId', description: 'Automation ID' }) + @ApiBody({ type: CreateVersionDto }) async createVersion( @OrganizationId() organizationId: string, @Param('taskId') taskId: string, @Param('automationId') automationId: string, - @Body() body: { version: number; scriptKey: string; changelog?: string }, + @Body() body: CreateVersionDto, ) { await this.tasksService.verifyTaskAccess(organizationId, taskId); return this.automationsService.createVersion(automationId, body); diff --git a/apps/api/src/tasks/automations/automations.service.spec.ts b/apps/api/src/tasks/automations/automations.service.spec.ts new file mode 100644 index 0000000000..f7a2746ae2 --- /dev/null +++ b/apps/api/src/tasks/automations/automations.service.spec.ts @@ -0,0 +1,83 @@ +import { ConflictException, NotFoundException } from '@nestjs/common'; + +// Mock the DB layer before importing the service. We also provide a stand-in +// Prisma.PrismaClientKnownRequestError so the service's `instanceof` checks and +// error-code branches can be exercised without a real database. +jest.mock('@db', () => { + class PrismaClientKnownRequestError extends Error { + code: string; + constructor(message: string, { code }: { code: string }) { + super(message); + this.code = code; + this.name = 'PrismaClientKnownRequestError'; + } + } + + return { + db: { + $transaction: jest.fn(), + evidenceAutomationVersion: { create: jest.fn() }, + evidenceAutomation: { update: jest.fn() }, + }, + Prisma: { PrismaClientKnownRequestError }, + }; +}); + +import { db, Prisma } from '@db'; +import { AutomationsService } from './automations.service'; + +const prismaError = (code: string) => + new Prisma.PrismaClientKnownRequestError(code, { + code, + clientVersion: '5.0.0', + }); + +describe('AutomationsService.createVersion — error mapping', () => { + let service: AutomationsService; + const input = { version: 1, scriptKey: 'org_1/tsk_1/aut_1.v1.js' }; + + beforeEach(() => { + jest.clearAllMocks(); + service = new AutomationsService(); + }); + + it('records the version and returns it on success', async () => { + const created = { id: 'eav_1', version: 1, scriptKey: input.scriptKey }; + (db.$transaction as jest.Mock).mockResolvedValue([created, { id: 'aut_1' }]); + + const result = await service.createVersion('aut_1', input); + + expect(result).toEqual({ success: true, version: created }); + }); + + it('maps a duplicate version (P2002) to a 409 ConflictException', async () => { + (db.$transaction as jest.Mock).mockRejectedValue(prismaError('P2002')); + + await expect(service.createVersion('aut_1', input)).rejects.toBeInstanceOf( + ConflictException, + ); + }); + + it('maps a missing automation (P2003 FK violation) to a 404 NotFoundException', async () => { + (db.$transaction as jest.Mock).mockRejectedValue(prismaError('P2003')); + + await expect( + service.createVersion('missing', input), + ).rejects.toBeInstanceOf(NotFoundException); + }); + + it('maps a missing automation (P2025 record not found) to a 404 NotFoundException', async () => { + (db.$transaction as jest.Mock).mockRejectedValue(prismaError('P2025')); + + await expect( + service.createVersion('missing', input), + ).rejects.toBeInstanceOf(NotFoundException); + }); + + it('rethrows unexpected errors untouched (no masking real 500s)', async () => { + const boom = new Error('db exploded'); + (db.$transaction as jest.Mock).mockRejectedValue(boom); + + await expect(service.createVersion('aut_1', input)).rejects.toBe(boom); + }); +}); diff --git a/apps/api/src/tasks/automations/automations.service.ts b/apps/api/src/tasks/automations/automations.service.ts index 86a6fd8ee9..1e8bf00254 100644 --- a/apps/api/src/tasks/automations/automations.service.ts +++ b/apps/api/src/tasks/automations/automations.service.ts @@ -1,5 +1,10 @@ -import { Injectable, NotFoundException } from '@nestjs/common'; -import { db } from '@db'; +import { + ConflictException, + Injectable, + NotFoundException, +} from '@nestjs/common'; +import { db, Prisma } from '@db'; +import { CreateVersionDto } from './dto/create-version.dto'; import { UpdateAutomationDto } from './dto/update-automation.dto'; @Injectable() @@ -135,26 +140,40 @@ export class AutomationsService { }; } - async createVersion( - automationId: string, - data: { version: number; scriptKey: string; changelog?: string }, - ) { - const [version] = await db.$transaction([ - db.evidenceAutomationVersion.create({ - data: { - evidenceAutomationId: automationId, - version: data.version, - scriptKey: data.scriptKey, - changelog: data.changelog, - }, - }), - // Enable automation on publish if not already enabled - db.evidenceAutomation.update({ - where: { id: automationId }, - data: { isEnabled: true }, - }), - ]); - return { success: true, version }; + async createVersion(automationId: string, data: CreateVersionDto) { + try { + const [version] = await db.$transaction([ + db.evidenceAutomationVersion.create({ + data: { + evidenceAutomationId: automationId, + version: data.version, + scriptKey: data.scriptKey, + changelog: data.changelog, + }, + }), + // Enable automation on publish if not already enabled + db.evidenceAutomation.update({ + where: { id: automationId }, + data: { isEnabled: true }, + }), + ]); + return { success: true, version }; + } catch (err) { + if (err instanceof Prisma.PrismaClientKnownRequestError) { + // Duplicate (evidenceAutomationId, version) — version already published. + if (err.code === 'P2002') { + throw new ConflictException( + `Version ${data.version} already exists for this automation`, + ); + } + // Automation row missing — FK on create (P2003) or update target gone + // (P2025). Surface a clean 404 instead of a raw 500. + if (err.code === 'P2003' || err.code === 'P2025') { + throw new NotFoundException(`Automation ${automationId} not found`); + } + } + throw err; + } } async findRunsByAutomationId(automationId: string) { diff --git a/apps/api/src/tasks/automations/dto/create-version.dto.spec.ts b/apps/api/src/tasks/automations/dto/create-version.dto.spec.ts new file mode 100644 index 0000000000..5396fff702 --- /dev/null +++ b/apps/api/src/tasks/automations/dto/create-version.dto.spec.ts @@ -0,0 +1,66 @@ +import { plainToInstance } from 'class-transformer'; +import { validate } from 'class-validator'; +import { CreateVersionDto } from './create-version.dto'; + +/** + * The original endpoint accepted an inline, untyped `@Body()` — invisible to the + * ValidationPipe — so a missing `version`/`scriptKey` slipped through and blew up + * with a Prisma non-null violation (500). These tests prove the DTO now rejects + * those payloads at the validation layer (400) before they reach the service. + */ +describe('CreateVersionDto', () => { + async function validatePayload(payload: Record) { + return validate(plainToInstance(CreateVersionDto, payload)); + } + + it('accepts a valid payload', async () => { + const errors = await validatePayload({ + version: 1, + scriptKey: 'org_1/tsk_1/aut_1.v1.js', + changelog: 'initial publish', + }); + expect(errors).toHaveLength(0); + }); + + it('rejects a missing version (previously a 500)', async () => { + const errors = await validatePayload({ scriptKey: 'k' }); + expect(errors.some((e) => e.property === 'version')).toBe(true); + }); + + it('rejects a missing scriptKey (previously a 500)', async () => { + const errors = await validatePayload({ version: 1 }); + expect(errors.some((e) => e.property === 'scriptKey')).toBe(true); + }); + + it('rejects a version below 1', async () => { + const errors = await validatePayload({ version: 0, scriptKey: 'k' }); + expect(errors.some((e) => e.property === 'version')).toBe(true); + }); + + it('rejects an empty scriptKey', async () => { + const errors = await validatePayload({ version: 1, scriptKey: '' }); + expect(errors.some((e) => e.property === 'scriptKey')).toBe(true); + }); + + it('rejects a whitespace-only scriptKey (would otherwise persist a blank key)', async () => { + for (const scriptKey of [' ', '\t\n', '   ']) { + const errors = await validatePayload({ version: 1, scriptKey }); + expect(errors.some((e) => e.property === 'scriptKey')).toBe(true); + } + }); + + it('trims surrounding whitespace from a valid scriptKey', async () => { + const dto = plainToInstance(CreateVersionDto, { + version: 1, + scriptKey: ' org_1/tsk_1/aut_1.v1.js ', + }); + const errors = await validate(dto); + expect(errors).toHaveLength(0); + expect(dto.scriptKey).toBe('org_1/tsk_1/aut_1.v1.js'); + }); + + it('treats changelog as optional', async () => { + const errors = await validatePayload({ version: 2, scriptKey: 'k' }); + expect(errors.some((e) => e.property === 'changelog')).toBe(false); + }); +}); diff --git a/apps/api/src/tasks/automations/dto/create-version.dto.ts b/apps/api/src/tasks/automations/dto/create-version.dto.ts new file mode 100644 index 0000000000..8bc521f111 --- /dev/null +++ b/apps/api/src/tasks/automations/dto/create-version.dto.ts @@ -0,0 +1,40 @@ +import { ApiProperty } from '@nestjs/swagger'; +import { Transform } from 'class-transformer'; +import { IsInt, IsNotEmpty, IsOptional, IsString, Min } from 'class-validator'; + +/** + * Records that an automation script has been generated + published to S3. + * `version` and `scriptKey` are REQUIRED — the row references an already-stored + * script. The web UI's publish flow supplies both from the enterprise publish + * step; calling this without them used to 500 (Prisma non-null violation). + */ +export class CreateVersionDto { + @ApiProperty({ + description: 'Version number for this published script', + example: 1, + }) + @IsInt() + @Min(1) + version!: number; + + @ApiProperty({ + description: + 'S3 key of the already-generated & published automation script (returned by the publish step).', + example: 'org_abc123/tsk_abc123/aut_abc123.v1.js', + }) + @IsString() + // Trim first so a whitespace-only key collapses to '' and @IsNotEmpty rejects + // it — otherwise a blank key would persist and the automation would later + // fail to fetch a script at that key. Non-strings pass through for @IsString. + @Transform(({ value }) => (typeof value === 'string' ? value.trim() : value)) + @IsNotEmpty() + scriptKey!: string; + + @ApiProperty({ + description: 'Optional changelog describing this version', + required: false, + }) + @IsOptional() + @IsString() + changelog?: string; +} diff --git a/apps/mcp-server/.speakeasy/mcp-uploads-overlay.yaml b/apps/mcp-server/.speakeasy/mcp-uploads-overlay.yaml index 083332aba5..8e470deeda 100644 --- a/apps/mcp-server/.speakeasy/mcp-uploads-overlay.yaml +++ b/apps/mcp-server/.speakeasy/mcp-uploads-overlay.yaml @@ -93,3 +93,22 @@ actions: remove: true - target: "$.components.schemas.CompleteChecklistItemDto.properties.fileData" remove: true + + # 10-11. Disable the automation-authoring tools for MCP. Creating a working + # automation requires generating its evidence-collection SCRIPT, and that + # step lives ONLY in the enterprise-api browser chat (an AI tool writes the + # .draft.js to S3, then publish copies it to .vN.js). Neither the public API + # nor MCP can produce that script. So over MCP these two POSTs only ever + # create an empty automation shell + a version row pointing at a script that + # was never generated — a dead end that confuses agents. Hide them until the + # script-generation step is exposed as a first-class (async) endpoint. + # The HTTP endpoints stay live for the web UI, which drives the full flow. + # Read/list/update automation tools remain available over MCP. + - target: "$.paths['/v1/tasks/{taskId}/automations'].post" + update: + x-speakeasy-mcp: + disabled: true + - target: "$.paths['/v1/tasks/{taskId}/automations/{automationId}/versions'].post" + update: + x-speakeasy-mcp: + disabled: true diff --git a/packages/docs/openapi.json b/packages/docs/openapi.json index da59719315..354d460138 100644 --- a/packages/docs/openapi.json +++ b/packages/docs/openapi.json @@ -1,5 +1,6 @@ { "openapi": "3.0.0", + "x-speakeasy-timeout": 120000, "paths": { "/v1/organization": { "get": {