Skip to content

Commit db56bb7

Browse files
authored
fix(limrun): keep the owner's reverse mappings on attached Android instances (#3204)
* fix(android): add a no-rebind mode to the port-reverse provider createExecAndroidPortReverseProvider and createAndroidPortReverseManager (executor form) take { noRebind }. The provider then runs adb reverse --no-rebind for an endpoint it did not bind, still rebinds its own mappings, and fails a refusal with COMMAND_FAILED and details.reason 'android_port_reverse_rebind_refused'. The refusal is classified from adb reverse --list, not from adb's stderr. The localhost URL auto-reverse keeps that reason on the error it rethrows. * fix(limrun): keep the owner's reverse mappings on attached Android instances createLimrunAndroidSession asks createPortReverse for no-rebind mode when ownership is 'attached'. configurePortReverse and the localhost URL auto-reverse can then no longer replace an owner mapping such as tcp:8081, so teardown cannot remove it. Created instances keep plain adb reverse. * fix(android): never rebind an existing mapping in no-rebind mode No-rebind mode now passes --no-rebind on every ensure, so a stale record of a mapping this provider bound can no longer turn into a plain adb reverse over another client's mapping. The manager serializes ensures per device endpoint, so a concurrent duplicate sees the first mapping instead of a refusal, and two owners can no longer both pass its ownership check. Docs state when the typed reason is available. * docs(android): state when the no-rebind refusal carries its typed reason Name adb reverse --list as the condition in the noRebind JSDoc, and scope the Limrun docs guarantee to attached Android instances. * fix(android): give localhost URL reverses an owner The localhost URL auto-reverse now ensures its mapping with the stable owner 'localhost-url'. Teardown of an attached Limrun instance removes only owned mappings, so before this change the mapping stayed on the owner's device after disconnect. A live run showed that adbd keeps it.
1 parent 7faae56 commit db56bb7

10 files changed

Lines changed: 355 additions & 73 deletions

File tree

‎packages/platform-android/src/__tests__/app-lifecycle-open.test.ts‎

Lines changed: 46 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,8 @@ test('openAndroidApp ensures Android reverse before localhost deep link launch',
171171
booted: true,
172172
};
173173
const calls: Array<
174-
{ kind: 'exec'; args: readonly string[] } | { kind: 'reverse'; local: string; remote: string }
174+
| { kind: 'exec'; args: readonly string[] }
175+
| { kind: 'reverse'; local: string; remote: string; ownerId?: string }
175176
> = [];
176177

