From 8e07f87b700c70532f26bf93ff1d18ff4b45cddc Mon Sep 17 00:00:00 2001 From: Haitao Pan Date: Fri, 10 Apr 2026 15:37:50 +0800 Subject: [PATCH] fix: remove stale ACP gateway fallback routing --- ...ntroller_desktop_external_acp_routing.dart | 18 +-------- ...ler_desktop_runtime_coordination_impl.dart | 20 ++++++++++ ...pp_controller_desktop_runtime_helpers.dart | 18 +++++---- ...ontroller_desktop_workspace_execution.dart | 12 ++++++ .../runtime_models_settings_snapshot.dart | 15 +------ .../acp_bridge_provider_hub_suite.dart | 22 ++++++----- ...pp_controller_thread_skills_suite_acp.dart | 4 ++ ..._controller_thread_skills_suite_fakes.dart | 6 +++ ...ntroller_thread_skills_suite_fixtures.dart | 12 ++++++ test/runtime/gateway_acp_client_suite.dart | 39 +++++++++++++++++++ 10 files changed, 118 insertions(+), 48 deletions(-) diff --git a/lib/app/app_controller_desktop_external_acp_routing.dart b/lib/app/app_controller_desktop_external_acp_routing.dart index ff136d55..643f477d 100644 --- a/lib/app/app_controller_desktop_external_acp_routing.dart +++ b/lib/app/app_controller_desktop_external_acp_routing.dart @@ -50,27 +50,11 @@ extension AppControllerDesktopExternalAcpRouting on AppController { if (providerId.isEmpty || endpoint.isEmpty) { continue; } - var authorizationHeader = effectiveProfile.authRef.trim().isEmpty + final authorizationHeader = effectiveProfile.authRef.trim().isEmpty ? '' : await settingsControllerInternal.resolveSecretValueInternal( refName: effectiveProfile.authRef.trim(), ); - if (authorizationHeader.isEmpty && - builtinProvider != null && - settings.acpBridgeServerModeConfig.usesSelfHostedBase) { - final selfHosted = settings.acpBridgeServerModeConfig.selfHosted; - final username = selfHosted.username.trim(); - final passwordRef = selfHosted.passwordRef.trim(); - final password = passwordRef.isEmpty - ? '' - : await settingsControllerInternal.loadSecretValueByRef( - passwordRef, - ); - if (username.isNotEmpty && password.trim().isNotEmpty) { - authorizationHeader = - 'Basic ${base64Encode(utf8.encode('$username:${password.trim()}'))}'; - } - } providers.add( ExternalCodeAgentAcpSyncedProvider( providerId: providerId, diff --git a/lib/app/app_controller_desktop_runtime_coordination_impl.dart b/lib/app/app_controller_desktop_runtime_coordination_impl.dart index b4f51755..ce2c50a9 100644 --- a/lib/app/app_controller_desktop_runtime_coordination_impl.dart +++ b/lib/app/app_controller_desktop_runtime_coordination_impl.dart @@ -53,8 +53,28 @@ Future refreshAcpCapabilitiesRuntimeInternal( bool persistMountTargets = false, }) async { try { + 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, ); } catch (_) { // Keep mount refresh resilient when ACP is temporarily unavailable. diff --git a/lib/app/app_controller_desktop_runtime_helpers.dart b/lib/app/app_controller_desktop_runtime_helpers.dart index c55fe28f..2ed81a91 100644 --- a/lib/app/app_controller_desktop_runtime_helpers.dart +++ b/lib/app/app_controller_desktop_runtime_helpers.dart @@ -702,18 +702,22 @@ extension AppControllerDesktopRuntimeHelpers on AppController { return ''; } + Future resolveSingleAgentAuthorizationHeaderForProviderInternal( + SingleAgentProvider provider, + ) async { + final endpoint = resolveSingleAgentEndpointInternal(provider); + if (endpoint == null) { + return ''; + } + return resolveSingleAgentAuthorizationHeaderInternal(endpoint); + } + Uri? resolveGatewayAcpEndpointInternal() { final target = assistantExecutionTargetForSession( sessionsControllerInternal.currentSessionKey, ); if (target == AssistantExecutionTarget.singleAgent) { - final remote = gatewayProfileBaseUriInternal( - settings.primaryRemoteGatewayProfile, - ); - if (remote != null) { - return remote; - } - return gatewayProfileBaseUriInternal(settings.primaryLocalGatewayProfile); + return null; } return gatewayProfileBaseUriInternal( gatewayProfileForAssistantExecutionTargetInternal(target), diff --git a/lib/app/app_controller_desktop_workspace_execution.dart b/lib/app/app_controller_desktop_workspace_execution.dart index 5876ee27..681d9a9a 100644 --- a/lib/app/app_controller_desktop_workspace_execution.dart +++ b/lib/app/app_controller_desktop_workspace_execution.dart @@ -396,6 +396,16 @@ extension AppControllerDesktopWorkspaceExecution on AppController { ); return; } + final endpointOverride = resolveSingleAgentEndpointInternal(provider); + if (endpointOverride == null) { + await replaceSingleAgentThreadSkillsInternal( + normalizedSessionKey, + localSkills, + ); + return; + } + final authorizationOverride = + await resolveSingleAgentAuthorizationHeaderForProviderInternal(provider); await replaceSingleAgentThreadSkillsInternal( normalizedSessionKey, localSkills, @@ -410,6 +420,8 @@ extension AppControllerDesktopWorkspaceExecution on AppController { 'mode': 'single-agent', 'provider': provider.providerId, }, + endpointOverride: endpointOverride, + authorizationOverride: authorizationOverride, ); final result = asMap(response['result']); final payload = result.isNotEmpty ? result : response; diff --git a/lib/runtime/runtime_models_settings_snapshot.dart b/lib/runtime/runtime_models_settings_snapshot.dart index 701b87bd..731a9363 100644 --- a/lib/runtime/runtime_models_settings_snapshot.dart +++ b/lib/runtime/runtime_models_settings_snapshot.dart @@ -516,21 +516,8 @@ class SettingsSnapshot { ExternalAcpEndpointProfile externalAcpEndpointForProvider( SingleAgentProvider provider, ) { - final profile = - externalAcpEndpointForProviderId(provider.providerId) ?? + return externalAcpEndpointForProviderId(provider.providerId) ?? ExternalAcpEndpointProfile.defaultsForProvider(provider); - final bridgeBaseUrl = acpBridgeBuiltinEndpointBaseUrl; - if (provider.isAuto || bridgeBaseUrl.isEmpty) { - return profile; - } - return profile.copyWith(endpoint: bridgeBaseUrl); - } - - String get acpBridgeBuiltinEndpointBaseUrl { - if (!acpBridgeServerModeConfig.usesSelfHostedBase) { - return ''; - } - return acpBridgeServerModeConfig.selfHosted.serverUrl.trim(); } ExternalAcpEndpointProfile? externalAcpEndpointForProviderId( diff --git a/test/runtime/acp_bridge_provider_hub_suite.dart b/test/runtime/acp_bridge_provider_hub_suite.dart index 0e6d6293..5b8942e8 100644 --- a/test/runtime/acp_bridge_provider_hub_suite.dart +++ b/test/runtime/acp_bridge_provider_hub_suite.dart @@ -1,8 +1,6 @@ @TestOn('vm') library; -import 'dart:convert'; - import 'package:flutter_test/flutter_test.dart'; import 'package:shared_preferences/shared_preferences.dart'; import 'package:xworkmate/app/app_controller.dart'; @@ -16,7 +14,7 @@ import 'app_controller_ai_gateway_chat_suite_fakes.dart'; void main() { group('ACP bridge provider hub', () { test( - 'self-hosted ACP bridge base makes builtin single-agent providers visible without per-provider endpoints', + 'self-hosted ACP bridge base does not override builtin single-agent endpoints', () { final snapshot = SettingsSnapshot.defaults().copyWith( acpBridgeServerModeConfig: AcpBridgeServerModeConfig.defaults() @@ -33,13 +31,13 @@ void main() { snapshot .externalAcpEndpointForProvider(SingleAgentProvider.codex) .endpoint, - 'https://bridge.example.com', + '', ); }, ); test( - 'builtin provider sync uses bridge base endpoint and self-hosted basic auth when endpoint auth is empty', + 'builtin provider sync does not inject self-hosted bridge endpoint or auth fallback', () async { SharedPreferences.setMockInitialValues({}); final store = createIsolatedTestStore(enableSecureStorage: false); @@ -69,6 +67,13 @@ void main() { username: 'review@example.com', ), ), + externalAcpEndpoints: replaceExternalAcpEndpointForProvider( + controller.settings.externalAcpEndpoints, + SingleAgentProvider.opencode, + controller.settings + .externalAcpEndpointForProvider(SingleAgentProvider.opencode) + .copyWith(endpoint: 'https://acp.example.com/opencode'), + ), ), refreshAfterSave: false, ); @@ -79,11 +84,8 @@ void main() { (item) => item.providerId == 'opencode', ); - expect(opencode.endpoint, 'https://bridge.example.com'); - expect( - opencode.authorizationHeader, - 'Basic ${base64Encode(utf8.encode('review@example.com:top-secret'))}', - ); + expect(opencode.endpoint, 'https://acp.example.com/opencode'); + expect(opencode.authorizationHeader, ''); }, ); diff --git a/test/runtime/app_controller_thread_skills_suite_acp.dart b/test/runtime/app_controller_thread_skills_suite_acp.dart index 92c22a76..445e9b8e 100644 --- a/test/runtime/app_controller_thread_skills_suite_acp.dart +++ b/test/runtime/app_controller_thread_skills_suite_acp.dart @@ -85,6 +85,8 @@ void registerThreadSkillsAcpTests() { singleAgentTestSettingsInternal( workspacePath: tempDirectory.path, gatewayPort: acpServer.port, + singleAgentProviderEndpoint: + 'http://127.0.0.1:${acpServer.port}/opencode', ), ); await store.saveTaskThreads([ @@ -218,6 +220,8 @@ void registerThreadSkillsAcpTests() { singleAgentTestSettingsInternal( workspacePath: tempDirectory.path, gatewayPort: acpServer.port, + singleAgentProviderEndpoint: + 'http://127.0.0.1:${acpServer.port}/opencode', ), ); diff --git a/test/runtime/app_controller_thread_skills_suite_fakes.dart b/test/runtime/app_controller_thread_skills_suite_fakes.dart index 02b456a2..cfca8384 100644 --- a/test/runtime/app_controller_thread_skills_suite_fakes.dart +++ b/test/runtime/app_controller_thread_skills_suite_fakes.dart @@ -66,6 +66,8 @@ class AcpSkillsStatusServerInternal { final HttpServer serverInternal; List> skills; Map? skillsError; + String? lastAuthorizationHeader; + String? lastRequestPath; int get port => serverInternal.port; @@ -97,6 +99,10 @@ class AcpSkillsStatusServerInternal { } Future handleRpcInternal(HttpRequest request) async { + lastRequestPath = request.uri.path; + lastAuthorizationHeader = request.headers.value( + HttpHeaders.authorizationHeader, + ); final body = await utf8.decodeStream(request); final envelope = jsonDecode(body) as Map; final id = envelope['id']; diff --git a/test/runtime/app_controller_thread_skills_suite_fixtures.dart b/test/runtime/app_controller_thread_skills_suite_fixtures.dart index d432cddc..d298674f 100644 --- a/test/runtime/app_controller_thread_skills_suite_fixtures.dart +++ b/test/runtime/app_controller_thread_skills_suite_fixtures.dart @@ -56,6 +56,8 @@ Future createStoreInternal(String rootPath) async { SettingsSnapshot singleAgentTestSettingsInternal({ required String workspacePath, int gatewayPort = 9, + String singleAgentProviderEndpoint = '', + String singleAgentProviderAuthRef = '', }) { final defaults = SettingsSnapshot.defaults(); return defaults.copyWith( @@ -78,5 +80,15 @@ SettingsSnapshot singleAgentTestSettingsInternal({ ), assistantExecutionTarget: AssistantExecutionTarget.singleAgent, workspacePath: workspacePath, + externalAcpEndpoints: replaceExternalAcpEndpointForProvider( + defaults.externalAcpEndpoints, + SingleAgentProvider.opencode, + defaults.externalAcpEndpointForProvider( + SingleAgentProvider.opencode, + ).copyWith( + endpoint: singleAgentProviderEndpoint, + authRef: singleAgentProviderAuthRef, + ), + ), ); } diff --git a/test/runtime/gateway_acp_client_suite.dart b/test/runtime/gateway_acp_client_suite.dart index 7b1f3a25..93a62987 100644 --- a/test/runtime/gateway_acp_client_suite.dart +++ b/test/runtime/gateway_acp_client_suite.dart @@ -187,6 +187,28 @@ void main() { }, ); + test( + 'routes generic ACP requests through the explicit hosted provider endpoint without fallback', + () async { + final server = await _AcpFakeServer.start(pathPrefix: '/gemini'); + addTearDown(server.close); + + final client = GatewayAcpClient( + endpointResolver: () => Uri.parse('http://127.0.0.1:9'), + ); + + await client.request( + method: 'skills.status', + params: const {'provider': 'gemini'}, + endpointOverride: server.baseHttpUri, + authorizationOverride: 'Bearer provider-secret', + ); + + expect(server.lastHttpRequestPath, '/gemini/acp/rpc'); + expect(server.lastHttpAuthorization, 'Bearer provider-secret'); + }, + ); + test('preserves hosted ACP base path for websocket requests', () async { final server = await _AcpFakeServer.start(pathPrefix: '/opencode'); addTearDown(server.close); @@ -498,6 +520,23 @@ class _AcpFakeServer { ), ); return; + case 'skills.status': + await respond( + _resultEnvelope( + id: id, + result: const { + 'skills': >[ + { + 'skillKey': 'gemini-remote', + 'name': 'Gemini Remote', + 'description': 'Hosted ACP skill payload', + 'source': 'acp', + }, + ], + }, + ), + ); + return; case 'session.cancel': await respond( _resultEnvelope(