From 39a11de2f1ec43a74d082301e54eab330b4fa485 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 11 Jun 2026 11:11:38 +0000 Subject: [PATCH] fix(action): restore page apiMap after executeAction completes Reusable actions merge scoped API definitions into the shared page apiMap via createChildScope, but never removed them when the action finished. That let action-internal APIs remain callable from the rest of the screen and could permanently override page-level APIs with the same name. Snapshot affected keys before prepareScope and restore them in a finally block so scoped APIs are only visible while the action runs. Co-authored-by: Sharjeel Yunus --- .../lib/action/action_scope_util.dart | 41 ++++++++ .../ensemble/lib/action/execute_action.dart | 52 ++++++---- .../test/action_scope_api_restore_test.dart | 99 +++++++++++++++++++ 3 files changed, 171 insertions(+), 21 deletions(-) create mode 100644 modules/ensemble/test/action_scope_api_restore_test.dart diff --git a/modules/ensemble/lib/action/action_scope_util.dart b/modules/ensemble/lib/action/action_scope_util.dart index 4f81edc72..d49323beb 100644 --- a/modules/ensemble/lib/action/action_scope_util.dart +++ b/modules/ensemble/lib/action/action_scope_util.dart @@ -1,4 +1,5 @@ import 'package:ensemble/ensemble.dart'; +import 'package:flutter/foundation.dart'; import 'package:ensemble/framework/action.dart'; import 'package:ensemble/framework/data_context.dart'; import 'package:ensemble/framework/definition_providers/provider.dart'; @@ -92,6 +93,46 @@ class ActionScopeUtil { return normalizeActionDefinition(yaml) ?? YamlMap(); } + /// Save page-level API entries that a reusable action will override via + /// [createChildScope]. Call before [prepareScope] and pair with + /// [restorePageApisAfterAction] in a `finally` block. + @visibleForTesting + static Map? snapshotPageApisForAction( + ScopeManager parentScope, Map? actionApiMap) { + if (actionApiMap == null || actionApiMap.isEmpty) { + return null; + } + + final pageApiMap = parentScope.pageData.apiMap ??= {}; + final snapshot = {}; + for (final key in actionApiMap.keys) { + snapshot[key] = pageApiMap.containsKey(key) ? pageApiMap[key] : null; + } + return snapshot; + } + + /// Undo the temporary [mergedApiMap] merge performed by [prepareScope]. + @visibleForTesting + static void restorePageApisAfterAction( + ScopeManager parentScope, Map? snapshot) { + if (snapshot == null || snapshot.isEmpty) { + return; + } + + final pageApiMap = parentScope.pageData.apiMap; + if (pageApiMap == null) { + return; + } + + snapshot.forEach((key, original) { + if (original == null) { + pageApiMap.remove(key); + } else { + pageApiMap[key] = original; + } + }); + } + static Map? parseApiMap(YamlMap definition) { final dynamic apiNode = definition['API']; if (apiNode is! YamlMap) { diff --git a/modules/ensemble/lib/action/execute_action.dart b/modules/ensemble/lib/action/execute_action.dart index 2cbe969b0..77983cb89 100644 --- a/modules/ensemble/lib/action/execute_action.dart +++ b/modules/ensemble/lib/action/execute_action.dart @@ -71,29 +71,39 @@ class ExecuteActionAction extends EnsembleAction { } } - // Build a child scope with optional Import, API, Global, and input parameters - final ScopeManager childScope = ActionScopeUtil.prepareScope( - parentScope: scopeManager, - definition: definition, - parameters: parameters, - callInputs: rawInputs ?? const {}, - eventHandlers: eventHandlers, - ); + final Map? actionApiMap = + ActionScopeUtil.parseApiMap(definition); + final Map? apiSnapshot = + ActionScopeUtil.snapshotPageApisForAction(scopeManager, actionApiMap); - // Now resolve and execute the inner Action tree. - final dynamic bodyNode = definition['body']; - if (bodyNode == null) { - throw LanguageError( - "Action '$name' must define a 'body' payload to run."); - } + try { + // Build a child scope with optional Import, API, Global, and input parameters + final ScopeManager childScope = ActionScopeUtil.prepareScope( + parentScope: scopeManager, + definition: definition, + parameters: parameters, + callInputs: rawInputs ?? const {}, + eventHandlers: eventHandlers, + ); - final EnsembleAction? innerAction = - EnsembleAction.from(Utils.getYamlMap(bodyNode)); - if (innerAction == null) { - throw LanguageError("Action '$name' contains an invalid 'body' payload."); - } + // Now resolve and execute the inner Action tree. + final dynamic bodyNode = definition['body']; + if (bodyNode == null) { + throw LanguageError( + "Action '$name' must define a 'body' payload to run."); + } + + final EnsembleAction? innerAction = + EnsembleAction.from(Utils.getYamlMap(bodyNode)); + if (innerAction == null) { + throw LanguageError( + "Action '$name' contains an invalid 'body' payload."); + } - return ScreenController() - .executeActionWithScope(context, childScope, innerAction); + return ScreenController() + .executeActionWithScope(context, childScope, innerAction); + } finally { + ActionScopeUtil.restorePageApisAfterAction(scopeManager, apiSnapshot); + } } } diff --git a/modules/ensemble/test/action_scope_api_restore_test.dart b/modules/ensemble/test/action_scope_api_restore_test.dart new file mode 100644 index 000000000..2c80a5233 --- /dev/null +++ b/modules/ensemble/test/action_scope_api_restore_test.dart @@ -0,0 +1,99 @@ +import 'package:ensemble/action/action_scope_util.dart'; +import 'package:ensemble/framework/data_context.dart'; +import 'package:ensemble/framework/scope.dart'; +import 'package:flutter/widgets.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:mockito/mockito.dart'; +import 'package:yaml/yaml.dart'; + +class _MockBuildContext extends Mock implements BuildContext {} + +void main() { + group('reusable action scoped API restore', () { + late ScopeManager scopeManager; + late YamlMap pageApi; + late YamlMap actionApi; + + setUp(() { + pageApi = YamlMap.wrap({ + 'url': 'https://example.com/page', + 'method': 'GET', + }); + actionApi = YamlMap.wrap({ + 'url': 'https://example.com/action', + 'method': 'POST', + }); + + scopeManager = ScopeManager( + DataContext(buildContext: _MockBuildContext()), + PageData(apiMap: {'sharedApi': pageApi}), + ); + }); + + test('removes action-only APIs after restore', () { + final snapshot = ActionScopeUtil.snapshotPageApisForAction( + scopeManager, + {'actionOnly': actionApi}, + ); + + scopeManager.pageData.apiMap!.addAll({'actionOnly': actionApi}); + expect(scopeManager.pageData.apiMap!.containsKey('actionOnly'), isTrue); + + ActionScopeUtil.restorePageApisAfterAction(scopeManager, snapshot); + + expect(scopeManager.pageData.apiMap!.containsKey('actionOnly'), isFalse); + expect(scopeManager.pageData.apiMap!['sharedApi'], same(pageApi)); + }); + + test('restores page APIs overwritten by action-scoped names', () { + final snapshot = ActionScopeUtil.snapshotPageApisForAction( + scopeManager, + {'sharedApi': actionApi}, + ); + + scopeManager.pageData.apiMap!['sharedApi'] = actionApi; + expect( + scopeManager.pageData.apiMap!['sharedApi']!['url'], + 'https://example.com/action', + ); + + ActionScopeUtil.restorePageApisAfterAction(scopeManager, snapshot); + + expect(scopeManager.pageData.apiMap!['sharedApi'], same(pageApi)); + }); + + test('prepareScope merge is undone by restore', () { + final definition = YamlMap.wrap({ + 'API': { + 'actionOnly': actionApi, + 'sharedApi': actionApi, + }, + 'body': { + 'showToast': {'message': 'done'}, + }, + }); + + final snapshot = ActionScopeUtil.snapshotPageApisForAction( + scopeManager, + ActionScopeUtil.parseApiMap(definition), + ); + + ActionScopeUtil.prepareScope( + parentScope: scopeManager, + definition: definition, + parameters: const [], + ); + + expect(scopeManager.pageData.apiMap!.keys, containsAll(['actionOnly'])); + expect( + scopeManager.pageData.apiMap!['sharedApi']!['url'], + 'https://example.com/action', + ); + + ActionScopeUtil.restorePageApisAfterAction(scopeManager, snapshot); + + expect(scopeManager.pageData.apiMap!.containsKey('actionOnly'), isFalse); + expect(scopeManager.pageData.apiMap!['sharedApi'], same(pageApi)); + }); + }); +}