177178
await withAndroidAdbProvider(
@@ -182,7 +183,12 @@ test('openAndroidApp ensures Android reverse before localhost deep link launch',
182183
},
183184
reverse: {
184185
ensure: async (mapping) => {
185-
calls.push({ kind: 'reverse', local: mapping.local, remote: mapping.remote });
186+
calls.push({
187+
kind: 'reverse',
188+
local: mapping.local,
189+
remote: mapping.remote,
190+
ownerId: mapping.ownerId,
191+
});
186192
},
187193
remove: async () => {},
188194
removeAllOwned: async () => {},
@@ -193,7 +199,7 @@ test('openAndroidApp ensures Android reverse before localhost deep link launch',
193199
);
194200

195201
assert.deepEqual(calls, [
196-
{ kind: 'reverse', local: 'tcp:8083', remote: 'tcp:8083' },
202+
{ kind: 'reverse', local: 'tcp:8083', remote: 'tcp:8083', ownerId: 'localhost-url' },
197203
{
198204
kind: 'exec',
199205
args: [
@@ -210,6 +216,43 @@ test('openAndroidApp ensures Android reverse before localhost deep link launch',
210216
]);
211217
});
212218

219+
test('openAndroidApp keeps the typed reason of a refused localhost reverse', async () => {
220+
const device: DeviceInfo = {
221+
platform: 'android',
222+
id: 'emulator-5554',
223+
name: 'Pixel',
224+
kind: 'emulator',
225+
booted: true,
226+
};
227+
const launches: (readonly string[])[] = [];
228+
229+
await assert.rejects(
230+
() =>
231+
withAndroidAdbProvider(
232+
{
233+
exec: async (args) => {
234+
launches.push(args);
235+
return { stdout: '', stderr: '', exitCode: 0 };
236+
},
237+
reverse: {
238+
ensure: async () => {
239+
throw new AppError('COMMAND_FAILED', 'already mapped', {
240+
reason: 'android_port_reverse_rebind_refused',
241+
});
242+
},
243+
remove: async () => {},
244+
removeAllOwned: async () => {},
245+
},
246+
},
247+
{ serial: 'emulator-5554' },
248+
async () => await openAndroidApp(device, 'exp://127.0.0.1:8081'),
249+
),
250+
(error: unknown) =>
251+
error instanceof AppError && error.details?.reason === 'android_port_reverse_rebind_refused',
252+
);
253+
assert.deepEqual(launches, []);
254+
});
255+
213256
test('openAndroidApp ensures Android reverse before localhost app-bound deep link launch', async () => {
214257
const device: DeviceInfo = {
215258
platform: 'android',

‎packages/platform-android/src/adb-port-reverse.test.ts‎

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,3 +67,121 @@ test('a provider-owned reverse implementation is reused as-is when already manag
6767
const second = createAndroidPortReverseManager({ exec: async () => ok(), reverse: first });
6868
expect(second).toBe(first);
6969
});
70+
71+
test('no-rebind refuses a device mapping another client owns with a typed reason', async () => {
72+
bindAndroidAdbHostStub();
73+
const calls: (readonly string[])[] = [];
74+
const manager = createAndroidPortReverseManager(
75+
async (args) => {
76+
calls.push(args);
77+
if (args.includes('--no-rebind')) {
78+
return { exitCode: 1, stdout: '', stderr: 'adb: error: cannot rebind existing socket' };
79+
}
80+
return ok(args[1] === '--list' ? 'owner-host tcp:8081 tcp:8081\n' : '');
81+
},
82+
{ noRebind: true },
83+
);
84+
85+
await expect(
86+
manager.ensure({ local: 'tcp:8081', remote: 'tcp:8081', ownerId: 'metro' }),
87+
).rejects.toMatchObject({
88+
code: 'COMMAND_FAILED',
89+
details: {
90+
reason: 'android_port_reverse_rebind_refused',
91+
existing: { local: 'tcp:8081', remote: 'tcp:8081' },
92+
},
93+
});
94+
await manager.removeAllOwned('metro');
95+
96+
expect(calls).toEqual([
97+
['reverse', '--no-rebind', 'tcp:8081', 'tcp:8081'],
98+
['reverse', '--list'],
99+
]);
100+
});
101+
102+
test('no-rebind does not re-point a mapping the same provider created', async () => {
103+
bindAndroidAdbHostStub();
104+
const calls: (readonly string[])[] = [];
105+
const device = new Map<string, string>();
106+
const manager = createAndroidPortReverseManager(
107+
async (args) => {
108+
calls.push(args);
109+
if (args[1] === '--list') {
110+
return ok([...device].map(([local, remote]) => `host-1 ${local} ${remote}\n`).join(''));
111+
}
112+
const [local, remote] = args.slice(-2) as [string, string];
113+
if (args.includes('--no-rebind') && device.has(local)) {
114+
return { exitCode: 1, stdout: '', stderr: 'adb: error: cannot rebind existing socket' };
115+
}
116+
device.set(local, remote);
117+
return ok();
118+
},
119+
{ noRebind: true },
120+
);
121+
122+
await manager.ensure({ local: 'tcp:8081', remote: 'tcp:8081', ownerId: 'metro' });
123+
await expect(
124+
manager.ensure({ local: 'tcp:8081', remote: 'tcp:9090', ownerId: 'metro' }),
125+
).rejects.toMatchObject({
126+
details: {
127+
reason: 'android_port_reverse_rebind_refused',
128+
existing: { local: 'tcp:8081', remote: 'tcp:8081', ownerId: 'metro' },
129+
},
130+
});
131+
132+
expect(device.get('tcp:8081')).toBe('tcp:8081');
133+
expect(calls.filter((args) => args[1] !== '--list')).toEqual([
134+
['reverse', '--no-rebind', 'tcp:8081', 'tcp:8081'],
135+
['reverse', '--no-rebind', 'tcp:8081', 'tcp:9090'],
136+
]);
137+
});
138+
139+
test('concurrent ensures of one endpoint reach the device once', async () => {
140+
bindAndroidAdbHostStub();
141+
const binds: (readonly string[])[] = [];
142+
const device = new Set<string>();
143+
const manager = createAndroidPortReverseManager(
144+
async (args) => {
145+
if (args[1] === '--list')
146+
return ok([...device].map((local) => `host-1 ${local} ${local}\n`).join(''));
147+
binds.push(args);
148+
const local = args.at(-2) as string;
149+
if (device.has(local)) {
150+
return { exitCode: 1, stdout: '', stderr: 'adb: error: cannot rebind existing socket' };
151+
}
152+
device.add(local);
153+
await new Promise((resolve) => setTimeout(resolve, 5));
154+
return ok();
155+
},
156+
{ noRebind: true },
157+
);
158+
const mapping = { local: 'tcp:8081', remote: 'tcp:8081', ownerId: 'metro' } as const;
159+
160+
await Promise.all([manager.ensure(mapping), manager.ensure(mapping)]);
161+
162+
expect(binds).toEqual([['reverse', '--no-rebind', 'tcp:8081', 'tcp:8081']]);
163+
});
164+
165+
test('no-rebind reports an adb failure when the device lists no mapping for the endpoint', async () => {
166+
bindAndroidAdbHostStub();
167+
const manager = createAndroidPortReverseManager(
168+
async (args) =>
169+
args.includes('--no-rebind')
170+
? { exitCode: 1, stdout: '', stderr: 'error: device offline' }
171+
: ok(),
172+
{ noRebind: true },
173+
);
174+
175+
const failure = await manager
176+
.ensure({ local: 'tcp:8081', remote: 'tcp:8081', ownerId: 'metro' })
177+
.then(
178+
() => undefined,
179+
(error: unknown) => error,
180+
);
181+
182+
expect(failure).toMatchObject({
183+
code: 'COMMAND_FAILED',
184+
details: { adbFailure: 'device_offline' },
185+
});
186+
expect(failure).not.toMatchObject({ details: { reason: 'android_port_reverse_rebind_refused' } });
187+
});

0 commit comments

Comments
 (0)