Repository navigation
fix: support foldable simulators lent by an external device provider #1490
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,7 +7,7 @@ import { | |
| ADB_DEVICES_TIMEOUT_MS, | ||
| } from "../../utils/adb"; | ||
| import { listRunningVvdConsolePorts } from "../../utils/vega-process"; | ||
| import { listIosSimulators, type IosSimulator } from "../../utils/ios-devices"; | ||
| import { isFoldableSimulator, listIosSimulators, type IosSimulator } from "../../utils/ios-devices"; | ||
| import { listIosPhysicalDevices } from "../../utils/ios-device/devicectl"; | ||
| import { simctlListDevices } from "../../utils/sim-remote"; | ||
| import { withRemotePrefix } from "../../utils/device-info"; | ||
|
|
@@ -72,6 +72,11 @@ type AndroidDevice = { | |
| type ExternalListedDevice = { | ||
| capabilities: string[]; | ||
| external: true; | ||
| /** | ||
| * Present and true for a foldable simulator (the iPhone Duo), as on a plain | ||
| * iOS row. | ||
| */ | ||
| foldable?: true; | ||
| id: string; | ||
| kind: "device" | "emulator" | "simulator"; | ||
| name: string; | ||
|
|
@@ -233,10 +238,11 @@ export async function withDeadline<T>(p: Promise<T>, fallback: T, label: string) | |
| } | ||
| } | ||
|
|
||
| function toListedExternal(device: ExternalDevice): ExternalListedDevice { | ||
| function toListedExternal(device: ExternalDevice, foldable: boolean): ExternalListedDevice { | ||
| return { | ||
| capabilities: Array.from(device.capabilities).sort(), | ||
| external: true, | ||
| ...(foldable ? { foldable: true as const } : {}), | ||
| id: device.id, | ||
| kind: device.kind, | ||
| name: device.name, | ||
|
|
@@ -252,6 +258,30 @@ function toListedExternal(device: ExternalDevice): ExternalListedDevice { | |
| }; | ||
| } | ||
|
|
||
| type ExternalEntry = { device: ExternalDevice; foldable: boolean }; | ||
|
|
||
| /** | ||
| * Pair each external device with whether it is a foldable simulator (the iPhone | ||
| * Duo), read from the provider's device set. An agent needs the flag to know | ||
| * `fold` applies, whoever drives the device. This runs inside the external | ||
| * discovery branch, so it overlaps the other branches instead of adding to | ||
| * them; the branch's worst case (a provider probe, then one `simctl` listing | ||
| * and one device-type read) stays under `BRANCH_DEADLINE_MS`. | ||
|
Comment on lines
+268
to
+269
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Diplomat, Opus 5.5]: The worst case is longer than this says. After the probe (800 ms), the chain can run I reproduced it through the HTTP entry point (
An in-process run gave the same result: a 6 s |
||
| * `isFoldableSimulator` memoizes every verdict the device set answers, so a | ||
| * simulator the set lists costs one `simctl` call per tool-server run. | ||
| */ | ||
| async function withFoldableFlags(external: readonly ExternalDevice[]): Promise<ExternalEntry[]> { | ||
| return Promise.all( | ||
| external.map(async (device) => ({ | ||
| device, | ||
| foldable: | ||
| device.platform === "ios" && | ||
| device.kind === "simulator" && | ||
| (await isFoldableSimulator(device.id).catch(() => false)), | ||
| })) | ||
| ); | ||
| } | ||
|
|
||
| /** | ||
| * Native ids a provider is currently claiming. | ||
| * | ||
|
|
@@ -332,16 +362,19 @@ Booted/ready devices are listed first. Platforms whose CLI is unavailable are si | |
| ), | ||
| /** | ||
| * One directory stat when no provider is registered (the common case) | ||
| * and a short HTTP probe per provider otherwise. | ||
| * and a short HTTP probe per provider otherwise, then the foldable | ||
| * check of each lent simulator. | ||
| */ | ||
| withDeadline( | ||
| listExternalDevices().catch(() => []), | ||
| listExternalDevices() | ||
| .catch(() => []) | ||
| .then(withFoldableFlags), | ||
| [], | ||
| "external" | ||
| ), | ||
| ]); | ||
|
|
||
| const externalShadows = externalShadowIds(external); | ||
| const externalShadows = externalShadowIds(external.map(({ device }) => device)); | ||
|
|
||
| const iosTagged: IosDevice[] = ios | ||
| .filter((simulator) => !externalShadows.has(simulator.udid)) | ||
|
|
@@ -389,7 +422,7 @@ Booted/ready devices are listed first. Platforms whose CLI is unavailable are si | |
| ...androidDeduped, | ||
| ...chromium, | ||
| ...vega, | ||
| ...external.map(toListedExternal), | ||
| ...external.map(({ device, foldable }) => toListedExternal(device, foldable)), | ||
| ]; | ||
| devices.sort((a, b) => readinessRank(a) - readinessRank(b)); | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| import { createHash } from "node:crypto"; | ||
|
|
||
| /** | ||
| * The per-device part of a `/tmp/<prefix>-<tag>.sock` path. | ||
| * | ||
| * A plain UDID gives its first 8 characters. Any other id, such as an external | ||
| * provider's `ext:<provider>:<udid>`, gives 12 hex characters of its SHA-256: | ||
| * the first 8 characters of such an id are `ext:` plus the first 4 of the | ||
| * provider id, the same for every device of that provider and of any provider | ||
| * whose id starts the same way. The hash keeps the path short (macOS | ||
| * caps `sun_path` at 104 bytes) and can never equal an 8-character UDID tag. | ||
| */ | ||
| export function socketTag(deviceId: string): string { | ||
| if (/^[A-Za-z0-9-]+$/.test(deviceId)) return deviceId.slice(0, 8); | ||
| return createHash("sha256").update(deviceId).digest("hex").slice(0, 12); | ||
| } | ||
|
|
||
| /** The Unix socket the ax-service daemon of a device listens on. */ | ||
| export function axServiceSocketPath(deviceId: string): string { | ||
| return `/tmp/ax-${socketTag(deviceId)}.sock`; | ||
| } | ||
|
|
||
| /** | ||
| * The Unix socket the native-devtools library of a device connects to. At most | ||
| * 32 characters, well under the 104-byte macOS limit. | ||
| */ | ||
| export function nativeDevtoolsSocketPath(deviceId: string): string { | ||
| return `/tmp/argent-nd-${socketTag(deviceId)}.sock`; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -97,13 +97,29 @@ vi.mock("../src/utils/ios-device/devicectl", async () => { | |
| }; | ||
| }); | ||
|
|
||
| // Whether a provider's simulator is foldable is read from its device set; that | ||
| // read is the seam, off by default. The plain listing is wrapped so a test can | ||
| // hold it open. | ||
| vi.mock("../src/utils/ios-devices", async () => { | ||
| const actual = await vi.importActual<typeof import("../src/utils/ios-devices")>( | ||
| "../src/utils/ios-devices" | ||
| ); | ||
|
|
||
| return { | ||
| ...actual, | ||
| isFoldableSimulator: vi.fn(async () => false), | ||
| listIosSimulators: vi.fn(actual.listIosSimulators), | ||
| }; | ||
| }); | ||
|
|
||
| import * as fs from "node:fs"; | ||
| import * as http from "node:http"; | ||
| import * as os from "node:os"; | ||
| import * as path from "node:path"; | ||
| import type { AddressInfo } from "node:net"; | ||
| import { listDevicesTool } from "../src/tools/devices/list-devices"; | ||
| import { listIosPhysicalDevices } from "../src/utils/ios-device/devicectl"; | ||
| import { isFoldableSimulator, listIosSimulators } from "../src/utils/ios-devices"; | ||
| import { | ||
| __resetExternalDeviceCacheForTesting, | ||
| __resetProviderWarningsForTesting, | ||
|
|
@@ -664,6 +680,7 @@ describe("list-devices — external provider shadows", () => { | |
| process.env.ARGENT_DEVICE_PROVIDERS = descriptorPath; | ||
| __resetExternalDeviceCacheForTesting(); | ||
| __resetProviderWarningsForTesting(); | ||
| vi.mocked(isFoldableSimulator).mockClear(); | ||
| }); | ||
|
|
||
| afterEach(async () => { | ||
|
|
@@ -703,6 +720,93 @@ describe("list-devices — external provider shadows", () => { | |
| }); | ||
| }); | ||
|
|
||
| it("marks a claimed foldable simulator with foldable: true", async () => { | ||
| /** | ||
| * The provider's device set is not configured, so the plain listing does | ||
| * not see the device; the verdict is read from the provider's set. | ||
| */ | ||
| execFileMock.mockImplementation((cmd: string, args: string[]) => { | ||
| if (cmd === "adb" && args[0] === "devices") { | ||
| return { stderr: "", stdout: "List of devices attached\n" }; | ||
| } | ||
|
|
||
| return { stderr: "", stdout: "" }; | ||
| }); | ||
|
|
||
| vi.mocked(isFoldableSimulator).mockResolvedValueOnce(true); | ||
|
|
||
| const result = await listDevicesTool.execute!({}, {}); | ||
|
|
||
| const claimed = result.devices.find( | ||
| (device) => "udid" in device && device.udid === `ext:acme-3f2a9c:${CLAIMED_UDID}` | ||
| ); | ||
|
|
||
| expect(claimed).toMatchObject({ external: true, foldable: true }); | ||
| expect(isFoldableSimulator).toHaveBeenCalledWith(`ext:acme-3f2a9c:${CLAIMED_UDID}`); | ||
| }); | ||
|
|
||
| it("leaves foldable off a claimed simulator that is not foldable", async () => { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Diplomat, Opus 5.5]: This test does not tell main and this PR apart. With origin/main's The PR description names the shadowed simctl row as the first source of the flag, but no test covers a foldable shadowed row. I replaced
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Removed the shadowed-row shortcut, so every lent simulator now goes through the same check. The non-foldable test now asserts that the check ran and a new test covers the timing issue from the other comment. |
||
| execFileMock.mockImplementation((cmd: string, args: string[]) => { | ||
| if (cmd === "xcrun" && args[0] === "simctl") { | ||
| return { stderr: "", stdout: simctlJson() }; | ||
| } | ||
|
|
||
| if (cmd === "adb" && args[0] === "devices") { | ||
| return { stderr: "", stdout: "List of devices attached\n" }; | ||
| } | ||
|
|
||
| return { stderr: "", stdout: "" }; | ||
| }); | ||
|
|
||
| const result = await listDevicesTool.execute!({}, {}); | ||
|
|
||
| const claimed = result.devices.find( | ||
| (device) => "udid" in device && device.udid === `ext:acme-3f2a9c:${CLAIMED_UDID}` | ||
| ); | ||
|
|
||
| expect(claimed).toBeDefined(); | ||
| expect(claimed).not.toHaveProperty("foldable"); | ||
| expect(isFoldableSimulator).toHaveBeenCalledWith(`ext:acme-3f2a9c:${CLAIMED_UDID}`); | ||
| }); | ||
|
|
||
| it("checks foldables while the plain simulator listing is still running", async () => { | ||
| /** | ||
| * A wedged CoreSimulator holds every `simctl` call until its timeout. The | ||
| * foldable check must overlap the plain listing rather than wait for it or | ||
| * the two timeouts add up. Here the listing settles only once the check has | ||
| * run, so a check that waited for it would hang this test. | ||
| */ | ||
| let releaseListing: (() => void) | undefined; | ||
|
|
||
| vi.mocked(listIosSimulators).mockImplementationOnce( | ||
| () => | ||
| new Promise((resolve) => { | ||
| releaseListing = () => resolve([]); | ||
| }) | ||
| ); | ||
|
|
||
| vi.mocked(isFoldableSimulator).mockImplementationOnce(async () => { | ||
| releaseListing?.(); | ||
| return true; | ||
| }); | ||
|
|
||
| execFileMock.mockImplementation((cmd: string, args: string[]) => { | ||
| if (cmd === "adb" && args[0] === "devices") { | ||
| return { stderr: "", stdout: "List of devices attached\n" }; | ||
| } | ||
|
|
||
| return { stderr: "", stdout: "" }; | ||
| }); | ||
|
|
||
| const result = await listDevicesTool.execute!({}, {}); | ||
|
|
||
| const claimed = result.devices.find( | ||
| (device) => "udid" in device && device.udid === `ext:acme-3f2a9c:${CLAIMED_UDID}` | ||
| ); | ||
|
|
||
| expect(claimed).toMatchObject({ external: true, foldable: true }); | ||
| }); | ||
|
|
||
| it("leaves an unclaimed simulator in the plain listing", async () => { | ||
| execFileMock.mockImplementation((cmd: string, args: string[]) => { | ||
| if (cmd === "xcrun" && args[0] === "simctl") { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Diplomat, Opus 5.5]: An external row can now carry
foldable: true, but the "What the agent sees" section ofpackages/docs/docs/reference/device-providers.mdxstill lists onlyexternal,providerandcapabilitiesas the extra fields of an external entry.reference/tools.mdxandfundamentals/supported-platforms.mdxdescribe the flag only for plain simulators. The repoCLAUDE.mdasks for the docs update in the same PR, or a line in the description saying none is needed. This PR has neither.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added
foldableto the "What the agent sees" table.