diff --git a/lib/app/app_controller_desktop_core.dart b/lib/app/app_controller_desktop_core.dart index 0af2433b..5fee0c47 100644 --- a/lib/app/app_controller_desktop_core.dart +++ b/lib/app/app_controller_desktop_core.dart @@ -201,7 +201,6 @@ class AppController extends ChangeNotifier { singleAgentSharedSkillScanRootOverrides?.toList(growable: false); gatewayAcpClientInternal = GatewayAcpClient( endpointResolver: resolveGatewayAcpEndpointInternal, - authorizationResolver: resolveSingleAgentAuthorizationHeaderInternal, ); availableSingleAgentProvidersOverrideInternal = availableSingleAgentProvidersOverride; diff --git a/lib/app/app_controller_desktop_runtime_coordination_impl.dart b/lib/app/app_controller_desktop_runtime_coordination_impl.dart index be9c1268..5fd39d06 100644 --- a/lib/app/app_controller_desktop_runtime_coordination_impl.dart +++ b/lib/app/app_controller_desktop_runtime_coordination_impl.dart @@ -56,26 +56,17 @@ Future refreshAcpCapabilitiesRuntimeInternal( final target = controller.assistantExecutionTargetForSession( controller.sessionsControllerInternal.currentSessionKey, ); - final resolvedProvider = target == AssistantExecutionTarget.singleAgent - ? (controller.singleAgentResolvedProviderForSession( - controller.sessionsControllerInternal.currentSessionKey, - ) ?? - controller.currentSingleAgentResolvedProvider) - : null; - final endpointOverride = resolvedProvider == null - ? null - : controller.resolveSingleAgentEndpointInternal(resolvedProvider); - final authorizationOverride = resolvedProvider == null - ? '' - : await controller - .resolveSingleAgentAuthorizationHeaderForProviderInternal( - resolvedProvider, - ); - await controller.gatewayAcpClientInternal.loadCapabilities( - forceRefresh: forceRefresh, - endpointOverride: endpointOverride, - authorizationOverride: authorizationOverride, - ); + if (target == AssistantExecutionTarget.singleAgent) { + await controller.syncExternalAcpProvidersInternal(); + await controller.goTaskServiceClientInternal.loadExternalAcpCapabilities( + target: AssistantExecutionTarget.singleAgent, + forceRefresh: forceRefresh, + ); + } else { + await controller.gatewayAcpClientInternal.loadCapabilities( + forceRefresh: forceRefresh, + ); + } } catch (_) { // Keep mount refresh resilient when ACP is temporarily unavailable. } @@ -233,10 +224,17 @@ bool singleAgentProviderRequiresLocalPathRuntimeInternal( AppController controller, SingleAgentProvider provider, ) { - final endpoint = resolveSingleAgentEndpointRuntimeInternal( - controller, - provider, - ); + final configuredEndpoint = controller.settings + .externalAcpEndpointForProvider(provider) + .endpoint + .trim(); + if (configuredEndpoint.isEmpty) { + return true; + } + final normalizedInput = configuredEndpoint.contains('://') + ? configuredEndpoint + : 'ws://$configuredEndpoint'; + final endpoint = Uri.tryParse(normalizedInput); if (endpoint == null) { return true; } @@ -378,31 +376,3 @@ void recomputeTasksRuntimeInternal(AppController controller) { activeAgentName: controller.agentsControllerInternal.activeAgentName, ); } - -Uri? resolveSingleAgentEndpointRuntimeInternal( - AppController controller, - SingleAgentProvider provider, -) { - final endpoint = controller.settings - .externalAcpEndpointForProvider(provider) - .endpoint - .trim(); - if (endpoint.isEmpty) { - return null; - } - final normalizedInput = endpoint.contains('://') - ? endpoint - : 'ws://$endpoint'; - final uri = Uri.tryParse(normalizedInput); - if (uri == null || uri.host.trim().isEmpty) { - return null; - } - final scheme = uri.scheme.trim().toLowerCase(); - if (scheme != 'ws' && - scheme != 'wss' && - scheme != 'http' && - scheme != 'https') { - return null; - } - return uri; -} diff --git a/lib/app/app_controller_desktop_runtime_helpers.dart b/lib/app/app_controller_desktop_runtime_helpers.dart index b9f27ded..e59253d1 100644 --- a/lib/app/app_controller_desktop_runtime_helpers.dart +++ b/lib/app/app_controller_desktop_runtime_helpers.dart @@ -651,68 +651,6 @@ extension AppControllerDesktopRuntimeHelpers on AppController { notifyListeners(); } - Uri? resolveSingleAgentEndpointInternal(SingleAgentProvider provider) { - final endpoint = settings - .externalAcpEndpointForProvider(provider) - .endpoint - .trim(); - if (endpoint.isEmpty) { - return null; - } - final normalizedInput = endpoint.contains('://') - ? endpoint - : 'ws://$endpoint'; - final uri = Uri.tryParse(normalizedInput); - if (uri == null || uri.host.trim().isEmpty) { - return null; - } - final scheme = uri.scheme.trim().toLowerCase(); - if (scheme != 'ws' && - scheme != 'wss' && - scheme != 'http' && - scheme != 'https') { - return null; - } - return uri; - } - - Future resolveSingleAgentAuthorizationHeaderInternal( - Uri endpoint, - ) async { - final normalizedEndpoint = _normalizeExternalAcpEndpointInternal( - endpoint.toString(), - ); - if (normalizedEndpoint == null) { - return ''; - } - for (final profile in settings.externalAcpEndpoints) { - final profileEndpoint = _normalizeExternalAcpEndpointInternal( - profile.endpoint, - ); - if (profileEndpoint == null || profileEndpoint != normalizedEndpoint) { - continue; - } - final authRef = profile.authRef.trim(); - if (authRef.isEmpty) { - return ''; - } - return settingsControllerInternal.resolveSecretValueInternal( - refName: authRef, - ); - } - return ''; - } - - Future resolveSingleAgentAuthorizationHeaderForProviderInternal( - SingleAgentProvider provider, - ) async { - final endpoint = resolveSingleAgentEndpointInternal(provider); - if (endpoint == null) { - return ''; - } - return resolveSingleAgentAuthorizationHeaderInternal(endpoint); - } - Future> buildExternalAcpSyncedProvidersInternal() async { final providers = []; @@ -859,32 +797,6 @@ extension AppControllerDesktopRuntimeHelpers on AppController { return trimmed == '127.0.0.1' || trimmed == 'localhost'; } - String? _normalizeExternalAcpEndpointInternal(String raw) { - final trimmed = raw.trim(); - if (trimmed.isEmpty) { - return null; - } - final candidate = trimmed.contains('://') ? trimmed : 'ws://$trimmed'; - final uri = Uri.tryParse(candidate); - if (uri == null || uri.host.trim().isEmpty) { - return null; - } - final scheme = uri.scheme.trim().toLowerCase(); - if (scheme != 'ws' && - scheme != 'wss' && - scheme != 'http' && - scheme != 'https') { - return null; - } - final defaultPort = switch (scheme) { - 'https' || 'wss' => 443, - _ => 80, - }; - final port = uri.hasPort ? uri.port : defaultPort; - final path = uri.path.trim().isEmpty ? '/' : uri.path.trim(); - return '$scheme://${uri.host.toLowerCase()}:$port$path'; - } - AssistantExecutionTarget assistantExecutionTargetForModeInternal( RuntimeConnectionMode mode, ) { diff --git a/lib/app/app_controller_desktop_skill_permissions.dart b/lib/app/app_controller_desktop_skill_permissions.dart index 07cc15e3..9672ff15 100644 --- a/lib/app/app_controller_desktop_skill_permissions.dart +++ b/lib/app/app_controller_desktop_skill_permissions.dart @@ -216,40 +216,6 @@ extension AppControllerDesktopSkillPermissions on AppController { notifyIfActiveInternal(); } - AssistantThreadSkillEntry singleAgentSkillEntryFromAcpInternal( - Map item, - SingleAgentProvider provider, - ) { - return AssistantThreadSkillEntry( - key: item['skillKey']?.toString().trim().isNotEmpty == true - ? item['skillKey'].toString().trim() - : (item['name']?.toString().trim() ?? ''), - label: item['name']?.toString().trim() ?? '', - description: item['description']?.toString().trim() ?? '', - source: item['source']?.toString().trim() ?? provider.providerId, - sourcePath: item['path']?.toString().trim() ?? '', - scope: item['scope']?.toString().trim().isNotEmpty == true - ? item['scope'].toString().trim() - : 'session', - sourceLabel: item['sourceLabel']?.toString().trim().isNotEmpty == true - ? item['sourceLabel'].toString().trim() - : (item['source']?.toString().trim().isNotEmpty == true - ? item['source'].toString().trim() - : provider.label), - ); - } - - bool unsupportedAcpSkillsStatusInternal(GatewayAcpException error) { - final code = (error.code ?? '').trim(); - if (code == '-32601' || code == 'METHOD_NOT_FOUND') { - return true; - } - final message = error.toString().toLowerCase(); - return message.contains('unknown method') || - message.contains('method not found') || - message.contains('skills.status'); - } - void upsertTaskThreadInternal( String sessionKey, { ThreadOwnerScope? ownerScope, diff --git a/lib/app/app_controller_desktop_workspace_execution.dart b/lib/app/app_controller_desktop_workspace_execution.dart index 67610488..ea0563c5 100644 --- a/lib/app/app_controller_desktop_workspace_execution.dart +++ b/lib/app/app_controller_desktop_workspace_execution.dart @@ -388,76 +388,10 @@ extension AppControllerDesktopWorkspaceExecution on AppController { final localSkills = await singleAgentLocalSkillsForSessionInternal( normalizedSessionKey, ); - final provider = - singleAgentResolvedProviderForSession(normalizedSessionKey) ?? - currentSingleAgentResolvedProvider; - if (provider == null) { - await replaceSingleAgentThreadSkillsInternal( - normalizedSessionKey, - localSkills, - ); - return; - } - final endpointOverride = resolveSingleAgentEndpointInternal(provider); - if (endpointOverride == null) { - await replaceSingleAgentThreadSkillsInternal( - normalizedSessionKey, - localSkills, - ); - return; - } - final authorizationOverride = - await resolveSingleAgentAuthorizationHeaderForProviderInternal( - provider, - ); await replaceSingleAgentThreadSkillsInternal( normalizedSessionKey, localSkills, ); - try { - await refreshAcpCapabilitiesInternal(); - final response = await gatewayAcpClientInternal.request( - method: 'skills.status', - params: { - 'sessionId': normalizedSessionKey, - 'threadId': normalizedSessionKey, - 'mode': 'single-agent', - 'provider': provider.providerId, - }, - endpointOverride: endpointOverride, - authorizationOverride: authorizationOverride, - ); - final result = asMap(response['result']); - final payload = result.isNotEmpty ? result : response; - final skills = asList(payload['skills']) - .map(asMap) - .map((item) => singleAgentSkillEntryFromAcpInternal(item, provider)) - .where((item) => item.key.isNotEmpty && item.label.isNotEmpty) - .toList(growable: false); - await replaceSingleAgentThreadSkillsInternal( - normalizedSessionKey, - mergeSingleAgentSkillEntriesInternal( - groups: >[localSkills, skills], - ), - ); - } on GatewayAcpException catch (error) { - if (unsupportedAcpSkillsStatusInternal(error)) { - await replaceSingleAgentThreadSkillsInternal( - normalizedSessionKey, - localSkills, - ); - return; - } - await replaceSingleAgentThreadSkillsInternal( - normalizedSessionKey, - localSkills, - ); - } catch (_) { - await replaceSingleAgentThreadSkillsInternal( - normalizedSessionKey, - localSkills, - ); - } } Future refreshSingleAgentLocalSkillsForSession( diff --git a/test/app_controller_desktop_runtime_cleanup_test.dart b/test/app_controller_desktop_runtime_cleanup_test.dart new file mode 100644 index 00000000..87d1ad11 --- /dev/null +++ b/test/app_controller_desktop_runtime_cleanup_test.dart @@ -0,0 +1,290 @@ +import 'dart:convert'; +import 'dart:io'; + +import 'package:flutter_test/flutter_test.dart'; +import 'package:xworkmate/app/app_controller_desktop_core.dart'; +import 'package:xworkmate/app/app_controller_desktop_skill_permissions.dart'; +import 'package:xworkmate/app/app_controller_desktop_thread_sessions.dart'; +import 'package:xworkmate/app/app_controller_desktop_workspace_execution.dart'; +import 'package:xworkmate/runtime/desktop_platform_service.dart'; +import 'package:xworkmate/runtime/go_task_service_client.dart'; +import 'package:xworkmate/runtime/runtime_models.dart'; +import 'package:xworkmate/runtime/secure_config_store.dart'; +import 'package:xworkmate/runtime/skill_directory_access.dart'; + +void main() { + TestWidgetsFlutterBinding.ensureInitialized(); + + test('app-side runtime cleanup removes direct provider ACP side-channels', () { + final workspaceExecution = File( + 'lib/app/app_controller_desktop_workspace_execution.dart', + ).readAsStringSync(); + expect( + workspaceExecution.contains("'skills.status'"), + isFalse, + reason: + 'single-agent skill refresh should not query provider ACP skills.status directly', + ); + expect( + workspaceExecution.contains('gatewayAcpClientInternal.request('), + isFalse, + reason: 'workspace execution should not issue direct provider ACP RPCs', + ); + + final runtimeCoordination = File( + 'lib/app/app_controller_desktop_runtime_coordination_impl.dart', + ); + if (runtimeCoordination.existsSync()) { + final source = runtimeCoordination.readAsStringSync(); + expect( + source.contains('resolveSingleAgentEndpointRuntimeInternal'), + isFalse, + reason: + 'single-agent endpoint probing should not remain in app-side runtime coordination', + ); + expect( + source.contains('authorizationOverride'), + isFalse, + reason: + 'app-side runtime coordination should not own provider auth side-channels', + ); + } + }); + + test( + 'single-agent skill refresh stays bridge-owned and does not query provider endpoints directly', + () async { + final root = Directory.systemTemp.createTempSync( + 'xworkmate-runtime-cleanup-test-', + ); + final server = await HttpServer.bind(InternetAddress.loopbackIPv4, 0); + var requestCount = 0; + server.listen((request) async { + requestCount += 1; + final body = await utf8.decoder.bind(request).join(); + final payload = jsonDecode(body) as Map; + final method = payload['method']?.toString().trim() ?? ''; + final response = switch (method) { + 'acp.capabilities' => { + 'jsonrpc': '2.0', + 'id': payload['id'], + 'result': { + 'singleAgent': true, + 'multiAgent': false, + 'providerCatalog': >[ + {'providerId': 'codex', 'label': 'Codex'}, + ], + }, + }, + 'skills.status' => { + 'jsonrpc': '2.0', + 'id': payload['id'], + 'result': { + 'skills': >[ + { + 'skillKey': 'remote-skill', + 'name': 'Remote Skill', + 'description': 'stale remote side-channel', + }, + ], + }, + }, + _ => { + 'jsonrpc': '2.0', + 'id': payload['id'], + 'result': {}, + }, + }; + request.response.headers.contentType = ContentType.json; + request.response.write(jsonEncode(response)); + await request.response.close(); + }); + + final store = SecureConfigStore( + enableSecureStorage: false, + databasePathResolver: () async => '${root.path}/settings.sqlite3', + fallbackDirectoryPathResolver: () async => root.path, + defaultSupportDirectoryPathResolver: () async => root.path, + ); + final controller = AppController( + store: store, + desktopPlatformService: UnsupportedDesktopPlatformService(), + skillDirectoryAccessService: _FakeSkillDirectoryAccessService( + root.path, + ), + goTaskServiceClient: const _FakeGoTaskServiceClient(), + singleAgentSharedSkillScanRootOverrides: const [], + availableSingleAgentProvidersOverride: const [ + SingleAgentProvider.codex, + ], + ); + addTearDown(() async { + controller.dispose(); + await server.close(force: true); + if (root.existsSync()) { + await root.delete(recursive: true); + } + }); + + final endpoint = 'http://${server.address.address}:${server.port}'; + final nextSettings = controller.settings.copyWith( + externalAcpEndpoints: [ + ExternalAcpEndpointProfile.defaultsForProvider( + SingleAgentProvider.codex, + ).copyWith(endpoint: endpoint), + ], + ); + controller.settingsController.snapshotInternal = nextSettings; + controller.lastObservedSettingsSnapshotInternal = nextSettings; + + const sessionKey = 'draft:runtime-cleanup'; + controller.initializeAssistantThreadContext( + sessionKey, + executionTarget: AssistantExecutionTarget.singleAgent, + singleAgentProvider: SingleAgentProvider.codex, + ); + controller.upsertTaskThreadInternal( + sessionKey, + executionTarget: AssistantExecutionTarget.singleAgent, + singleAgentProvider: SingleAgentProvider.codex, + executionTargetSource: ThreadSelectionSource.explicit, + singleAgentProviderSource: ThreadSelectionSource.explicit, + ); + + expect( + controller.assistantExecutionTargetForSession(sessionKey), + AssistantExecutionTarget.singleAgent, + ); + expect( + controller.singleAgentProviderForSession(sessionKey), + SingleAgentProvider.codex, + ); + expect( + controller.singleAgentResolvedProviderForSession(sessionKey), + SingleAgentProvider.codex, + ); + + await controller.refreshSingleAgentSkillsForSession(sessionKey); + + expect(controller.assistantImportedSkillsForSession(sessionKey), isEmpty); + expect( + requestCount, + 0, + reason: + 'single-agent skill refresh should not probe provider ACP endpoints directly', + ); + }, + ); +} + +class _FakeSkillDirectoryAccessService implements SkillDirectoryAccessService { + const _FakeSkillDirectoryAccessService(this.homeDirectory); + + final String homeDirectory; + + @override + bool get isSupported => false; + + @override + Future> authorizeDirectories({ + List suggestedPaths = const [], + }) async { + return const []; + } + + @override + Future authorizeDirectory({ + String suggestedPath = '', + }) async { + return null; + } + + @override + Future openDirectory( + AuthorizedSkillDirectory directory, + ) async { + return null; + } + + @override + Future resolveUserHomeDirectory() async { + return homeDirectory; + } +} + +class _FakeGoTaskServiceClient implements GoTaskServiceClient { + const _FakeGoTaskServiceClient(); + + @override + Future cancelTask({ + required GoTaskServiceRoute route, + required AssistantExecutionTarget target, + required String sessionId, + required String threadId, + }) async {} + + @override + Future closeTask({ + required GoTaskServiceRoute route, + required AssistantExecutionTarget target, + required String sessionId, + required String threadId, + }) async {} + + @override + Future dispose() async {} + + @override + Future executeTask( + GoTaskServiceRequest request, { + required void Function(GoTaskServiceUpdate update) onUpdate, + }) async { + return const GoTaskServiceResult( + success: true, + message: '', + turnId: '', + raw: {}, + errorMessage: '', + resolvedModel: '', + route: GoTaskServiceRoute.externalAcpSingle, + ); + } + + @override + Future loadExternalAcpCapabilities({ + required AssistantExecutionTarget target, + bool forceRefresh = false, + }) async { + return const ExternalCodeAgentAcpCapabilities( + singleAgent: true, + multiAgent: false, + providerCatalog: [SingleAgentProvider.codex], + raw: {}, + ); + } + + @override + Future resolveExternalAcpRouting({ + required String taskPrompt, + required String workingDirectory, + required ExternalCodeAgentAcpRoutingConfig routing, + String aiGatewayBaseUrl = '', + String aiGatewayApiKey = '', + }) async { + return const ExternalCodeAgentAcpRoutingResolution( + raw: { + 'resolvedExecutionTarget': 'single-agent', + 'resolvedEndpointTarget': 'singleAgent', + 'resolvedProviderId': 'codex', + 'resolvedModel': '', + 'resolvedSkills': [], + 'unavailable': false, + }, + ); + } + + @override + Future syncExternalProviders( + List providers, + ) async {} +}