Skip to content

Commit a0315da

Browse files
authored
fix(agentic): deduplicate forwarded test ID targets (MetaMask#35762)
## **Description** React can forward one `testID` and press handler through several fibers. `pressTestId` now collapses only nested matches that share the same handler, disabled state, and measured native target. Distinct nested or sibling controls remain ambiguous and fail closed. ## **Changelog** CHANGELOG entry: null ## **Related issues** Refs: https://consensyssoftware.atlassian.net/browse/TAT-3919 Follow-up to MetaMask#35746 and MetaMask/experimental-metamask-harness#207. ## **Manual testing steps** ```gherkin Feature: Agentic press target resolution Scenario: Press a test ID forwarded through nested React fibers Given a Mobile dev build exposes one control through nested fibers When a Recipe v1 flow presses the forwarded test ID Then the control is pressed exactly once And distinct nested controls remain ambiguous ``` Validation: - `yarn jest app/dev-tools/AgenticService/AgenticService.test.ts --no-coverage` - `yarn lint:tsc` - `yarn eslint --quiet app/dev-tools/AgenticService/AgenticService.ts app/dev-tools/AgenticService/AgenticService.test.ts` - Live iOS advanced-order recipe: 134/134 nodes passed on commit `9c7554f2bf` source. ## **Screenshots/Recordings** ### **Before** N/A. This changes a development-only control bridge. ### **After** N/A. The bridge has no visible UI. Live iOS proof showed an active TWAP card, a Chase row at `Running · 0%`, and all three Scale child orders. ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I've included tests if applicable - [x] I've documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I've applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. #### Performance checks (if applicable) - [x] I've tested on Android - Ideally on a mid-range device; emulator is acceptable - [x] I've tested with a power user scenario - Use these [power-user SRPs](https://consensyssoftware.atlassian.net/wiki/spaces/TL1/pages/edit-v2/401401446401?draftShareId=9d77e1e1-4bdc-4be1-9ebb-ccd916988d93) to import wallets with many accounts and tokens - [x] I've instrumented key operations with Sentry traces for production performance metrics - See [`trace()`](/app/util/trace.ts) for usage and [`addToken`](/app/components/Views/AddAsset/components/AddCustomToken/AddCustomToken.tsx#L274) for an example For performance guidelines and tooling, see the [Performance Guide](https://consensyssoftware.atlassian.net/wiki/spaces/TL1/pages/400085549067/Performance+Guide+for+Engineers). ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. <!-- Generated with the help of the pr-description AI skill -->
1 parent 8e11f95 commit a0315da

2 files changed

Lines changed: 76 additions & 3 deletions

File tree

app/dev-tools/AgenticService/AgenticService.test.ts

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -902,6 +902,53 @@ describe('AgenticService.install', () => {
902902
expect(visiblePress).toHaveBeenCalledTimes(1);
903903
});
904904

905+
it('presses one control whose test ID is forwarded through nested fibers', async () => {
906+
const onPress = jest.fn();
907+
const stateNode = {
908+
measureInWindow: (callback) => callback(10, 10, 40, 40),
909+
} as FiberNode['stateNode'];
910+
const innerTarget = makeFiber({
911+
testID: 'forwarded-button',
912+
onPress,
913+
stateNode,
914+
});
915+
const outerTarget = makeFiber({
916+
testID: 'forwarded-button',
917+
onPress,
918+
child: innerTarget,
919+
});
920+
innerTarget.return = outerTarget;
921+
installFiberHook(makeFiber({ child: outerTarget }));
922+
923+
const result = await bridge().pressTestId('forwarded-button');
924+
925+
expect(result).toEqual({ ok: true, testId: 'forwarded-button' });
926+
expect(onPress).toHaveBeenCalledTimes(1);
927+
});
928+
929+
it('does not collapse nested controls with different press handlers', async () => {
930+
const outerPress = jest.fn();
931+
const innerPress = jest.fn();
932+
const innerTarget = makeFiber({
933+
testID: 'nested-button',
934+
onPress: innerPress,
935+
});
936+
const outerTarget = makeFiber({
937+
testID: 'nested-button',
938+
onPress: outerPress,
939+
child: innerTarget,
940+
});
941+
innerTarget.return = outerTarget;
942+
installFiberHook(makeFiber({ child: outerTarget }));
943+
944+
const result = await bridge().pressTestId('nested-button');
945+
946+
expect(result.ok).toBe(false);
947+
expect(result.error).toContain('duplicate matches');
948+
expect(outerPress).not.toHaveBeenCalled();
949+
expect(innerPress).not.toHaveBeenCalled();
950+
});
951+
905952
it('does not borrow a visible frame from a duplicate sibling', async () => {
906953
const unmeasurablePress = jest.fn();
907954
const visiblePress = jest.fn();

app/dev-tools/AgenticService/AgenticService.ts

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -367,6 +367,31 @@ function findFiberByTestId(
367367
return result;
368368
}
369369

370+
/**
371+
* Collapse matching fibers nested on the same component path. React forwards
372+
* props through composite components, so one rendered control can expose the
373+
* same testID and press handler on several ancestor/descendant fibers.
374+
*/
375+
function collapseNestedFibers(fibers: FiberNode[]): FiberNode[] {
376+
const candidates = new Set(fibers);
377+
return fibers.filter((fiber) => {
378+
let ancestor = fiber.return;
379+
while (ancestor) {
380+
if (
381+
candidates.has(ancestor) &&
382+
ancestor.memoizedProps?.onPress === fiber.memoizedProps?.onPress &&
383+
isFiberDisabled(ancestor) === isFiberDisabled(fiber) &&
384+
findMeasurableStateNode(ancestor, false) ===
385+
findMeasurableStateNode(fiber, false)
386+
) {
387+
return false;
388+
}
389+
ancestor = ancestor.return;
390+
}
391+
return true;
392+
});
393+
}
394+
370395
/**
371396
* Iterate all React renderer roots and call `visitor` on each root fiber.
372397
* Returns true if any visitor call returns true.
@@ -1217,9 +1242,10 @@ const AgenticService = {
12171242
});
12181243
return false;
12191244
});
1245+
const distinctCandidates = collapseNestedFibers(candidates);
12201246
const viewport = Dimensions.get('window');
12211247
const measuredCandidates = await Promise.all(
1222-
candidates.map(async (fiber) => ({
1248+
distinctCandidates.map(async (fiber) => ({
12231249
fiber,
12241250
rect: await measureStateNode(
12251251
findMeasurableStateNode(fiber, false),
@@ -1239,13 +1265,13 @@ const AgenticService = {
12391265
)?.fiber;
12401266
const candidate =
12411267
visibleCandidate ??
1242-
(candidates.length === 1 ? candidates[0] : null);
1268+
(distinctCandidates.length === 1 ? distinctCandidates[0] : null);
12431269
if (!candidate) {
12441270
return {
12451271
ok: false,
12461272
testId,
12471273
error:
1248-
candidates.length > 1
1274+
distinctCandidates.length > 1
12491275
? `No visible component with testID="${testId}" found among duplicate matches`
12501276
: `No component with testID="${testId}" found or no onPress prop`,
12511277
};

0 commit comments

Comments
 (0)