Skip to content

Commit a258fbc

Browse files
thymikeethymikee
committed
fix(android): fail closed when the IME rebind record cannot be read or cleared
An unreadable record keeps the recorded restore target, an idempotent activation keeps a rebind the record still marks, and a rebind stays unconfirmed until its record is written and later cleared.
1 parent ad5683c commit a258fbc

6 files changed

Lines changed: 174 additions & 33 deletions

File tree

‎packages/platform-android/src/ime-activation.test.ts‎

Lines changed: 101 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,17 @@ import {
66
} from '@agent-device/kernel/errors';
77
import type { DeviceInfo } from '@agent-device/kernel/device';
88
import type { AndroidImeHelperArtifact } from './helper-artifacts.ts';
9+
import type { AndroidAdbExecutor } from './adb-transport.ts';
910
import { bindAndroidAdbHostStub, type AndroidAdbHostStub } from './adb-host.fixtures.ts';
1011
import { withAndroidAdbProvider } from './adb-provider-scope.ts';
1112
import { activateAndroidTestIme, rebindAndroidTestIme } from './ime-activation.ts';
1213
import { restoreAndroidTestIme } from './ime-restore.ts';
1314
import { fakeImeDeviceAdb, type FakeImeDeviceState } from './ime-device.fixtures.ts';
14-
import { resetAndroidTestImeActivationCacheForTests, isAndroidTestImeActive } from './ime-state.ts';
15+
import {
16+
getAndroidTestImeOwnership,
17+
isAndroidTestImeActive,
18+
resetAndroidTestImeActivationCacheForTests,
19+
} from './ime-state.ts';
1520
import { resetAndroidImeHelperInstallCache } from './ime-helper.ts';
1621

1722
const DEVICE: DeviceInfo = {
@@ -286,3 +291,98 @@ test('a close-time restore waits for an in-flight rebind instead of racing it',
286291
expect(state.settings.get('default_input_method')).toBe('com.samsung/.Keyboard');
287292
expect(isAndroidTestImeActive(DEVICE)).toBe(false);
288293
});
294+
295+
async function withDeviceAdb<T>(exec: AndroidAdbExecutor, task: () => Promise<T>): Promise<T> {
296+
return await withAndroidAdbProvider(
297+
{ exec, imeHelperArtifact: ARTIFACT },
298+
{ serial: DEVICE.id },
299+
task,
300+
);
301+
}
302+
303+
const REBIND_DISPLACED = 'agent_device_ime_helper_rebind_displaced';
304+
305+
test('activation keeps the recorded IME when the rebind record cannot be read', async () => {
306+
activationHost();
307+
const state: FakeImeDeviceState = {
308+
settings: new Map([['default_input_method', 'com.samsung/.Keyboard']]),
309+
};
310+
await activateWith(state);
311+
state.imeDisableFallback = 'com.android.inputmethod.latin/.LatinIME';
312+
state.imeSetFails = true;
313+
await rebindWith(state);
314+
state.imeSetFails = false;
315+
resetAndroidTestImeActivationCacheForTests();
316+
const deviceAdb = fakeImeDeviceAdb(state);
317+
318+
const result = await withDeviceAdb(
319+
async (args) =>
320+
args[1] === 'settings' && args[2] === 'get' && args[4] === REBIND_DISPLACED
321+
? { exitCode: 1, stdout: '', stderr: 'timed out' }
322+
: await deviceAdb(args),
323+
async () => await activateAndroidTestIme(DEVICE, { stateDir: STATE_DIR }),
324+
);
325+
326+
expect(result).toMatchObject({ activated: true, previousIme: 'com.samsung/.Keyboard' });
327+
expect(state.settings.get('agent_device_ime_helper_previous_ime')).toBe('com.samsung/.Keyboard');
328+
});
329+
330+
test('an idempotent activation keeps a rebind the device record marks as unconfirmed', async () => {
331+
activationHost();
332+
const state: FakeImeDeviceState = {
333+
settings: new Map([['default_input_method', 'com.samsung/.Keyboard']]),
334+
};
335+
await activateWith(state);
336+
state.settings.set(REBIND_DISPLACED, '1');
337+
resetAndroidTestImeActivationCacheForTests();
338+
339+
const result = await activateWith(state);
340+
341+
expect(result).toMatchObject({ alreadyActive: true });
342+
expect(getAndroidTestImeOwnership(DEVICE)?.rebindUnconfirmed).toBe(true);
343+
});
344+
345+
test('a rebind whose device record cannot be written stays unconfirmed', async () => {
346+
activationHost();
347+
const state: FakeImeDeviceState = {
348+
settings: new Map([['default_input_method', 'com.samsung/.Keyboard']]),
349+
};
350+
await activateWith(state);
351+
const deviceAdb = fakeImeDeviceAdb(state);
352+
353+
const rebound = await withDeviceAdb(
354+
async (args) =>
355+
args[1] === 'settings' && args[2] === 'put' && args[4] === REBIND_DISPLACED
356+
? { exitCode: 1, stdout: '', stderr: 'rejected' }
357+
: await deviceAdb(args),
358+
async () => await rebindAndroidTestIme(DEVICE),
359+
);
360+
361+
expect(rebound).toBe(false);
362+
expect(getAndroidTestImeOwnership(DEVICE)?.rebindUnconfirmed).toBe(true);
363+
});
364+
365+
test('a confirmed rebind whose device record stays set is retried at the next entry', async () => {
366+
const host = activationHost();
367+
const state: FakeImeDeviceState = {
368+
settings: new Map([['default_input_method', 'com.samsung/.Keyboard']]),
369+
};
370+
await activateWith(state);
371+
const deviceAdb = fakeImeDeviceAdb(state);
372+
373+
const rebound = await withDeviceAdb(
374+
async (args) =>
375+
args[1] === 'settings' && args[2] === 'delete' && args[4] === REBIND_DISPLACED
376+
? { exitCode: 1, stdout: '', stderr: 'rejected' }
377+
: await deviceAdb(args),
378+
async () => await rebindAndroidTestIme(DEVICE),
379+
);
380+
381+
expect(rebound).toBe(true);
382+
expect(state.settings.get(REBIND_DISPLACED)).toBe('1');
383+
expect(getAndroidTestImeOwnership(DEVICE)?.rebindUnconfirmed).toBe(true);
384+
expect(host.diagnostics).toContainEqual({
385+
phase: 'android_test_ime_rebind_record_clear_failed',
386+
level: 'warn',
387+
});
388+
});

