diff --git a/packages/local-runtime-v2/src/service/turn-system/agent-host/runner/local-agent-turn-runner.ts b/packages/local-runtime-v2/src/service/turn-system/agent-host/runner/local-agent-turn-runner.ts index f54b1f6a2..d74e9b03d 100644 --- a/packages/local-runtime-v2/src/service/turn-system/agent-host/runner/local-agent-turn-runner.ts +++ b/packages/local-runtime-v2/src/service/turn-system/agent-host/runner/local-agent-turn-runner.ts @@ -1,3 +1,4 @@ +import { logger } from '../../../../infra/logging/index.js'; import { PiTurnRunner, type LLMModelConfig, @@ -633,6 +634,16 @@ async function settleNetworkStop( safetyWriter: LocalOutputSafetyEventWriter, ): Promise { if (safetyWriter.terminalFailed) return; + logger.warn( + { + sessionId: input.sessionId, + turnId: input.turnId, + reviewOutcome: safetyWriter.reviewStopVariant === 'auth' ? 'auth_error' : 'unavailable', + outputSuppressed: true, + terminalStatus: 'completed', + }, + '[content-safety] output review stopped turn', + ); state.networkStopped = true; state.retractionVariant = safetyWriter.reviewStopVariant; state.approvedPartial = safetyWriter.getApprovedPartial(); diff --git a/packages/local-runtime/src/content-safety/api-v2.ts b/packages/local-runtime/src/content-safety/api-v2.ts index b849ead27..3122d8df4 100644 --- a/packages/local-runtime/src/content-safety/api-v2.ts +++ b/packages/local-runtime/src/content-safety/api-v2.ts @@ -16,6 +16,7 @@ import { managedBackendRoutingHeaders, type LocalRuntimeRoutingContext, } from '../runtime/routing-headers.js'; +import { createSafetyFailureReporter } from './diagnostics.js'; import { resolveSafetyApiBase } from './api-base.js'; /** biz-gateway uses V2 numeric scenes with its desktop HTTP envelope. */ @@ -50,26 +51,44 @@ export async function callLocalSafetyCheckV2(input: { const buildEnv = (input.buildEnv ?? getRuntimeBuildEnv)(); const region = (input.region ?? getRuntimeRegion)(); const token = input.authContext?.accessToken?.trim() || process.env.MAVIS_ACCESS_TOKEN?.trim(); - const result = await postSafetyCheckV2({ - url: `${resolveSafetyApiBase(region, buildEnv, input.testBaseURL)}/mavis/api/v2/content?require_auth=true`, - headers: { - 'User-Agent': 'MiniMaxAgent', - ...(token ? { Authorization: `Bearer ${token}` } : {}), - ...managedBackendRoutingHeaders(input.routingContext, input.buildEnv ? buildEnv : undefined), - }, - body: { - scene: input.request.scene, - ...(input.request.content_text !== undefined - ? { content_text: input.request.content_text } - : {}), - ...(input.request.sessionId !== undefined ? { sessionId: input.request.sessionId } : {}), - ...(input.request.image_url !== undefined ? { image_url: input.request.image_url } : {}), - ...(input.request.files !== undefined ? { files: input.request.files } : {}), - }, - fetchImpl: input.fetchImpl, - signal: input.signal, - }); - return parseLocalSafetyCheckV2Result(result); + const url = `${resolveSafetyApiBase(region, buildEnv, input.testBaseURL)}/mavis/api/v2/content?require_auth=true`; + const reportFailure = createSafetyFailureReporter(url, 'v2', input.request.scene); + try { + const result = await postSafetyCheckV2({ + url, + headers: { + 'User-Agent': 'MiniMaxAgent', + ...(token ? { Authorization: `Bearer ${token}` } : {}), + ...managedBackendRoutingHeaders( + input.routingContext, + input.buildEnv ? buildEnv : undefined, + ), + }, + body: { + scene: input.request.scene, + ...(input.request.content_text !== undefined + ? { content_text: input.request.content_text } + : {}), + ...(input.request.sessionId !== undefined ? { sessionId: input.request.sessionId } : {}), + ...(input.request.image_url !== undefined ? { image_url: input.request.image_url } : {}), + ...(input.request.files !== undefined ? { files: input.request.files } : {}), + }, + fetchImpl: input.fetchImpl, + signal: input.signal, + }); + return parseLocalSafetyCheckV2Result(result); + } catch (error) { + reportFailure( + error instanceof SafetyCheckV2Error + ? { + failureKind: error.kind, + statusCode: error.statusCode, + transportKind: error.transportKind, + } + : { failureKind: 'internal' }, + ); + throw error; + } } function parseLocalSafetyCheckV2Result(body: Record): LocalSafetyCheckV2Result { diff --git a/packages/local-runtime/src/content-safety/api.ts b/packages/local-runtime/src/content-safety/api.ts index d613cbb76..ba832b0fb 100644 --- a/packages/local-runtime/src/content-safety/api.ts +++ b/packages/local-runtime/src/content-safety/api.ts @@ -1,3 +1,8 @@ +import { + classifySafetyTransportError, + isSafetyCheckV2Record, +} from "@mavis/shared/safety-check-v2"; +import { createSafetyFailureReporter } from "./diagnostics.js"; import { getRuntimeBuildEnv, getRuntimeRegion, @@ -8,7 +13,6 @@ import { import { callTurnSafetyApi } from "./turn-api.js"; import { resolveSafetyApiBase } from "./api-base.js"; -import { logger } from "../common/logger.js"; import type { LocalRuntimeAuthContext } from "../runtime/model-resolver.js"; import { managedBackendRoutingHeaders, @@ -126,12 +130,7 @@ export async function callSafetyApi(input: { const accessToken = input.authContext?.accessToken?.trim() || process.env.MAVIS_ACCESS_TOKEN?.trim(); - // TODO(shared-oauth): Remove this local-only diagnostic after the content-safety - // resource server accepts mcode-public Bearer tokens in the joint test environment. - // Never add the token, request body/content, or response body to these fields. - const logLocalUpstream = - resolvedBuildEnv === "dev" || resolvedBuildEnv === "test"; - const startedAt = Date.now(); + const reportFailure = createSafetyFailureReporter(url, "v1", input.scene); let response: Response; try { response = await input.fetchImpl(url, { @@ -162,20 +161,10 @@ export async function callSafetyApi(input: { signal: AbortSignal.timeout(CONTENT_REVIEW_TIMEOUT_MS), }); } catch (error) { - if (logLocalUpstream) { - logger.info( - { - url, - method: "POST", - status: null, - scene: input.scene, - hasBearer: Boolean(accessToken), - durationMs: Date.now() - startedAt, - errorType: error instanceof Error ? error.name : typeof error, - }, - "[content-safety] upstream transport failure", - ); - } + reportFailure({ + failureKind: "transport", + transportKind: classifySafetyTransportError(error), + }); // fetch threw (offline / DNS / connection refused / TLS) or the 10s timeout // fired: we never reached a verdict. No gateway confirmation → fail closed. return { @@ -185,26 +174,21 @@ export async function callSafetyApi(input: { }; } - if (logLocalUpstream) { - logger.info( - { - url, - method: "POST", - status: response.status, - scene: input.scene, - hasBearer: Boolean(accessToken), - durationMs: Date.now() - startedAt, - upstreamTraceId: - response.headers.get("trace-id") ?? - response.headers.get("x-trace-id") ?? - response.headers.get("x-request-id") ?? - null, - }, - "[content-safety] upstream response", - ); + let text: string; + try { + text = await response.text(); + } catch (error) { + reportFailure({ + failureKind: "transport", + transportKind: classifySafetyTransportError(error), + statusCode: response.status, + }); + return { + pass: false, + reason: "Service response could not be read", + errorKind: "local_error", + }; } - - const text = await response.text(); if (!response.ok) { // Non-2xx from the gateway, split by class so an auth failure can never be a // bypass: @@ -215,6 +199,10 @@ export async function callSafetyApi(input: { // acceptable, so there is NO valid verdict on our side → `local_error` // (fail closed). Treating 401/403 as `api_error` would let an // unauthenticated user bypass every content gate. + reportFailure({ + failureKind: response.status === 401 || response.status === 403 ? "auth" : "http", + statusCode: response.status, + }); const error = readErrorText(text); return { pass: false, @@ -240,6 +228,7 @@ export async function callSafetyApi(input: { try { data = JSON.parse(text) as typeof data; } catch { + reportFailure({ failureKind: "response", statusCode: response.status }); // 2xx but the body is not JSON — we cannot read a verdict. Treat as no // gateway confirmation → fail closed on the input path. return { @@ -249,8 +238,9 @@ export async function callSafetyApi(input: { }; } - const pass = data.pass ?? data.safe; + const pass = isSafetyCheckV2Record(data) ? data.pass ?? data.safe : undefined; if (typeof pass !== "boolean") { + reportFailure({ failureKind: "response", statusCode: response.status }); // 2xx JSON but no boolean pass/safe field — again no usable verdict → fail // closed on the input path. return { @@ -262,6 +252,9 @@ export async function callSafetyApi(input: { const activeRegion = region(); const upstreamCode = typeof data.errorCode === "number" ? data.errorCode : undefined; + if (!pass && upstreamCode === UPSTREAM_CODE.SERVICE_UNAVAILABLE) { + reportFailure({ failureKind: "upstream", statusCode: upstreamCode }); + } return { pass, reason: diff --git a/packages/local-runtime/src/content-safety/diagnostics.ts b/packages/local-runtime/src/content-safety/diagnostics.ts new file mode 100644 index 000000000..0cd664630 --- /dev/null +++ b/packages/local-runtime/src/content-safety/diagnostics.ts @@ -0,0 +1,27 @@ +import { randomUUID } from 'node:crypto'; +import type { SafetyTransportKind } from '@mavis/shared/safety-check-v2'; +import { logger } from '../common/logger.js'; + +/** One bounded record per failed check, in every build. Never log raw upstream data. */ +export function createSafetyFailureReporter(url: string, apiVersion: 'v1' | 'v2', scene: number) { + const startedAt = Date.now(); + const reviewId = randomUUID(); + const endpointHost = new URL(url).hostname; + return (failure: { + failureKind: 'transport' | 'auth' | 'http' | 'response' | 'upstream' | 'internal'; + transportKind?: SafetyTransportKind; + statusCode?: number; + }): void => { + logger.warn( + { + reviewId, + endpointHost, + apiVersion, + scene, + durationMs: Date.now() - startedAt, + ...failure, + }, + '[content-safety] review failed', + ); + }; +} diff --git a/packages/local-runtime/src/runtime/host.ts b/packages/local-runtime/src/runtime/host.ts index 13f4faa05..670f56342 100644 --- a/packages/local-runtime/src/runtime/host.ts +++ b/packages/local-runtime/src/runtime/host.ts @@ -1,3 +1,4 @@ +import { logger } from "../common/logger.js"; import type { LocalEventWriter } from "../events/sink.js"; import { PiTurnRunner } from "@mavis/agent-core/pi-turn-runner"; import { RuntimeEventStatus, RuntimeEventType } from "@mavis/protocol"; @@ -437,6 +438,16 @@ export class LocalRuntimeHost { // writer, so keep it authoritative and do not append a synthetic // COMPLETED/network-notice outcome. if (safetyWriter.terminalFailed) break; + logger.warn( + { + sessionId: input.sessionId, + turnId: input.turnId, + reviewOutcome: "unavailable", + outputSuppressed: true, + terminalStatus: "completed", + }, + "[content-safety] output review stopped turn", + ); networkStopped = true; retractionVariant = "network"; // Capture the approved prefix so the orchestration can rewrite pi history diff --git a/packages/local-runtime/test/unit/content-safety-api-fail-policy.test.ts b/packages/local-runtime/test/unit/content-safety-api-fail-policy.test.ts index 712dc321b..2a299ec14 100644 --- a/packages/local-runtime/test/unit/content-safety-api-fail-policy.test.ts +++ b/packages/local-runtime/test/unit/content-safety-api-fail-policy.test.ts @@ -276,46 +276,70 @@ describe('callSafetyApi fail-policy classification', () => { expect(headers.get('Token')).toBeNull(); }); - it('logs a sanitized upstream response in local test environments', async () => { - const log = vi.spyOn(logger, 'info').mockImplementation(() => {}); - const fetchImpl = vi.fn(async () => - new Response(JSON.stringify({ code: 401, message: 'auth failed' }), { - status: 401, - headers: { - 'Content-Type': 'application/json', - 'Trace-Id': 'trace-content-safety-test', - }, - }), - ) as unknown as typeof fetch; - - await callSafetyApi({ - content: 'must-not-appear-in-log', - scene: SAFETY_SCENE.UserInput, - authContext: { accessToken: 'must-not-appear-token' }, - fetchImpl, - region, - buildEnv, - }); - - expect(log).toHaveBeenCalledWith( - expect.objectContaining({ - url: 'https://matrix-test.example.invalid/mavis/api/v1/content', - method: 'POST', - status: 401, + it.each([ + ['auth', () => jsonResponse({ error: 'private-response' }, 401), 401], + ['http', () => jsonResponse({ error: 'private-response' }, 503), 503], + ['response', () => new Response('private-response'), 200], + ['response', () => jsonResponse(null), 200], + ['response', () => jsonResponse({ secret: 'private-response' }), 200], + [ + 'transport', + () => { + throw new TypeError('private-error https://secret.invalid/?token=private-token', { + cause: { code: 'ENOTFOUND' }, + }); + }, + undefined, + ], + [ + 'transport', + () => + new Response( + new ReadableStream({ + start(controller) { + controller.error(new DOMException('private-error', 'TimeoutError')); + }, + }), + ), + 200, + ], + ] as const)( + 'logs redacted %s failures in production', + async (failureKind, response, statusCode) => { + const log = vi.spyOn(logger, 'warn').mockImplementation(() => {}); + const result = await callSafetyApi({ + content: 'private-content', scene: SAFETY_SCENE.UserInput, - hasBearer: true, - durationMs: expect.any(Number), - upstreamTraceId: 'trace-content-safety-test', - }), - '[content-safety] upstream response', - ); - const serialized = JSON.stringify(log.mock.calls); - expect(serialized).not.toContain('must-not-appear-token'); - expect(serialized).not.toContain('must-not-appear-in-log'); - }); + authContext: { accessToken: 'private-token' }, + attachments: [{ fileUrl: 'https://secret.invalid/private-file', fileName: 'private-name' }], + fetchImpl: async () => response(), + region, + buildEnv: () => 'prod', + }); + expect(result.pass).toBe(false); + expect(reviewBlocks(result)).toBe(failureKind !== 'http'); + expect(log).toHaveBeenCalledOnce(); + expect(log).toHaveBeenCalledWith( + expect.objectContaining({ + reviewId: expect.any(String), + endpointHost: 'agent.minimax.cn', + apiVersion: 'v1', + scene: SAFETY_SCENE.UserInput, + durationMs: expect.any(Number), + failureKind, + ...(statusCode === undefined ? { transportKind: 'dns' } : { statusCode }), + }), + '[content-safety] review failed', + ); + const serialized = JSON.stringify(log.mock.calls); + expect(serialized).not.toContain('private-'); + expect(serialized).not.toContain('secret.invalid'); + expect(serialized).not.toContain('https://'); + }, + ); - it('does not emit the temporary upstream diagnostic in production', async () => { - const log = vi.spyOn(logger, 'info').mockImplementation(() => {}); + it('does not log successful reviews in production', async () => { + const log = vi.spyOn(logger, 'warn').mockImplementation(() => {}); await callSafetyApi({ content: 'hello', diff --git a/packages/local-runtime/test/unit/content-safety-api-v2.test.ts b/packages/local-runtime/test/unit/content-safety-api-v2.test.ts index 5e057dba5..cedba065a 100644 --- a/packages/local-runtime/test/unit/content-safety-api-v2.test.ts +++ b/packages/local-runtime/test/unit/content-safety-api-v2.test.ts @@ -1,3 +1,4 @@ +import { logger } from '../../src/common/logger.js'; import { afterEach, describe, expect, it, vi } from 'vitest'; import { SAFETY_CHECK_V2_SCENE } from '@mavis/shared/safety-check-v2'; import { callLocalSafetyCheckV2 } from '../../src/content-safety/api-v2.js'; @@ -14,7 +15,10 @@ const defaults = { }; const fetchResponse = (body: unknown) => vi.fn().mockImplementation(async () => Response.json(body)); -afterEach(() => vi.unstubAllEnvs()); +afterEach(() => { + vi.unstubAllEnvs(); + vi.restoreAllMocks(); +}); describe('local biz-gateway SafetyCheckV2 client', () => { it('uses the V2 endpoint and scene with the current Bearer/routing headers', async () => { @@ -248,3 +252,81 @@ describe('local biz-gateway SafetyCheckV2 client', () => { }); }); }); + +describe('V2 production failure diagnostics', () => { + it.each([ + [{ cause: { code: 'ENOTFOUND' } }, 'dns'], + [{ cause: { code: 'ECONNREFUSED' } }, 'connect'], + [{ cause: { code: 'ERR_TLS_CERT_ALTNAME_INVALID' } }, 'tls'], + [{ name: 'TimeoutError' }, 'timeout'], + [{ cause: { code: 'UND_ERR_BODY_TIMEOUT' } }, 'timeout'], + [{ name: 'AbortError' }, 'aborted'], + [{ cause: { code: 'private-code' } }, 'unknown'], + ] as const)('retains only the transport category %j', async (details, transportKind) => { + const log = vi.spyOn(logger, 'warn').mockImplementation(() => {}); + const error = Object.assign( + new Error('private-error https://secret.invalid/?token=private-token'), + details, + ); + await expect( + callLocalSafetyCheckV2({ + ...defaults, + buildEnv: () => 'prod', + authContext: { accessToken: 'private-token' }, + request: { scene: 100, content_text: 'private-content', sessionId: 'private-session' }, + fetchImpl: async () => { + throw error; + }, + }), + ).rejects.toMatchObject({ kind: 'transport', transportKind }); + expect(log).toHaveBeenCalledOnce(); + expect(log).toHaveBeenCalledWith( + expect.objectContaining({ + reviewId: expect.any(String), + endpointHost: 'agent.minimax.cn', + apiVersion: 'v2', + scene: 100, + durationMs: expect.any(Number), + failureKind: 'transport', + transportKind, + }), + '[content-safety] review failed', + ); + expect(JSON.stringify(log.mock.calls)).not.toContain('private-'); + expect(JSON.stringify(log.mock.calls)).not.toContain('secret.invalid'); + }); + + it.each([ + ['auth', () => new Response('private-body', { status: 401 })], + ['http', () => new Response('private-body', { status: 503 })], + ['response', () => new Response('private-body')], + ['response', () => Response.json({ unexpected: 'private-body' })], + ['upstream', () => Response.json({ errorCode: 50200, error: 'private-body' })], + [ + 'transport', + () => + new Response( + new ReadableStream({ + start(controller) { + controller.error(new DOMException('private-body', 'TimeoutError')); + }, + }), + ), + ], + ] as const)('logs failed %s reviews without response data', async (kind, response) => { + const log = vi.spyOn(logger, 'warn').mockImplementation(() => {}); + await expect( + callLocalSafetyCheckV2({ + ...defaults, + buildEnv: () => 'prod', + fetchImpl: async () => response(), + }), + ).rejects.toMatchObject({ kind }); + expect(log).toHaveBeenCalledOnce(); + expect(log).toHaveBeenCalledWith( + expect.objectContaining({ failureKind: kind }), + '[content-safety] review failed', + ); + expect(JSON.stringify(log.mock.calls)).not.toContain('private-body'); + }); +}); diff --git a/packages/local-runtime/test/unit/local-runtime-host-output-safety-concurrency.test.ts b/packages/local-runtime/test/unit/local-runtime-host-output-safety-concurrency.test.ts index b83a2e333..246ebcb58 100644 --- a/packages/local-runtime/test/unit/local-runtime-host-output-safety-concurrency.test.ts +++ b/packages/local-runtime/test/unit/local-runtime-host-output-safety-concurrency.test.ts @@ -1,3 +1,5 @@ +import { LocalAgentTurnRunner } from '../../../local-runtime-v2/src/service/turn-system/agent-host/runner/local-agent-turn-runner.js'; +import { logger } from '../../src/common/logger.js'; // Concurrency isolation for the output-safety host loop. // // The blocked flag / abort / review buffers live on per-attempt writer instances @@ -8,7 +10,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { LocalRuntimeHost, type LocalTurnRunner } from '../../src/runtime/host.js'; import { RespDataType } from '@mavis/agent-core/protocol/agent-message'; -import { RuntimeEventType, type IRuntimeEvent } from '@mavis/protocol'; +import { RuntimeEventStatus, RuntimeEventType, type IRuntimeEvent } from '@mavis/protocol'; function finalMessage(content: string): IRuntimeEvent { return { @@ -57,6 +59,91 @@ describe('LocalRuntimeHost output safety — concurrency isolation', () => { delete process.env.IDC; }); + it('records a review stop while retaining completed terminal semantics', async () => { + const log = vi.spyOn(logger, 'warn').mockImplementation(() => {}); + const fetchMock = vi + .spyOn(globalThis, 'fetch') + .mockRejectedValue(new DOMException('timeout', 'TimeoutError')); + const host = new LocalRuntimeHost({ + safetyApiVersion: 'v2', + outputSafetyRetryDelay: async () => {}, + piRunner: { + async runTurn(input) { + await input.eventWriter.pushRuntime(finalMessage('unreviewed-output')); + }, + }, + }); + const output = await host.runTurn({ + workspaceDir: '/tmp/ws', + systemPrompt: '', + userMessage: { text: 'hi' }, + llm: { model: {} as never }, + sessionId: 'ses', + turnId: 'turn', + }); + expect(fetchMock).toHaveBeenCalledTimes(4); + expect(output).toMatchObject({ networkStopped: true, retracted: false }); + expect(output.events.filter((event) => event.type === RuntimeEventType.STREAM_RESP)).toEqual( + [], + ); + expect(output.events).toContainEqual( + expect.objectContaining({ + type: RuntimeEventType.SESSION_STATUS, + payload: expect.objectContaining({ status: RuntimeEventStatus.COMPLETED }), + }), + ); + expect(log).toHaveBeenCalledWith( + { + sessionId: 'ses', + turnId: 'turn', + reviewOutcome: 'unavailable', + outputSuppressed: true, + terminalStatus: 'completed', + }, + '[content-safety] output review stopped turn', + ); + expect(JSON.stringify(log.mock.calls)).not.toContain('unreviewed-output'); + }); + + it.each([false, true])('diagnoses the V2 review stop without overriding runner failure: %s', async (runnerFailed) => { + const log = vi.spyOn(logger, 'warn').mockImplementation(() => {}); + const events: IRuntimeEvent[] = []; + const eventWriter = { + events, + async pushRuntime(event: IRuntimeEvent) { events.push(event); }, + async appendEvents(next: IRuntimeEvent[]) { events.push(...next); }, + }; + const reviewContent = vi.fn(async () => ({ pass: false, errorKind: 'local_error' as const })); + const runner = new LocalAgentTurnRunner({ + reviewContent, + outputSafetyRetryDelay: async () => {}, + piRunner: { async runTurn(input) { + await input.eventWriter.pushRuntime(finalMessage('unreviewed-output')); + if (runnerFailed) await input.eventWriter.pushRuntime({ + ...finalMessage(''), type: RuntimeEventType.SESSION_STATUS, + payload: { status: RuntimeEventStatus.FAILED }, + }); + } }, + }); + const output = await runner.runTurn({ + workspaceDir: '/tmp/ws', systemPrompt: '', userMessage: { text: 'hi' }, + llm: { model: {} as never }, sessionId: 'ses', turnId: 'turn', eventWriter, + toolContext: { agentName: 'test', parentAgentConfig: {}, trustedExactWritePaths: [], eventWriter } as never, + }); + expect(reviewContent).toHaveBeenCalledTimes(4); + expect(events.filter(event => event.type === RuntimeEventType.STREAM_RESP)).toEqual([]); + expect(output.outcome.status).toBe(runnerFailed ? 'failed' : 'completed'); + if (runnerFailed) { + expect(log).not.toHaveBeenCalled(); + } else { + expect(output).toMatchObject({ networkStopped: true, retracted: false, reconcile: { kind: 'network-reconcile' } }); + expect(log).toHaveBeenCalledWith({ + sessionId: 'ses', turnId: 'turn', reviewOutcome: 'unavailable', + outputSuppressed: true, terminalStatus: 'completed', + }, '[content-safety] output review stopped turn'); + } + }); + it('a rejected turn never contaminates a concurrent clean turn', async () => { const fetchMock = installContentAwareFetch(); // One shared runner + one shared host drive both turns. Each turn emits diff --git a/packages/shared/src/safety-check-v2.ts b/packages/shared/src/safety-check-v2.ts index d41f5bc13..40e42e381 100644 --- a/packages/shared/src/safety-check-v2.ts +++ b/packages/shared/src/safety-check-v2.ts @@ -36,11 +36,52 @@ export type SafetyCheckV2Headers = NonNullable; export type SafetyCheckV2ErrorKind = 'auth' | 'http' | 'transport' | 'response' | 'upstream'; +export type SafetyTransportKind = 'dns' | 'connect' | 'tls' | 'timeout' | 'aborted' | 'unknown'; + +/** Only allowlisted categories survive; never retain exception text, URLs or causes. */ +export function classifySafetyTransportError(error: unknown): SafetyTransportKind { + let current = error; + for (let depth = 0; depth < 4 && isSafetyCheckV2Record(current); depth += 1) { + const { name } = current; + const code = typeof current.code === 'string' ? current.code : ''; + if ( + name === 'TimeoutError' || + [ + 'ETIMEDOUT', + 'UND_ERR_CONNECT_TIMEOUT', + 'UND_ERR_HEADERS_TIMEOUT', + 'UND_ERR_BODY_TIMEOUT', + ].includes(code) + ) + return 'timeout'; + if (name === 'AbortError' || code === 'ABORT_ERR') return 'aborted'; + if (code === 'ENOTFOUND' || code === 'EAI_AGAIN') return 'dns'; + if ( + ['ECONNREFUSED', 'ECONNRESET', 'ENETUNREACH', 'EHOSTUNREACH', 'UND_ERR_SOCKET'].includes(code) + ) + return 'connect'; + if ( + [ + 'CERT_HAS_EXPIRED', + 'DEPTH_ZERO_SELF_SIGNED_CERT', + 'SELF_SIGNED_CERT_IN_CHAIN', + 'UNABLE_TO_VERIFY_LEAF_SIGNATURE', + 'ERR_TLS_CERT_ALTNAME_INVALID', + 'UNABLE_TO_GET_ISSUER_CERT_LOCALLY', + ].includes(code) + ) + return 'tls'; + current = current.cause; + } + return 'unknown'; +} + /** A failed check has no verdict. Callers must handle it separately from Allow. */ export class SafetyCheckV2Error extends Error { constructor( readonly kind: SafetyCheckV2ErrorKind, readonly statusCode?: number, + readonly transportKind?: SafetyTransportKind, ) { super(`SafetyCheckV2 ${kind} error`); this.name = 'SafetyCheckV2Error'; @@ -71,8 +112,8 @@ export async function postSafetyCheckV2(input: { signal: input.signal ? AbortSignal.any([input.signal, timeout]) : timeout, redirect: 'error', }); - } catch { - throw new SafetyCheckV2Error('transport'); + } catch (error) { + throw new SafetyCheckV2Error('transport', undefined, classifySafetyTransportError(error)); } if (!response.ok) { throw new SafetyCheckV2Error( @@ -83,8 +124,13 @@ export async function postSafetyCheckV2(input: { let body: unknown; try { body = await response.json(); - } catch { - throw new SafetyCheckV2Error('response'); + } catch (error) { + const transportKind = classifySafetyTransportError(error); + throw new SafetyCheckV2Error( + transportKind === 'unknown' ? 'response' : 'transport', + response.status, + transportKind, + ); } if (!isSafetyCheckV2Record(body)) throw new SafetyCheckV2Error('response'); return body; diff --git a/packages/tui/src/tui/controller/product/command-flow.ts b/packages/tui/src/tui/controller/product/command-flow.ts index 7a430e258..4bdb3af98 100644 --- a/packages/tui/src/tui/controller/product/command-flow.ts +++ b/packages/tui/src/tui/controller/product/command-flow.ts @@ -1218,7 +1218,7 @@ export class TuiCommandFlow { this.options.append( formatTuiActionFailure(error, { summary: "Couldn't reload TUI configuration.", - nextStep: 'Fix keybindings.json and retry /reload.', + nextStep: 'Resolve the reported configuration or plugin refresh error, then retry /reload.', }), 'warning', ); diff --git a/packages/tui/src/tui/controller/runtime/runtime-event-flow.ts b/packages/tui/src/tui/controller/runtime/runtime-event-flow.ts index 8ffd3bbac..5ce68b04b 100644 --- a/packages/tui/src/tui/controller/runtime/runtime-event-flow.ts +++ b/packages/tui/src/tui/controller/runtime/runtime-event-flow.ts @@ -458,7 +458,7 @@ export class TuiRuntimeEventFlow { event.variant === 'auth' ? 'Content review authentication expired. Run /login, then retry.' : event.variant === 'network' - ? 'The content review service could not be reached, so this response stopped. Check your network and proxy or VPN settings, then retry.' + ? 'Content review did not return a usable result, so this response stopped. Unreviewed output was withheld. Retry; if this persists, use /feedback to report the review failure.' : 'Content review withdrew this response. Rephrase your request, then retry.', 'warning', ); diff --git a/packages/tui/test/unit/tui-app.test.ts b/packages/tui/test/unit/tui-app.test.ts index 1d004f6fa..b646d5c11 100644 --- a/packages/tui/test/unit/tui-app.test.ts +++ b/packages/tui/test/unit/tui-app.test.ts @@ -10607,7 +10607,10 @@ describe("createTuiApp", () => { await app.stop(); }); - it("restores a content-review retraction to the Composer without a failed Queue item", async () => { + it.each([ + ['content', 'Content review withdrew this response. Rephrase your request, then retry.'], + ['network', 'Content review did not return a usable result, so this response stopped.'], + ] as const)("presents a %s review stop without a failed Queue item", async (variant, notice) => { const terminal = new FakeTerminal(); const runtime = createRuntime(); const busEvents: TuiRuntimeEvent[] = []; @@ -10655,7 +10658,7 @@ describe("createTuiApp", () => { type: "content.retry.exceeded", timestamp: 100, source: "runtime-v2", - payload: { sessionId: "session-1", variant: "content" }, + payload: { sessionId: "session-1", variant }, }), runtimeEvent({ type: "message.rewind", @@ -10671,9 +10674,8 @@ describe("createTuiApp", () => { expect(app.editor.getText()).toBe("review this response"), ); const rendered = app.tui.render(100).join("\n"); - expect(rendered).toContain( - "Content review withdrew this response. Rephrase your request, then retry.", - ); + expect(rendered).toContain(notice); + expect(rendered).not.toContain('Check your network'); expect(rendered).not.toContain("Message kept under Failed"); await app.submit("/queue"); expect(app.interaction.current()).toBeUndefined(); diff --git a/packages/tui/test/unit/tui/controller/product/command-flow.test.ts b/packages/tui/test/unit/tui/controller/product/command-flow.test.ts index c46265d89..d101bf8e1 100644 --- a/packages/tui/test/unit/tui/controller/product/command-flow.test.ts +++ b/packages/tui/test/unit/tui/controller/product/command-flow.test.ts @@ -37,6 +37,7 @@ function createReadinessCommandFlow(options: { }; persistTuiMode?: (mode: "regular" | "fullscreen") => void; reloadTui?: () => Promise; + append?: (text: string, kind?: "info" | "warning" | "error") => void; queuedCount?: number; }) { return new TuiCommandFlow({ @@ -86,7 +87,7 @@ function createReadinessCommandFlow(options: { ...(options.whenControllerReady ? { whenControllerReady: options.whenControllerReady } : {}), - append: vi.fn(), + append: options.append ?? vi.fn(), setHint: options.setHint ?? vi.fn(), onChanged: vi.fn(), }); @@ -347,6 +348,18 @@ describe("TuiCommandFlow", () => { expect(setHint).toHaveBeenLastCalledWith("TUI configuration reloaded."); }); + it.each(['Plugin refresh failed', 'Invalid keybindings.json'])("preserves /reload cause: %s", async (message) => { + const append = vi.fn(); + const flow = createReadinessCommandFlow({ + whenReady: async () => {}, append, + reloadTui: async () => { throw new Error(message); }, + }); + await expect(flow.submit('/reload')).resolves.toBe('consumed'); + expect(append).toHaveBeenCalledWith(expect.stringContaining(message), 'warning'); + expect(append).toHaveBeenCalledWith(expect.stringContaining('configuration or plugin refresh error'), 'warning'); + expect(JSON.stringify(append.mock.calls)).not.toContain('Fix keybindings.json'); + }); + it("retains /reload while queued work is present", async () => { const reloadTui = vi.fn(async () => undefined); const setHint = vi.fn(); diff --git a/release/public-source.json b/release/public-source.json index 369186f7e..4c4adeeff 100644 --- a/release/public-source.json +++ b/release/public-source.json @@ -2235,6 +2235,7 @@ "packages/local-runtime/src/content-safety/api-v2.ts", "packages/local-runtime/src/content-safety/api.ts", "packages/local-runtime/src/content-safety/config-fields.ts", + "packages/local-runtime/src/content-safety/diagnostics.ts", "packages/local-runtime/src/content-safety/turn-api.ts", "packages/local-runtime/src/context/context-usage-calibration.ts", "packages/local-runtime/src/context/context-usage-provider-diagnostics.ts",