diff --git a/package.json b/package.json index 0cee43a70..e8da264f0 100644 --- a/package.json +++ b/package.json @@ -98,7 +98,7 @@ "test:hosted-tunnel:server": "PROPR_DEMO_MODE=true npx tsx --test test/orchestratorConfig.test.mjs packages/cli/src/commands/setup/engine.test.ts packages/cli/src/commands/setup/sequential.test.ts packages/cli/src/commands/setupCommand.test.ts packages/cli/src/tui/SetupApp.test.tsx packages/cli/src/commands/tunnelCommand.test.ts test/orchestratorTunnelLifecycle.test.mjs test/orchestratorTunnelStatus.test.mjs test/orchestratorProprUrlsDrift.test.ts test/routingWebSocketProtocol.test.ts test/routingWebSocketIntakeService.test.ts packages/api/test/corsValidation.test.ts", "test:hosted-tunnel:ui": "npm --workspace propr-ui test -- runtimeConfig.test.ts compatibility.test.ts", "test:hosted-tunnel": "npm run build -w @propr/shared && npm run build -w @propr/local-setup && npm run test:hosted-tunnel:server && npm run test:hosted-tunnel:ui", - "test:mcp": "NODE_ENV=test npx tsx --experimental-test-module-mocks --test packages/api/test/mcpOAuth.test.ts packages/api/test/mcpOperations.test.ts packages/api/test/mcpTaskSubmissions.test.ts packages/api/test/mcpPresentation.test.ts packages/api/test/mcpConnectedApps.test.ts packages/api/test/mcpIntegration.test.ts packages/api/test/mcpAgentActivity.test.ts packages/api/test/mcpActivity.test.ts packages/api/test/mcpRecentActivity.test.ts packages/api/test/mcpGoalTaskDepth.test.ts packages/api/test/mcpAccessLog.test.ts packages/api/test/mcpAccessLogDispatch.test.ts packages/api/test/mcpDelegation.test.ts packages/api/test/mcpWorkflows.test.ts packages/api/test/mcpPullRequests.test.ts packages/api/test/mcpOperatorSurface.test.ts packages/api/test/mcpReviewerConcurrency.test.ts test/mcpConnectHarness.test.mjs", + "test:mcp": "NODE_ENV=test npx tsx --experimental-test-module-mocks --test packages/api/test/mcpErrorEnvelope.test.ts packages/api/test/mcpOAuth.test.ts packages/api/test/mcpOperations.test.ts packages/api/test/mcpTaskSubmissions.test.ts packages/api/test/mcpPresentation.test.ts packages/api/test/mcpConnectedApps.test.ts packages/api/test/mcpIntegration.test.ts packages/api/test/mcpAgentActivity.test.ts packages/api/test/mcpActivity.test.ts packages/api/test/mcpRecentActivity.test.ts packages/api/test/mcpGoalTaskDepth.test.ts packages/api/test/mcpAccessLog.test.ts packages/api/test/mcpAccessLogDispatch.test.ts packages/api/test/mcpDelegation.test.ts packages/api/test/mcpWorkflows.test.ts packages/api/test/mcpPullRequests.test.ts packages/api/test/mcpOperatorSurface.test.ts packages/api/test/mcpReviewerConcurrency.test.ts test/mcpConnectHarness.test.mjs", "test:mcp:browser": "NODE_ENV=test npx tsx --test packages/api/test/mcpBrowser.test.ts", "test:mcp:connect": "node scripts/test-mcp-connect.mjs", "mcp:connect:register": "tsx scripts/mcp-connect-register.ts", diff --git a/packages/api/mcp/accessLog.ts b/packages/api/mcp/accessLog.ts index e46ab70b4..7e849846d 100644 --- a/packages/api/mcp/accessLog.ts +++ b/packages/api/mcp/accessLog.ts @@ -1,7 +1,6 @@ import { AsyncLocalStorage } from 'node:async_hooks'; import type { Knex } from 'knex'; -import { z } from 'zod'; -import { McpError } from './config.js'; +import { classifyError } from './errorEnvelope.js'; import type { McpPrincipal } from './policy.js'; /** @@ -172,12 +171,9 @@ export function accessPrincipal(principal?: Pick * scope, forbidden repository, invalid arguments) are denials; anything the * instance itself could not complete is an error. */ -export function classifyMcpFailure(error: unknown): { status: number; outcome: McpAccessOutcome; errorCode: string } { - if (error instanceof McpError) { - return { status: error.status, outcome: error.status >= 500 ? 'error' : 'denied', errorCode: error.code }; - } - if (error instanceof z.ZodError) return { status: 400, outcome: 'denied', errorCode: 'INVALID_INPUT' }; - return { status: 500, outcome: 'error', errorCode: 'INTERNAL_ERROR' }; +export function classifyMcpFailure(error: unknown, options: { sideEffectsPossible?: boolean } = {}): { status: number; outcome: McpAccessOutcome; errorCode: string } { + const envelope = classifyError(error, { sideEffectsPossible: options.sideEffectsPossible ?? false }); + return { status: envelope.status, outcome: envelope.code === 'OUTCOME_UNKNOWN' || envelope.status >= 500 ? 'error' : 'denied', errorCode: envelope.code }; } /** JSON-RPC request identifiers are scalars; anything else is not correlatable. */ diff --git a/packages/api/mcp/config.ts b/packages/api/mcp/config.ts index 7c3070ae3..231106af8 100644 --- a/packages/api/mcp/config.ts +++ b/packages/api/mcp/config.ts @@ -1,3 +1,5 @@ +import { redactDetails, redactSecrets, type McpErrorEnvelope, type McpErrorStage } from './errorEnvelope.js'; + export const MCP_SCOPES = ['read', 'plan', 'publish', 'execute', 'review', 'merge', 'deploy', 'manage'] as const; export type McpScope = typeof MCP_SCOPES[number]; @@ -35,7 +37,33 @@ export function loadMcpConfig(env: NodeJS.ProcessEnv = process.env): McpConfig | } export class McpError extends Error { - constructor(public readonly code: string, message: string, public readonly status = 400) { super(message); } + readonly stage: McpErrorStage | null; + readonly retryable: boolean; + readonly details?: Record; + + constructor( + public readonly code: string, + message: string, + public readonly status = 400, + options: { stage?: McpErrorStage | null; retryable?: boolean; details?: Record } = {}, + ) { + super(message); + this.name = 'McpError'; + this.stage = options.stage ?? null; + this.retryable = options.retryable ?? false; + this.details = options.details; + } + + toEnvelope(): McpErrorEnvelope { + return { + code: this.code, + message: redactSecrets(this.message), + stage: this.stage, + retryable: this.retryable, + status: this.status, + ...(this.details ? { details: redactDetails(this.details) } : {}), + }; + } } function loadConnectConfig(env: NodeJS.ProcessEnv, instanceId: string): McpConfig['connect'] { diff --git a/packages/api/mcp/errorEnvelope.ts b/packages/api/mcp/errorEnvelope.ts new file mode 100644 index 000000000..815325770 --- /dev/null +++ b/packages/api/mcp/errorEnvelope.ts @@ -0,0 +1,235 @@ +import { z } from 'zod'; +import type { McpError } from './config.js'; + +/** Stable locations a tool may expose without leaking implementation detail. */ +export type McpErrorStage = 'validation' | 'authorization' | 'precondition' | 'github' | 'transport' | 'database' | 'queue' | 'internal'; + +/** The one public and durable representation of an MCP tool failure. */ +export interface McpErrorEnvelope { + code: string; + message: string; + stage: McpErrorStage | null; + retryable: boolean; + status: number; + details?: Record; + cause?: { code: string; message: string }; +} + +const SENSITIVE_DETAIL_KEY = /token|secret|password|authorization|cookie|private.?key|credential/i; +const GITHUB_TOKEN = /\b(?:gh[pousr]_[A-Za-z0-9_]+|github_pat_[A-Za-z0-9_]+)\b/g; +const MCP_TOKEN = /\b(?:pia|propr)_mcp_[A-Za-z0-9._~-]+\b/g; +const BEARER_VALUE = /\bBearer\s+[^\s,;"']+/gi; +const CREDENTIALED_GIT_URL = /(https:\/\/x-access-token:)[^@\s/]+(@[^\s]+)/gi; +const SIGNED_QUERY_VALUE = /([?&](?:X-Amz-Signature|token|access_token|signature)=)[^&#\s]*/gi; + +/** Remove credentials that can occur in upstream messages and safe detail values. */ +export function redactSecrets(value: string): string { + return value + .replace(GITHUB_TOKEN, '[REDACTED]') + .replace(MCP_TOKEN, '[REDACTED]') + .replace(BEARER_VALUE, 'Bearer [REDACTED]') + .replace(CREDENTIALED_GIT_URL, '$1[REDACTED]$2') + .replace(SIGNED_QUERY_VALUE, '$1[REDACTED]'); +} + +function basename(value: string): string { + const withoutTrailingSeparator = value.replace(/[\\/]+$/, ''); + return withoutTrailingSeparator.split(/[\\/]/).pop() || '[REDACTED_PATH]'; +} + +function stripAbsolutePaths(value: string): string { + const posix = value.replace(/(^|[\s("'=])\/(?:[^/\s"'<>]+\/)+([^/\s"'<>]+)/g, (_match, prefix: string, filename: string) => `${prefix}${filename}`); + return posix.replace(/(^|[\s("'=])(?:[A-Za-z]:[\\/]|\\\\)(?:[^\\/\s"'<>]+[\\/])+([^\\/\s"'<>]+)/g, + (_match, prefix: string, filename: string) => `${prefix}${filename}`); +} + +function redactDetailValue(value: unknown, seen: WeakSet): unknown { + if (typeof value === 'string') { + const redacted = redactSecrets(value); + // Detail fields often carry paths directly or inside diagnostic prose. + // Keep only filenames for POSIX, Windows-drive and UNC absolute paths. + if (redacted.startsWith('/') || /^[A-Za-z]:[\\/]/.test(redacted)) return basename(redacted); + return stripAbsolutePaths(redacted); + } + if (Array.isArray(value)) { + if (seen.has(value)) return '[REDACTED]'; + seen.add(value); + return value.map(item => redactDetailValue(item, seen)); + } + if (value && typeof value === 'object') { + if (seen.has(value)) return '[REDACTED]'; + seen.add(value); + const result: Record = {}; + for (const [key, item] of Object.entries(value)) { + if (SENSITIVE_DETAIL_KEY.test(key)) continue; + result[redactSecrets(key)] = redactDetailValue(item, seen); + } + return result; + } + if (typeof value === 'bigint') return String(value); + return value; +} + +/** Recursively sanitize optional structured error detail. Sensitive keys are omitted. */ +export function redactDetails(details: Record): Record { + return redactDetailValue(details, new WeakSet()) as Record; +} + +type ErrorLike = { + name?: unknown; + code?: unknown; + message?: unknown; + status?: unknown; + request?: unknown; + response?: unknown; + cause?: unknown; +}; + +function errorLike(value: unknown): ErrorLike | undefined { + return value !== null && typeof value === 'object' ? value as ErrorLike : undefined; +} + +function isMcpError(value: unknown): value is McpError { + return value instanceof Error + && value.name === 'McpError' + && typeof (value as Partial).code === 'string' + && typeof (value as Partial).status === 'number' + && typeof (value as Partial).toEnvelope === 'function'; +} + +function record(value: unknown): Record | undefined { + return value !== null && typeof value === 'object' ? value as Record : undefined; +} + +function finiteStatus(value: unknown): number | undefined { + return typeof value === 'number' && Number.isFinite(value) ? value : undefined; +} + +function githubError(error: unknown): { value: ErrorLike; status: number; response?: Record } | undefined { + const value = errorLike(error); + if (!value) return undefined; + const response = record(value.response); + const status = finiteStatus(value.status) ?? finiteStatus(response?.status); + if (!status) return undefined; + // RequestError uses name=HttpError and carries request/response metadata. + // Accept response-bearing test doubles too, without mistaking every domain + // error with an HTTP-like status for a GitHub failure. + if (value.name !== 'HttpError' && value.request === undefined && !response) return undefined; + return { value, status, response }; +} + +function githubHeaders(response: Record | undefined): Record { + const source = record(response?.headers); + if (!source) return {}; + return Object.fromEntries(Object.entries(source).map(([key, value]) => [key.toLowerCase(), value])); +} + +function githubMessage(value: ErrorLike, response: Record | undefined): string { + const data = record(response?.data); + const primary = typeof data?.message === 'string' ? data.message + : typeof value.message === 'string' ? value.message : 'GitHub rejected the request.'; + const errors = Array.isArray(data?.errors) ? data.errors.flatMap(item => { + if (typeof item === 'string') return [item]; + const message = record(item)?.message; + return typeof message === 'string' ? [message] : []; + }) : []; + const additions = errors.filter(message => message && message !== primary); + return redactSecrets([primary, ...additions].join(': ')); +} + +function classifyGithub(error: unknown): McpErrorEnvelope | undefined { + const github = githubError(error); + if (!github) return undefined; + const { value, status, response } = github; + const headers = githubHeaders(response); + const rateLimited = status === 429 || (status === 403 && ( + String(headers['x-ratelimit-remaining'] ?? '') === '0' + || headers['retry-after'] !== undefined + )); + const common = { stage: 'github' as const, status, message: githubMessage(value, response) }; + if (rateLimited) return { code: 'GITHUB_RATE_LIMITED', retryable: true, ...common }; + if (status === 401) return { code: 'GITHUB_AUTH_FAILED', retryable: false, ...common }; + if (status === 403) return { code: 'GITHUB_FORBIDDEN', retryable: false, ...common }; + if (status === 404) return { code: 'GITHUB_NOT_FOUND', retryable: false, ...common }; + if (status >= 500) return { code: 'GITHUB_UNAVAILABLE', retryable: true, ...common }; + if (status === 409 || status === 422 || (status >= 400 && status < 500)) { + return { code: 'GITHUB_REJECTED', retryable: false, ...common }; + } + return undefined; +} + +function errorChain(error: unknown): ErrorLike[] { + const chain: ErrorLike[] = []; + const seen = new Set(); + let current: unknown = error; + while (current && !seen.has(current) && chain.length < 5) { + seen.add(current); + const value = errorLike(current); + if (!value) break; + chain.push(value); + current = value.cause; + } + return chain; +} + +function classifyKnown(error: unknown): McpErrorEnvelope { + if (isMcpError(error)) return error.toEnvelope(); + + if (error instanceof z.ZodError) { + return { + code: 'INVALID_INPUT', + message: 'Invalid or missing tool arguments.', + stage: 'validation', + retryable: false, + status: 400, + details: redactDetails({ issues: error.issues.map(issue => ({ + path: issue.path.map(part => typeof part === 'symbol' ? part.description ?? 'symbol' : part), + message: issue.message, + })) }), + }; + } + + const github = classifyGithub(error); + if (github) return github; + + const chain = errorChain(error); + if (chain.some(item => item.name === 'AbortError' || item.name === 'TimeoutError' || item.code === 'ETIMEDOUT')) { + return { code: 'UPSTREAM_TIMEOUT', message: 'The upstream request timed out.', stage: 'transport', retryable: true, status: 504 }; + } + if (chain.some(item => ['ECONNRESET', 'ECONNREFUSED'].includes(String(item.code)))) { + return { code: 'UPSTREAM_UNREACHABLE', message: 'The upstream service could not be reached.', stage: 'transport', retryable: true, status: 503 }; + } + if (chain.some(item => ['SQLITE_BUSY', 'SQLITE_LOCKED'].includes(String(item.code)))) { + return { code: 'DATABASE_BUSY', message: 'The database is temporarily busy.', stage: 'database', retryable: true, status: 503 }; + } + return { code: 'INTERNAL_ERROR', message: 'The request could not be completed.', stage: 'internal', retryable: false, status: 500 }; +} + +/** Classify any thrown value, retaining mutation uncertainty when effects may have occurred. */ +export function classifyError(error: unknown, options: { sideEffectsPossible: boolean }): McpErrorEnvelope { + const classified = classifyKnown(error); + if (!options.sideEffectsPossible || (isMcpError(error) && classified.status < 500)) return classified; + return { + code: 'OUTCOME_UNKNOWN', + message: 'Outcome uncertain. Inspect the target before issuing a new action.', + stage: classified.stage, + retryable: false, + status: classified.status, + cause: { code: classified.code, message: redactSecrets(classified.message) }, + }; +} + +/** Produce the protocol result understood by both text-only and structured clients. */ +export function toToolErrorResult(envelope: McpErrorEnvelope): { + isError: true; + content: [{ type: 'text'; text: string }]; + structuredContent: { error: McpErrorEnvelope }; +} { + const safe: McpErrorEnvelope = { + ...envelope, + message: redactSecrets(envelope.message), + ...(envelope.details ? { details: redactDetails(envelope.details) } : {}), + ...(envelope.cause ? { cause: { code: envelope.cause.code, message: redactSecrets(envelope.cause.message) } } : {}), + }; + return { isError: true, content: [{ type: 'text', text: JSON.stringify({ error: safe }) }], structuredContent: { error: safe } }; +} diff --git a/packages/api/mcp/operations.ts b/packages/api/mcp/operations.ts index 9f3d5de06..827ad39bb 100644 --- a/packages/api/mcp/operations.ts +++ b/packages/api/mcp/operations.ts @@ -1,6 +1,7 @@ import { randomUUID } from 'node:crypto'; import type { Knex } from 'knex'; import { McpError } from './config.js'; +import { classifyError } from './errorEnvelope.js'; import { digest } from './store.js'; import type { McpPrincipal } from './policy.js'; @@ -47,9 +48,9 @@ export class McpOperations { } catch (error) { // A transport failure can follow an external side effect. Never replay it // automatically or claim it was rolled back. The handle remains durable. - const code = error instanceof McpError && error.status < 500 ? error.code : 'OUTCOME_UNKNOWN'; - await this.db('mcp_operations').where({ id }).update({ state: code === 'OUTCOME_UNKNOWN' ? 'unknown' : 'failed', - result: JSON.stringify({ error: { code, message: error instanceof McpError ? error.message : 'Outcome uncertain. Inspect the target before issuing a new action.' } }), updated_at: Date.now() }); + const envelope = classifyError(error, { sideEffectsPossible: true }); + await this.db('mcp_operations').where({ id }).update({ state: envelope.code === 'OUTCOME_UNKNOWN' ? 'unknown' : 'failed', + result: JSON.stringify({ error: envelope }), updated_at: Date.now() }); } return this.project((await this.db('mcp_operations').where({ id }).first())!); } diff --git a/packages/api/mcp/server.ts b/packages/api/mcp/server.ts index b15437c27..2f77dcd5d 100644 --- a/packages/api/mcp/server.ts +++ b/packages/api/mcp/server.ts @@ -17,6 +17,7 @@ import { createToolCatalog, executeTool, type McpTool, type ToolDeps } from './t import { accessPrincipal, mcpRequestId, recordMcpAccess, withMcpDispatch, withMcpRequestContext, withMcpSurface } from './accessLog.js'; import { presentResultText } from './presentation.js'; import { resolveMcpConfig, isMcpEnabledSync, getMcpScopeCeilingSync } from './configResolver.js'; +import { classifyError, toToolErrorResult } from './errorEnvelope.js'; const prompts: Record = { plan_change: 'Resolve the repository and inspect indexed context. Create a draft plan, generate or refine it, and show it to the user. Publishing and implementation are separate explicit actions.', @@ -43,9 +44,7 @@ export function buildMcpServer(principal: McpPrincipal, deps: ToolDeps, catalog: const result = await call(tool.name, args); return { content: [{ type: 'text', text: presentResultText(result) }], structuredContent: result }; } catch (error) { - const code = error instanceof McpError ? error.code : error instanceof z.ZodError ? 'INVALID_INPUT' : 'INTERNAL_ERROR'; - const message = error instanceof McpError ? error.message : error instanceof z.ZodError ? 'Invalid or missing tool arguments.' : 'The request could not be completed.'; - return { isError: true, content: [{ type: 'text', text: `${code}: ${message}` }], structuredContent: { error: { code, message } } }; + return toToolErrorResult(classifyError(error, { sideEffectsPossible: false })); } }); } diff --git a/packages/api/mcp/toolExecution.ts b/packages/api/mcp/toolExecution.ts index eeb21fd82..265e8fc39 100644 --- a/packages/api/mcp/toolExecution.ts +++ b/packages/api/mcp/toolExecution.ts @@ -96,7 +96,7 @@ async function runTool({ tool, raw, principal, deps, access }: ToolInvocation): // instead of throwing, so the classification is captured here, before that // projection consumes it. : await new McpOperations(deps.db).run(principal, { tool: tool.name, args, repository: operationRepository }, - operationId => tool.run({ principal, args, operationId }).catch(error => { access.failure = classifyMcpFailure(error); throw error; }))); + operationId => tool.run({ principal, args, operationId }).catch(error => { access.failure = classifyMcpFailure(error, { sideEffectsPossible: true }); throw error; }))); const data = redact(result) as Record; noteToolOutcome(tool, access, data); if (access.resultBytes > 256 * 1024) throw new McpError('RESULT_TOO_LARGE', 'Request a smaller page or narrower target.'); diff --git a/packages/api/test/mcpAccessLogDispatch.test.ts b/packages/api/test/mcpAccessLogDispatch.test.ts index 5632f3d64..984581ac8 100644 --- a/packages/api/test/mcpAccessLogDispatch.test.ts +++ b/packages/api/test/mcpAccessLogDispatch.test.ts @@ -152,6 +152,11 @@ test('a failed mutation is recorded with its own outcome, on the first attempt a const mutation = (name: string, run: McpTool['run']): McpTool => ({ name, description: 'fixture', scope: 'read', schema, run }); const denied = mutation('denied_fixture', async () => { throw new McpError('REPOSITORY_FORBIDDEN', 'Denied', 403); }); const broken = mutation('broken_fixture', async () => { throw new Error('fixture failure'); }); + const mcpToken = 'propr_mcp_abcdefghijklmnopqrstuvwxyz'; + const githubRejected = mutation('github_rejected_fixture', async () => { throw Object.assign(new Error('request failed'), { + name: 'HttpError', status: 422, + response: { status: 422, headers: {}, data: { message: 'Validation Failed', errors: [{ message: `Rejected credential ${mcpToken}` }] } }, + }); }); const queued = mutation('queued_fixture', async () => ({ status: 202, data: { state: 'queued' } })); const args = { repository: 'acme/repo', idempotencyKey: 'mutation-fixture-1' }; @@ -160,13 +165,25 @@ test('a failed mutation is recorded with its own outcome, on the first attempt a const replayed = await executeTool(denied, args, principal(), deps); assert.equal((replayed.data as { operationId: string }).operationId, (receipt.data as { operationId: string }).operationId); await executeTool(broken, { ...args, idempotencyKey: 'mutation-fixture-2' }, principal(), deps); - await executeTool(queued, { ...args, idempotencyKey: 'mutation-fixture-3' }, principal(), deps); + const githubArgs = { ...args, idempotencyKey: 'mutation-fixture-3' }; + const githubReceipt = await executeTool(githubRejected, githubArgs, principal(), deps); + assert.equal((githubReceipt.data as { state: string }).state, 'unknown'); + assert.ok(!JSON.stringify(githubReceipt).includes(mcpToken)); + const operationId = (githubReceipt.data as { operationId: string }).operationId; + const persisted = await deps.db('mcp_operations').where({ id: operationId }).first('result'); + assert.ok(!String(persisted?.result).includes(mcpToken)); + assert.match(String(persisted?.result), /Rejected credential \[REDACTED\]/); + const githubReplay = await executeTool(githubRejected, githubArgs, principal(), deps); + assert.equal((githubReplay.data as { operationId: string }).operationId, operationId); + await executeTool(queued, { ...args, idempotencyKey: 'mutation-fixture-4' }, principal(), deps); const recorded = await rows(deps.db); assert.deepEqual(recorded.map(row => [row.name, row.outcome, row.error_code, row.status]), [ ['denied_fixture', 'denied', 'REPOSITORY_FORBIDDEN', 403], ['denied_fixture', 'denied', 'REPOSITORY_FORBIDDEN', 400], - ['broken_fixture', 'error', 'INTERNAL_ERROR', 500], + ['broken_fixture', 'error', 'OUTCOME_UNKNOWN', 500], + ['github_rejected_fixture', 'error', 'OUTCOME_UNKNOWN', 422], + ['github_rejected_fixture', 'error', 'OUTCOME_UNKNOWN', 500], ['queued_fixture', 'success', null, 200], ]); // Each row still carries the durable handle an operator follows. diff --git a/packages/api/test/mcpErrorEnvelope.test.ts b/packages/api/test/mcpErrorEnvelope.test.ts new file mode 100644 index 000000000..44956037f --- /dev/null +++ b/packages/api/test/mcpErrorEnvelope.test.ts @@ -0,0 +1,69 @@ +import assert from 'node:assert/strict'; +import { test } from 'node:test'; +import { z } from 'zod'; +import { McpError } from '../mcp/config.js'; +import { classifyError, redactDetails, toToolErrorResult } from '../mcp/errorEnvelope.js'; + +test('McpError keeps legacy defaults and round-trips optional envelope fields', () => { + assert.deepEqual(new McpError('STALE_HEAD', 'Pull request head changed.', 409).toEnvelope(), { + code: 'STALE_HEAD', message: 'Pull request head changed.', stage: null, retryable: false, status: 409, + }); + assert.deepEqual(new McpError('CHECKS_PENDING', 'Wait for checks.', 409, { + stage: 'precondition', retryable: true, details: { pending: 2 }, + }).toEnvelope(), { + code: 'CHECKS_PENDING', message: 'Wait for checks.', stage: 'precondition', retryable: true, status: 409, details: { pending: 2 }, + }); +}); + +test('validation, transport and database failures receive stable classifications', () => { + let zodError: unknown; + try { z.object({ repository: z.string() }).parse({}); } catch (error) { zodError = error; } + const invalid = classifyError(zodError, { sideEffectsPossible: false }); + assert.equal(invalid.code, 'INVALID_INPUT'); + assert.equal(invalid.stage, 'validation'); + assert.deepEqual((invalid.details?.issues as Array<{ path: unknown }>)[0].path, ['repository']); + assert.equal(classifyError(Object.assign(new Error('late'), { code: 'ETIMEDOUT' }), { sideEffectsPossible: false }).code, 'UPSTREAM_TIMEOUT'); + assert.equal(classifyError(Object.assign(new Error('busy'), { code: 'SQLITE_BUSY' }), { sideEffectsPossible: false }).code, 'DATABASE_BUSY'); +}); + +test('GitHub rejections retain safe detail and mutation uncertainty', () => { + const mcpToken = 'propr_mcp_abcdefghijklmnopqrstuvwxyz'; + const github = Object.assign(new Error('request failed'), { + name: 'HttpError', status: 422, + response: { status: 422, headers: {}, data: { message: `Validation Failed for ${mcpToken}`, errors: [{ message: 'Reference does not exist' }] } }, + }); + const read = classifyError(github, { sideEffectsPossible: false }); + assert.equal(read.code, 'GITHUB_REJECTED'); + assert.equal(read.message, 'Validation Failed for [REDACTED]: Reference does not exist'); + const mutation = classifyError(github, { sideEffectsPossible: true }); + assert.equal(mutation.code, 'OUTCOME_UNKNOWN'); + assert.equal(mutation.retryable, false); + assert.equal(mutation.stage, 'github'); + assert.deepEqual(mutation.cause, { code: 'GITHUB_REJECTED', message: 'Validation Failed for [REDACTED]: Reference does not exist' }); + const result = toToolErrorResult(read); + assert.equal(result.isError, true); + assert.equal(result.structuredContent.error.code, 'GITHUB_REJECTED'); + assert.deepEqual(JSON.parse(result.content[0].text), result.structuredContent); + assert.ok(!result.content[0].text.includes(mcpToken)); + assert.ok(!JSON.stringify(result.structuredContent).includes(mcpToken)); +}); + +test('all envelope strings and details redact credentials and local paths', () => { + const input = [ + 'ghp_abcdefghijklmnopqrstuvwxyz123456', + 'github_pat_abcdefghijklmnopqrstuvwxyz_123456', + 'Bearer abc.def', + 'https://x-access-token:abc@github.com/acme/repo.git', + 'https://example.test/file?X-Amz-Signature=abc&token=def', + 'pia_mcp_abcdefghijklmnopqrstuvwxyz', + 'propr_mcp_abcdefghijklmnopqrstuvwxyz', + ].join(' '); + const details = redactDetails({ message: `${input} failed at /home/user/private/source.ts`, accessToken: 'never-visible', nested: { password: 'never-visible' }, path: '/home/user/private/file.txt' }); + const envelope = new McpError('SAFE_ERROR', input, 400, { details }).toEnvelope(); + const serialized = JSON.stringify(toToolErrorResult(envelope)); + for (const secret of ['ghp_', 'github_pat_', 'Bearer abc', 'x-access-token:abc', 'X-Amz-Signature=abc', 'token=def', 'pia_mcp_', 'propr_mcp_', 'accessToken', 'never-visible', '/home/user']) { + assert.ok(!serialized.includes(secret), secret); + } + assert.equal(envelope.details?.path, 'file.txt'); + assert.match(String(envelope.details?.message), /source\.ts/); +}); diff --git a/packages/api/test/mcpOperations.test.ts b/packages/api/test/mcpOperations.test.ts index 9e3d6782a..112ce64a1 100644 --- a/packages/api/test/mcpOperations.test.ts +++ b/packages/api/test/mcpOperations.test.ts @@ -7,6 +7,7 @@ import knex from 'knex'; import { up } from '../../core/src/db/migrations/20260910220000_add_mcp.js'; import { McpOperations } from '../mcp/operations.js'; import { McpError } from '../mcp/config.js'; +import { callWorkflow } from '../mcp/adapter.js'; import { parseClientMetadataDocument } from '../mcp/clients.js'; test('mutation deduplication survives concurrent callers and reopening the SQLite database', async () => { @@ -31,8 +32,27 @@ test('mutation deduplication survives concurrent callers and reopening the SQLit await assert.rejects(restarted.get({ user: { id: '999' }, grant: { id: 'grant-1' } } as never, String(result.operationId)), /not found/); const uncertain = await restarted.run(principal, { tool: 'external_action', args: { idempotencyKey: 'uncertain-key-1' }, repository: 'acme/repo' }, async () => { throw new Error('Network disconnected after possible side effect'); }); assert.equal(uncertain.state, 'unknown'); + assert.deepEqual(uncertain.result, { error: { + code: 'OUTCOME_UNKNOWN', message: 'Outcome uncertain. Inspect the target before issuing a new action.', stage: 'internal', retryable: false, status: 500, + cause: { code: 'INTERNAL_ERROR', message: 'The request could not be completed.' }, + } }); + const githubRejected = await restarted.run(principal, { tool: 'publish_plan', args: { idempotencyKey: 'github-reject-1' }, repository: 'acme/repo' }, async () => { + throw Object.assign(new Error('request failed'), { name: 'HttpError', status: 422, + response: { status: 422, headers: {}, data: { message: 'Validation Failed', errors: [{ message: 'Reference does not exist' }] } } }); + }); + assert.deepEqual(githubRejected.result, { error: { + code: 'OUTCOME_UNKNOWN', message: 'Outcome uncertain. Inspect the target before issuing a new action.', stage: 'github', retryable: false, status: 422, + cause: { code: 'GITHUB_REJECTED', message: 'Validation Failed: Reference does not exist' }, + } }); const rejected = await restarted.run(principal, { tool: 'guarded_action', args: { idempotencyKey: 'rejected-key-1' }, repository: 'acme/repo' }, async () => { throw new McpError('STALE_HEAD', 'Head changed', 409); }); assert.equal(rejected.state, 'failed'); + const workflowFailure = await restarted.run(principal, { tool: 'workflow_action', args: { idempotencyKey: 'workflow-failure-1' }, repository: 'acme/repo' }, async () => + callWorkflow(async (_req, res) => { res.status(500).json({ error: 'The workflow may have changed the target.' }); }, principal, {})); + assert.equal(workflowFailure.state, 'unknown'); + assert.deepEqual(workflowFailure.result, { error: { + code: 'OUTCOME_UNKNOWN', message: 'Outcome uncertain. Inspect the target before issuing a new action.', stage: null, retryable: false, status: 500, + cause: { code: 'WORKFLOW_REJECTED', message: 'Workflow failed; inspect the operation and target before retrying.' }, + } }); await db('task_drafts').insert({ draft_id: 'plan-1', name: 'Initial' }); await db('task_drafts').where({ draft_id: 'plan-1' }).update({ name: 'Browser edit' }); await db('task_drafts').where({ draft_id: 'plan-1' }).update({ name: 'Background edit' });