From c8dc5fdb53355def926c9538b783fc1f8df94f25 Mon Sep 17 00:00:00 2001 From: Haitao Pan Date: Fri, 10 Apr 2026 11:22:59 +0800 Subject: [PATCH] fix: surface ACP failure diagnostics in bridge flows --- lib/runtime/go_task_service_client.dart | 22 ++++ test/runtime/bridge_real_e2e_suite.dart | 100 ++++++++++++++++-- test/runtime/go_task_service_client_test.dart | 41 +++++++ 3 files changed, 156 insertions(+), 7 deletions(-) diff --git a/lib/runtime/go_task_service_client.dart b/lib/runtime/go_task_service_client.dart index 6d8331c3..f3bfea3d 100644 --- a/lib/runtime/go_task_service_client.dart +++ b/lib/runtime/go_task_service_client.dart @@ -580,6 +580,26 @@ GoTaskServiceResult goTaskServiceResultFromAcpResponse( String? completedMessage, }) { final result = _castMap(response['result']); + final skillCandidates = _castMapList(result['skillCandidates']) + .map((item) => item['id']?.toString().trim() ?? '') + .where((item) => item.isNotEmpty) + .toList(growable: false); + final fallbackFailureText = () { + final success = _boolValue(result['success']) ?? true; + if (success) { + return ''; + } + final errorText = result['error']?.toString().trim() ?? ''; + final needsSkillInstall = _boolValue(result['needsSkillInstall']) ?? false; + if (needsSkillInstall && skillCandidates.isNotEmpty) { + final candidateText = skillCandidates.join(', '); + if (errorText.isNotEmpty) { + return '$errorText (skills: $candidateText)'; + } + return 'Skill install required: $candidateText'; + } + return errorText; + }(); final responseText = (result['output']?.toString().trim().isNotEmpty == true ? result['output'].toString().trim() @@ -592,6 +612,8 @@ GoTaskServiceResult goTaskServiceResultFromAcpResponse( ? completedMessage!.trim() : responseText.isNotEmpty ? responseText + : fallbackFailureText.isNotEmpty + ? fallbackFailureText : streamedText.trim().isNotEmpty ? streamedText.trim() : '') diff --git a/test/runtime/bridge_real_e2e_suite.dart b/test/runtime/bridge_real_e2e_suite.dart index 0ebc0fb6..e4a02a30 100644 --- a/test/runtime/bridge_real_e2e_suite.dart +++ b/test/runtime/bridge_real_e2e_suite.dart @@ -1,7 +1,6 @@ @TestOn('vm') library; -import 'dart:async'; import 'dart:io'; import 'package:flutter_test/flutter_test.dart'; @@ -13,14 +12,14 @@ import 'package:xworkmate/runtime/runtime_models.dart'; void main() { final config = _BridgeRealTestConfig.load(); final skipReason = config.skipReason; - final bridgeClient = config.bridgeClient; final artifactService = DesktopThreadArtifactService(); group('xworkmate-bridge real E2E', () { test( 'bridge contract keeps HTTP RPC reachable and advertises single-agent support', () async { - final capabilities = await bridgeClient.loadCapabilities( + await config.syncExternalProviders(); + final capabilities = await config.bridgeClient.loadCapabilities( forceRefresh: true, ); @@ -35,6 +34,7 @@ void main() { test( 'scenario ${scenario.key} binds thread workdir, supports follow-up, and records artifacts', () async { + await config.syncExternalProviders(); final root = await Directory.systemTemp.createTemp( 'xworkmate-bridge-${scenario.key}-', ); @@ -57,7 +57,7 @@ void main() { resumeSession: false, ); - final firstResponse = await bridgeClient.request( + final firstResponse = await config.bridgeClient.request( method: 'session.start', params: firstRequest.toExternalAcpParams(), ); @@ -66,6 +66,12 @@ void main() { route: firstRequest.route, ); + _expectSuccessfulBridgeResult( + firstResponse, + firstResult, + scenarioKey: scenario.key, + phase: 'start', + ); expect(firstResult.turnId, isNotEmpty); expect(firstResult.message, isNotEmpty); expect( @@ -85,7 +91,7 @@ void main() { prompt: scenario.followUpPrompt, resumeSession: true, ); - final resumeResponse = await bridgeClient.request( + final resumeResponse = await config.bridgeClient.request( method: 'session.message', params: resumeRequest.toExternalAcpParams(), ); @@ -94,6 +100,12 @@ void main() { route: resumeRequest.route, ); + _expectSuccessfulBridgeResult( + resumeResponse, + resumeResult, + scenarioKey: scenario.key, + phase: 'resume', + ); expect(resumeResult.turnId, isNotEmpty); expect(resumeResult.message, isNotEmpty); expect( @@ -237,10 +249,34 @@ class _BridgeRealTestConfig { const _BridgeRealTestConfig({ required this.skipReason, required this.bridgeClient, + required this.bridgeAuthToken, + required this.syncedProviders, }); final String? skipReason; final GatewayAcpClient bridgeClient; + final String bridgeAuthToken; + final List syncedProviders; + + Future syncExternalProviders() async { + await bridgeClient.request( + method: 'xworkmate.providers.sync', + params: { + 'providers': syncedProviders + .map( + (item) => { + 'providerId': item.providerId, + 'label': item.label, + 'endpoint': item.endpoint, + 'authorizationHeader': item.authorizationHeader, + 'enabled': item.enabled, + }, + ) + .toList(growable: false), + }, + authorizationOverride: 'Bearer $bridgeAuthToken', + ); + } static _BridgeRealTestConfig load() { final env = {..._loadEnvFile(), ...Platform.environment}; @@ -259,18 +295,68 @@ class _BridgeRealTestConfig { skipReason: 'Set BRIDGE_SERVER_URL and BRIDGE_AUTH_TOKEN (or ACP_AUTH_TOKEN) to run real bridge E2E tests.', bridgeClient: GatewayAcpClient(endpointResolver: () => null), + bridgeAuthToken: '', + syncedProviders: const [], ); } final endpoint = _normalizeEndpoint(rawUrl); + final normalizedToken = token.trim(); + final codexProviderEndpoint = + env['CODEX_PROVIDER_ENDPOINT'] ?? 'https://acp-server.svc.plus/codex'; final client = GatewayAcpClient( endpointResolver: () => endpoint, - authorizationResolver: (_) async => 'Bearer ${token.trim()}', + authorizationResolver: (_) async => 'Bearer $normalizedToken', + ); + return _BridgeRealTestConfig( + skipReason: null, + bridgeClient: client, + bridgeAuthToken: normalizedToken, + syncedProviders: [ + ExternalCodeAgentAcpSyncedProvider( + providerId: SingleAgentProvider.codex.providerId, + label: 'codex', + endpoint: codexProviderEndpoint, + authorizationHeader: 'Bearer $normalizedToken', + enabled: true, + ), + ], ); - return _BridgeRealTestConfig(skipReason: null, bridgeClient: client); } } +void _expectSuccessfulBridgeResult( + Map response, + GoTaskServiceResult result, { + required String scenarioKey, + required String phase, +}) { + final raw = Map.from(result.raw); + final success = result.success; + final errorText = raw['error']?.toString().trim() ?? ''; + final needsSkillInstall = raw['needsSkillInstall'] == true; + final provider = raw['provider']?.toString().trim() ?? ''; + final skillCandidates = + (raw['skillCandidates'] as List?) + ?.map( + (item) => item is Map ? item['id']?.toString().trim() ?? '' : '', + ) + .where((item) => item.isNotEmpty) + .cast() + .toList(growable: false) ?? + const []; + + expect( + success, + isTrue, + reason: + 'bridge $phase should succeed for $scenarioKey. ' + 'error="$errorText", needsSkillInstall=$needsSkillInstall, ' + 'provider="$provider", skillCandidates=$skillCandidates, ' + 'response=$response', + ); +} + Uri _normalizeEndpoint(String raw) { final trimmed = raw.trim(); if (trimmed.startsWith('https:') && !trimmed.startsWith('https://')) { diff --git a/test/runtime/go_task_service_client_test.dart b/test/runtime/go_task_service_client_test.dart index b91033cd..130e8f54 100644 --- a/test/runtime/go_task_service_client_test.dart +++ b/test/runtime/go_task_service_client_test.dart @@ -265,6 +265,47 @@ void main() { }, ); + test('run result falls back to error text when failed payload has no message', () { + final result = goTaskServiceResultFromAcpResponse( + { + 'result': { + 'success': false, + 'turnId': 'turn-error', + 'error': 'missing bearer authorization', + }, + }, + route: GoTaskServiceRoute.externalAcpSingle, + ); + + expect(result.success, isFalse); + expect(result.turnId, 'turn-error'); + expect(result.message, 'missing bearer authorization'); + }); + + test( + 'run result includes skill install diagnostics when failed payload requires install', + () { + final result = goTaskServiceResultFromAcpResponse( + { + 'result': { + 'success': false, + 'turnId': 'turn-skill-install', + 'error': 'missing bearer authorization', + 'needsSkillInstall': true, + 'skillCandidates': >[ + {'id': 'pptx', 'label': 'pptx'}, + ], + }, + }, + route: GoTaskServiceRoute.externalAcpSingle, + ); + + expect(result.success, isFalse); + expect(result.turnId, 'turn-skill-install'); + expect(result.message, 'missing bearer authorization (skills: pptx)'); + }, + ); + test('session update recognizes delta notifications', () { final update = goTaskServiceUpdateFromAcpNotification({ 'method': 'session.update',