‎packages/platform-android/src/ime-activation.ts‎

Lines changed: 60 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { emitAndroidAdbDiagnostic, requireAndroidAdbHost } from './adb-host.ts';
55
import { resolveAndroidAdbExecutor, resolveAndroidAdbProvider } from './adb-provider-scope.ts';
66
import { runAdbShell } from './adb-executor.ts';
77
import type { AndroidAdbExecutor } from './adb-transport.ts';
8+
import type { AndroidImeHelperArtifact } from './helper-artifacts.ts';
89
import {
910
ANDROID_IME_HELPER_SERVICE_COMPONENT,
1011
ensureAndroidImeHelper,
@@ -80,15 +81,20 @@ async function readActivationRestoreTarget(
8081
priorPersistedIme: string | undefined,
8182
): Promise<string> {
8283
if (priorPersistedIme === undefined) return currentIme;
83-
return (await readPersistedRebindDisplacement(adb)) ? priorPersistedIme : currentIme;
84+
// An unreadable record keeps the recorded target rather than risk recording the fallback.
85+
return (await readPersistedRebindDisplacement(adb)) === false ? currentIme : priorPersistedIme;
8486
}
8587

86-
/** A confirmed activation settles any earlier unconfirmed rebind, which only a prior record can carry. */
88+
/**
89+
* A confirmed activation settles any earlier unconfirmed rebind, which only a prior record can carry.
90+
* Answers false when the record may still mark the rebind.
91+
*/
8792
async function settleRebindDisplacement(
8893
adb: AndroidAdbExecutor,
8994
priorPersistedIme: string | undefined,
90-
): Promise<void> {
91-
if (priorPersistedIme !== undefined) await clearPersistedRebindDisplacement(adb);
95+
): Promise<boolean> {
96+
if (priorPersistedIme === undefined) return true;
97+
return await clearPersistedRebindDisplacement(adb);
9298
}
9399

94100
function androidTestImeUnavailableOutcome(error: unknown): AndroidTestImeActivationResult {
@@ -123,23 +129,7 @@ async function activateAndroidTestImeAfterStartupRecovery(
123129

124130
const currentIme = await readAndroidDefaultInputMethod(adb);
125131
if (currentIme === manifest.serviceComponent) {
126-
// Already active (idempotent call, or a previous crashed daemon left it active); keep the
127-
// existing persisted previous-IME record rather than overwriting it, but make sure this
128-
// process's crash is covered by a recovery marker.
129-
const markerPersisted = await markers.write(options.stateDir, device.id);
130-
const previousIme = await readPersistedPreviousIme(adb);
131-
if (markerPersisted) {
132-
activeTestImeDevices.set(deviceKey, { stateDir: options.stateDir, rebindUnconfirmed: false });
133-
}
134-
return {
135-
outcome: 'settled',
136-
activated: false,
137-
alreadyActive: true,
138-
...(markerPersisted ? {} : { persistFailed: true }),
139-
previousIme,
140-
helperServiceComponent: manifest.serviceComponent,
141-
helperPackageName: manifest.packageName,
142-
};
132+
return await claimAlreadyActiveTestIme(device, adb, manifest, options.stateDir);
143133
}
144134

145135
// Durably record the restore target BEFORE the switch: confirm the settings write succeeded and
@@ -222,8 +212,11 @@ async function activateAndroidTestImeAfterStartupRecovery(
222212

223213
// The recovery lock spans both durable records and the switch, so this process only claims
224214
// active ownership after the helper is confirmed active on the device.
225-
activeTestImeDevices.set(deviceKey, { stateDir: options.stateDir, rebindUnconfirmed: false });
226-
await settleRebindDisplacement(adb, priorPersistedIme);
215+
const rebindSettled = await settleRebindDisplacement(adb, priorPersistedIme);
216+
activeTestImeDevices.set(deviceKey, {
217+
stateDir: options.stateDir,
218+
rebindUnconfirmed: !rebindSettled,
219+
});
227220
emitAndroidAdbDiagnostic({
228221
phase: 'android_test_ime_activated',
229222
data: { device: device.id, previousIme },
@@ -238,6 +231,39 @@ async function activateAndroidTestImeAfterStartupRecovery(
238231
};
239232
}
240233

234+
async function claimAlreadyActiveTestIme(
235+
device: DeviceInfo,
236+
adb: AndroidAdbExecutor,
237+
manifest: AndroidImeHelperArtifact['manifest'],
238+
stateDir: string,
239+
): Promise<AndroidTestImeActivationResult> {
240+
const deviceKey = getAndroidImeHelperDeviceKey(device);
241+
// Already active (idempotent call, or a previous crashed daemon left it active); keep the
242+
// existing persisted previous-IME record rather than overwriting it, but make sure this
243+
// process's crash is covered by a recovery marker.
244+
const markerPersisted = await requireAndroidAdbHost().imeRecoveryMarkers.write(
245+
stateDir,
246+
device.id,
247+
);
248+
const previousIme = await readPersistedPreviousIme(adb);
249+
if (markerPersisted) {
250+
// A rebind that a previous activation or process left unconfirmed still needs one.
251+
const rebindUnconfirmed =
252+
activeTestImeDevices.get(deviceKey)?.rebindUnconfirmed === true ||
253+
(await readPersistedRebindDisplacement(adb)) !== false;
254+
activeTestImeDevices.set(deviceKey, { stateDir, rebindUnconfirmed });
255+
}
256+
return {
257+
outcome: 'settled',
258+
activated: false,
259+
alreadyActive: true,
260+
...(markerPersisted ? {} : { persistFailed: true }),
261+
previousIme,
262+
helperServiceComponent: manifest.serviceComponent,
263+
helperPackageName: manifest.packageName,
264+
};
265+
}
266+
241267
/**
242268
* Rebinds the owned test IME so the focused field starts a fresh input session with it, and answers
243269
* whether the helper reads back as the selected IME. Serialized with activation and restore under
@@ -256,6 +282,7 @@ export async function rebindAndroidTestIme(device: DeviceInfo): Promise<boolean>
256282
phase: 'android_test_ime_rebind',
257283
data: { device: device.id },
258284
});
285+
ownership.rebindUnconfirmed = true;
259286
if (!(await writePersistedRebindDisplacement(adb))) {
260287
emitAndroidAdbDiagnostic({
261288
level: 'warn',
@@ -264,11 +291,18 @@ export async function rebindAndroidTestIme(device: DeviceInfo): Promise<boolean>
264291
});
265292
return false;
266293
}
267-
ownership.rebindUnconfirmed = true;
268294
const activeIme = await rebindAndReadSelectedIme(adb);
269295
if (activeIme === ANDROID_IME_HELPER_SERVICE_COMPONENT) {
270-
ownership.rebindUnconfirmed = false;
271-
await clearPersistedRebindDisplacement(adb);
296+
// The helper holds a fresh session either way; an uncleared record keeps the next entry on
297+
// the rebind path, which retries the clear.
298+
ownership.rebindUnconfirmed = !(await clearPersistedRebindDisplacement(adb));
299+
if (ownership.rebindUnconfirmed) {
300+
emitAndroidAdbDiagnostic({
301+
level: 'warn',
302+
phase: 'android_test_ime_rebind_record_clear_failed',
303+
data: { device: device.id },
304+
});
305+
}
272306
return true;
273307
}
274308
emitAndroidAdbDiagnostic({

‎packages/platform-android/src/ime-restore.test.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,5 +163,6 @@ test("startup recovery undoes a crashed daemon's unconfirmed rebind", async () =
163163

164164
expect(state.settings.get('default_input_method')).toBe('com.samsung/.Keyboard');
165165
expect(state.settings.has('agent_device_ime_helper_rebind_displaced')).toBe(false);
166+
expect(state.settings.has('agent_device_ime_helper_previous_ime')).toBe(false);
166167
expect([...(host.markerStore.get(STATE_DIR) ?? [])]).toEqual([]);
167168
});

‎packages/platform-android/src/ime-restore.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ async function restoreAndroidTestImeFor(
8585
const currentIme = await readAndroidDefaultInputMethod(adb);
8686
if (
8787
currentIme !== ANDROID_IME_HELPER_SERVICE_COMPONENT &&
88-
!(await readPersistedRebindDisplacement(adb))
88+
(await readPersistedRebindDisplacement(adb)) !== true
8989
) {
9090
// Helper is not active — the user switched away, or the helper was never really set. Do not
9191
// overwrite the current IME, and do not clear the device record (a concurrent activation could

‎packages/platform-android/src/ime-settings-record.ts‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -62,13 +62,17 @@ export async function clearPersistedPreviousIme(adb: AndroidAdbExecutor): Promis
6262
});
6363
}
6464

65-
export async function readPersistedRebindDisplacement(adb: AndroidAdbExecutor): Promise<boolean> {
65+
/** Whether the device record marks an unconfirmed rebind, or `undefined` when it cannot be read. */
66+
export async function readPersistedRebindDisplacement(
67+
adb: AndroidAdbExecutor,
68+
): Promise<boolean | undefined> {
6669
const result = await runAdbShell(
6770
adb,
6871
['settings', 'get', SETTINGS_NAMESPACE, SETTINGS_KEY_REBIND_DISPLACED],
6972
{ allowFailure: true, timeoutMs: 5_000 },
7073
);
71-
return result.exitCode === 0 && normalizeSettingsValue(result.stdout) === '1';
74+
if (result.exitCode !== 0) return undefined;
75+
return normalizeSettingsValue(result.stdout) === '1';
7276
}
7377

7478
/** Answers true only when the write succeeded and reads back. */
@@ -79,10 +83,11 @@ export async function writePersistedRebindDisplacement(adb: AndroidAdbExecutor):
7983
{ allowFailure: true, timeoutMs: 5_000 },
8084
);
8185
if (result.exitCode !== 0) return false;
82-
return await readPersistedRebindDisplacement(adb);
86+
return (await readPersistedRebindDisplacement(adb)) === true;
8387
}
8488

85-
export async function clearPersistedRebindDisplacement(adb: AndroidAdbExecutor): Promise<void> {
89+
/** Answers true only when the record reads back as cleared. */
90+
export async function clearPersistedRebindDisplacement(adb: AndroidAdbExecutor): Promise<boolean> {
8691
await runAdbShell(
8792
adb,
8893
['settings', 'delete', SETTINGS_NAMESPACE, SETTINGS_KEY_REBIND_DISPLACED],
@@ -91,6 +96,7 @@ export async function clearPersistedRebindDisplacement(adb: AndroidAdbExecutor):
9196
timeoutMs: 5_000,
9297
},
9398
);
99+
return (await readPersistedRebindDisplacement(adb)) === false;
94100
}
95101

96102
/** Restores the device record changed by a failed pre-switch transaction; never touches markers. */

‎packages/platform-android/src/text-input.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,7 @@ async function confirmAndroidTestImeRebound(device: DeviceInfo): Promise<void> {
223223
if (await rebindAndroidTestIme(device)) return;
224224
throw new AppError(
225225
'COMMAND_FAILED',
226-
`Android test IME is not the selected input method on ${device.name ?? device.id}.`,
226+
`Could not confirm the Android test IME rebind on ${device.name ?? device.id}.`,
227227
{
228228
reason: 'android_test_ime_rebind_unconfirmed',
229229
deviceId: device.id,

0 commit comments

Comments
 (0)