Merge remote-tracking branch 'origin/main' into claude/ios-webview-remote-content-wait-fix

* origin/main:
  fix(android): retire completed recording evidence after pid reuse (#2487)

# Conflicts:
#	CHANGELOG.md

Author: thymikee

CHANGELOG.md

@@ -2,6 +2,16 @@
 
 ## Unreleased
 
+- Fixed: Android `record start` no longer refuses to begin after a reused emulator reassigned the
+  previous recorder's pid. A completed recording's native marker is retired only once its recorder is
+  proven gone, but only an absent pid counted as proof — a pid that now names an unrelated process,
+  or a recorder that exited and waits to be reaped, did not. `record start` then failed every later
+  attempt with `Android screenrecord completed evidence cannot be safely retired`, and `record stop`
+  could not return an already-finalized recording. Proven termination now retires the marker and
+  returns the stored completion; a live or unreadable recorder still blocks, and the unrelated
+  process is never signalled. A reused pid that runs a replacement `screenrecord` on the same
+  remote path proves the old recorder gone but not that the path is free, so that marker and
+  artifact are retained until the replacement ends, and neither is signalled (#2476).
 - Fixed: a polling `wait` no longer surrenders its whole budget the first time the iOS runner
   answers `RUNNER_BUSY`. That code means an earlier command exceeded the runner's execution
   watchdog and its abandoned main-thread work is still draining, which clears on its own, so a

packages/contracts/src/screen-recording-runtime-host.test.ts

@@ -0,0 +1,28 @@
+import { expect, test } from 'vitest';
+import {
+  type AndroidScreenRecordingProcessOwnership,
+  provesAndroidScreenRecordPathUnclaimed,
+  provesAndroidScreenRecordTermination,
+} from './screen-recording-runtime-host.ts';
+
+type Proof = readonly [
+  ownership: AndroidScreenRecordingProcessOwnership,
+  termination: boolean,
+  pathUnclaimed: boolean,
+];
+
+const PROOFS: readonly Proof[] = [
+  ['missing', true, true],
+  ['ownership-lost', true, true],
+  ['foreign-writer', true, false],
+  ['owned-alive', false, false],
+  ['uncertain', false, false],
+];
+
+test.each(PROOFS)(
+  'observation %s proves termination as %s and an unclaimed path as %s',
+  (ownership, termination, pathUnclaimed) => {
+    expect(provesAndroidScreenRecordTermination(ownership)).toBe(termination);
+    expect(provesAndroidScreenRecordPathUnclaimed(ownership)).toBe(pathUnclaimed);
+  },
+);

packages/contracts/src/screen-recording-runtime-host.ts

@@ -94,12 +94,57 @@ export type AndroidScreenRecordingProcessIdentity = Readonly<{
   startTime: string;
 }>;
 
+/**
+ * `ownership-lost`: the pid is present, yet the identity readable there names something else — a
+ * reassigned pid, or an exited task whose command line is already gone. `foreign-writer`: the pid
+ * runs `screenrecord` on the recorded remote path but started at a different time, so a recorder
+ * that is not ours is writing that artifact. `uncertain` reads nothing conclusive.
+ */
 export type AndroidScreenRecordingProcessOwnership =
   | 'missing'
   | 'owned-alive'
   | 'ownership-lost'
+  | 'foreign-writer'
   | 'uncertain';
 
+/**
+ * Whether an observation proves that the process named by the inspected identity is gone. Both
+ * `ownership-lost` and `foreign-writer` are proof rather than doubt: the recorded process can no
+ * longer write its artifact.
+ */
+export function provesAndroidScreenRecordTermination(
+  ownership: AndroidScreenRecordingProcessOwnership,
+): boolean {
+  switch (ownership) {
+    case 'missing':
+    case 'ownership-lost':
+    case 'foreign-writer':
+      return true;
+    case 'owned-alive':
+    case 'uncertain':
+      return false;
+  }
+}
+
+/**
+ * Whether an observation proves that nothing writes the recorded remote path any more, which is
+ * what removing the artifact requires. A recorder proven gone is not proof of that: a
+ * `foreign-writer` has claimed the same path.
+ */
+export function provesAndroidScreenRecordPathUnclaimed(
+  ownership: AndroidScreenRecordingProcessOwnership,
+): boolean {
+  switch (ownership) {
+    case 'missing':
+    case 'ownership-lost':
+      return true;
+    case 'foreign-writer':
+    case 'owned-alive':
+    case 'uncertain':
+      return false;
+  }
+}
+
 export type AndroidScreenRecordingStopOutcome =
   | 'stopped'
   | 'already-missing'

packages/platform-android/src/recording/chunks.ts

@@ -1,5 +1,6 @@
 import path from 'node:path';
 import type { PlatformRuntimeHost } from '@agent-device/contracts/platform-runtime-operations';
+import { provesAndroidScreenRecordTermination } from '@agent-device/contracts/screen-recording-runtime-host';
 import type {
   ScreenRecordingChunk,
   ScreenRecordingStartInput,
@@ -235,7 +236,7 @@ async function waitForStopped(
   for (let elapsed = 0; elapsed <= GRACEFUL_STOP_TIMEOUT_MS; elapsed += STOP_POLL_INTERVAL_MS) {
     const state = await transport.inspect(processIdentity);
     if (state === 'missing') return true;
-    if (state === 'ownership-lost') {
+    if (provesAndroidScreenRecordTermination(state)) {
       throw new Error(
         `Android screenrecord ownership could not be confirmed for pid ${processIdentity.pid}`,
       );

packages/platform-android/src/recording/launch.ts

@@ -1,6 +1,7 @@
 import type { DeviceInfo } from '@agent-device/kernel/device';
 import type { PlatformRuntimeHost } from '@agent-device/contracts/platform-runtime-operations';
 import type { ScreenRecordingStartInput } from '@agent-device/contracts/screen-recording-runtime';
+import { provesAndroidScreenRecordPathUnclaimed } from '@agent-device/contracts/screen-recording-runtime-host';
 import {
   cleanupChunks,
   AndroidScreenRecordingStartRollbackUnconfirmed,
@@ -167,7 +168,11 @@ async function retireCompletedEvidence(
       remotePath: chunk.remotePath,
       startTime: chunk.remoteStartTime,
     });
-    if (state !== 'missing')
+    if (state === 'foreign-writer')
+      throw new Error(
+        'Android screenrecord completed evidence names an artifact another recorder is writing; it is retained until that recorder ends',
+      );
+    if (!provesAndroidScreenRecordPathUnclaimed(state))
       throw new Error('Android screenrecord completed evidence cannot be safely retired');
   }
   await cleanupChunks(transport, evidence.chunks);

packages/platform-android/src/recording/recovery.test.ts

@@ -72,6 +72,29 @@ test('reattaches an ended pid with an artifact as finishable recovery and report
     });
 });
 
+test('discloses truncation when recovery proved the recorder gone before record stop', async () => {
+  let manifest = '';
+  const observations = ['ownership-lost', 'missing'] as const;
+  let observation = 0;
+  const runtime = await start({
+    writeManifest: async ({ contents }: { contents: string }) => {
+      manifest = contents;
+    },
+    readManifest: async () =>
+      manifest ? { status: 'read' as const, contents: manifest } : { status: 'missing' as const },
+    inspect: async () => observations[Math.min(observation++, observations.length - 1)],
+    stop: async () => 'uncertain' as const,
+  });
+  const started = await runtime.screenRecordingStart(recordingInput());
+  const reattached = await runtime.screenRecordingReattach({ envelope: started.envelope });
+  expect(reattached.status).toBe('active');
+  if (reattached.status === 'active')
+    await expect(reattached.handle.finish()).resolves.toMatchObject({
+      status: 'completed',
+      result: { warning: expect.stringContaining('likely after reaching the 180s platform limit') },
+    });
+});
+
 test('returns fenced native completion after a crash between native finalization and daemon terminalization', async () => {
   let manifest = '';
   const removals: string[] = [];
@@ -135,6 +158,62 @@ test('retains completed evidence while an exact persisted recorder identity rema
   expect(JSON.parse(manifest)).toHaveProperty('completion');
 });
 
+test.each([
+  ['reassigned to an unrelated process', 'ownership-lost'],
+  ['reused by a replacement recorder on the same path', 'foreign-writer'],
+] as const)(
+  'terminalizes completed evidence whose recorder pid was %s, touching nothing',
+  async (_name, ownership) => {
+    let manifest = '';
+    const removals: string[] = [];
+    const runtime = await start({
+      writeManifest: async ({ contents }: { contents: string }) => {
+        manifest = contents;
+      },
+      readManifest: async () =>
+        manifest ? { status: 'read' as const, contents: manifest } : { status: 'missing' as const },
+      inspect: async () => ownership,
+      remove: async (remotePath: string) => {
+        removals.push(`artifact:${remotePath}`);
+        return true;
+      },
+      removeManifest: async (manifestPath: string) => {
+        removals.push(`marker:${manifestPath}`);
+        return true;
+      },
+    });
+    const started = await runtime.screenRecordingStart(recordingInput());
+    const native = JSON.parse(manifest);
+    manifest = JSON.stringify({
+      ...native,
+      completion: {
+        backend: 'adb screenrecord',
+        outPath: native.outputPath,
+        startedAt: native.startedAt,
+        completedAt: native.startedAt + 1,
+        scope: native.scope,
+        showTouches: native.showTouches,
+        recordOnlySession: native.recordOnlySession,
+      },
+    });
+
+    await expect(runtime.screenRecordingReattach({ envelope: started.envelope })).resolves.toEqual({
+      status: 'completed',
+      result: {
+        backend: 'adb screenrecord',
+        outPath: native.outputPath,
+        startedAt: native.startedAt,
+        completedAt: native.startedAt + 1,
+        scope: native.scope,
+        showTouches: native.showTouches,
+        recordOnlySession: native.recordOnlySession,
+      },
+    });
+    expect(removals).toEqual([]);
+    expect(JSON.parse(manifest)).toHaveProperty('completion');
+  },
+);
+
 test('makes matching pending evidence cleanup-eligible and stops discovered exact recorder pids', async () => {
   let manifest = '';
   const removed: string[] = [];

packages/platform-android/src/recording/recovery.ts

@@ -4,6 +4,7 @@ import type {
   ScreenRecordingRuntimeOperations,
   ScreenRecordingStartInput,
 } from '@agent-device/contracts/screen-recording-runtime';
+import { provesAndroidScreenRecordTermination } from '@agent-device/contracts/screen-recording-runtime-host';
 import { createScreenRecordingLiveHandle } from '@agent-device/capture-kit';
 import {
   androidScreenRecordingDescriptorCodec,
@@ -175,7 +176,7 @@ async function reattachEvidence(params: {
         evidence,
         manifestPath: descriptor.manifestPath,
         recording: current,
-        reachedLimit: running === 'missing',
+        reachedLimit: provesAndroidScreenRecordTermination(running),
       });
       nativeCleanupConfirmed = true;
       return outcome;
@@ -192,11 +193,13 @@ async function completedEvidenceIsTerminal(transport: Transport, evidence: Nativ
   try {
     for (const chunk of evidence.chunks) {
       if (
-        (await transport.inspect({
-          pid: chunk.remotePid,
-          remotePath: chunk.remotePath,
-          startTime: chunk.remoteStartTime,
-        })) !== 'missing'
+        !provesAndroidScreenRecordTermination(
+          await transport.inspect({
+            pid: chunk.remotePid,
+            remotePath: chunk.remotePath,
+            startTime: chunk.remoteStartTime,
+          }),
+        )
       )
         return false;
     }

packages/platform-android/src/recording/start-reconciliation.test.ts

@@ -56,6 +56,125 @@ test('reconciles coherent completed evidence before output preparation or launch
   await started.pendingHandle.transfer().forceCleanup();
 });
 
+test('retires completed evidence whose recorder pid was reassigned, signaling nothing', async () => {
+  let marker = JSON.stringify(completedEvidence());
+  const calls: string[] = [];
+  const runtime = await start({
+    readManifest: async (path: string) =>
+      path.startsWith('/sdcard') && marker
+        ? { status: 'read' as const, contents: marker }
+        : { status: 'missing' as const },
+    inspect: async () => 'ownership-lost' as const,
+    stop: async ({ pid }: { pid: string }) => {
+      calls.push(`signal:${pid}`);
+      return 'already-missing' as const;
+    },
+    remove: async (path: string) => {
+      calls.push(`artifact:${path}`);
+      return true;
+    },
+    removeManifest: async () => {
+      calls.push('manifest');
+      marker = '';
+      return true;
+    },
+    outputs: {
+      prepare: async () => {
+        calls.push('prepare');
+      },
+    },
+    start: async () => {
+      calls.push('launch');
+      return recordingProcess('77');
+    },
+  });
+  const started = await runtime.screenRecordingStart(newInput());
+  expect(calls).toEqual([
+    'artifact:/sdcard/agent-device-recording-1.mp4',
+    'manifest',
+    'prepare',
+    'launch',
+  ]);
+  await started.pendingHandle.transfer().forceCleanup();
+});
+
+test.each([
+  ['an unconfirmed recorder identity', 'uncertain'],
+  ['a live recorder', 'owned-alive'],
+])('refuses completed evidence named by %s', async (_name, ownership) => {
+  const marker = JSON.stringify(completedEvidence());
+  const calls: string[] = [];
+  const runtime = await start({
+    readManifest: async (path: string) =>
+      path.startsWith('/sdcard')
+        ? { status: 'read' as const, contents: marker }
+        : { status: 'missing' as const },
+    inspect: async () => ownership,
+    stop: async ({ pid }: { pid: string }) => {
+      calls.push(`signal:${pid}`);
+      return 'already-missing' as const;
+    },
+    remove: async () => {
+      calls.push('artifact');
+      return true;
+    },
+    removeManifest: async () => {
+      calls.push('manifest');
+      return true;
+    },
+    outputs: {
+      prepare: async () => {
+        calls.push('prepare');
+      },
+    },
+    start: async () => {
+      calls.push('launch');
+      return recordingProcess('77');
+    },
+  });
+  await expect(runtime.screenRecordingStart(newInput())).rejects.toThrow(
+    'cannot be safely retired',
+  );
+  expect(calls).toEqual([]);
+});
+
+test('retains completed evidence and its marker while a replacement recorder writes the same path', async () => {
+  const marker = JSON.stringify(completedEvidence());
+  const calls: string[] = [];
+  const runtime = await start({
+    readManifest: async (path: string) =>
+      path.startsWith('/sdcard')
+        ? { status: 'read' as const, contents: marker }
+        : { status: 'missing' as const },
+    inspect: async () => 'foreign-writer' as const,
+    stop: async ({ pid }: { pid: string }) => {
+      calls.push(`signal:${pid}`);
+      return 'already-missing' as const;
+    },
+    remove: async (path: string) => {
+      calls.push(`artifact:${path}`);
+      return true;
+    },
+    removeManifest: async () => {
+      calls.push('manifest');
+      return true;
+    },
+    outputs: {
+      prepare: async () => {
+        calls.push('prepare');
+      },
+    },
+    start: async () => {
+      calls.push('launch');
+      return recordingProcess('77');
+    },
+  });
+  await expect(runtime.screenRecordingStart(newInput())).rejects.toThrow(
+    'another recorder is writing',
+  );
+  expect(calls).toEqual([]);
+});
+
 test('retirement failure blocks launch and can succeed on a later retry', async () => {
   let marker = JSON.stringify(completedEvidence());
   let allowRemoval = false;