Skip to content

Commit a696834

Browse files
amankansal-ltamankansal-lt
authored
Merge pull request #2 from amankansal-lt/fix/testmu-review-followups
fix(provider-webdriver): address TestMu AI provider review
2 parents 2228167 + 417ff5b commit a696834

56 files changed

Lines changed: 1416 additions & 377 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎docs/adr/0007-remote-device-leases.md‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,16 @@ one minute, while a cloud WebDriver connection profile asks for ten. A single co
6767
longer than its own lease is therefore ordinary on the default and only reachable through a profile
6868
on the longer one.
6969

70+
## Provider allocation
71+
72+
Allocation is admitted work too. The registry records a lease before a hosted provider creates the
73+
session behind it, and creating that session can take longer than the lease's inactivity TTL, so a
74+
lease timed from its record was already expired, or nearly so, when its client first received it, and
75+
the paid session it pointed at was orphaned. The allocation therefore holds a work pass while the
76+
provider allocates: the lease cannot expire underneath it, and a successful allocation still wanted by
77+
its requester starts the inactivity TTL from the moment allocation completed. A requester that hung up
78+
preserves nothing, and its allocation is released as before.
79+
7080
## Client-side work that precedes admission
7181

7282
Protecting admitted work covers nothing that happens before a request is admitted. Installing an

‎packages/contracts/package.json‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -364,6 +364,10 @@
364364
"types": "./src/runtime-operation-names.ts",
365365
"default": "./src/runtime-operation-names.ts"
366366
},
367+
"./provider-profile-fields": {
368+
"types": "./src/provider-profile-fields.ts",
369+
"default": "./src/provider-profile-fields.ts"
370+
},
367371
"./progress": {
368372
"types": "./src/facades/progress.ts",
369373
"default": "./src/facades/progress.ts"

‎packages/contracts/src/app-deployment-runtime.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,11 @@ export type AppDeploymentSource =
1313
export type MaterializedAppSource = Readonly<{
1414
archivePath?: string;
1515
installablePath: string;
16+
/**
17+
* The single file a hosted provider uploads for this build, named by the materializer that knows
18+
* its format. Absent when the installable is already that file or no such file exists.
19+
*/
20+
uploadPath?: string;
1621
bundleId?: string;
1722
packageName?: string;
1823
appName?: string;

‎packages/contracts/src/provider-profile-fields.test.ts‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
import { test } from 'vitest';
2+
import assert from 'node:assert/strict';
3+
import { AppError } from '@agent-device/kernel/errors';
4+
import {
5+
rejectRefusedProviderProfileFields,
6+
type ProviderProfileFieldDeclaration,
7+
} from './provider-profile-fields.ts';
8+
9+
const DECLARATION: ProviderProfileFieldDeclaration = {
10+
provider: 'fake',
11+
label: 'Fake Cloud',
12+
fields: {
13+
providerApp: 'consumed',
14+
providerOsVersion: 'consumed',
15+
providerDeviceType: 'refused',
16+
providerProject: 'consumed',
17+
providerBuild: 'consumed',
18+
providerSessionName: 'consumed',
19+
providerDeviceOrientation: 'consumed',
20+
providerGeoLocation: 'refused',
21+
providerTimezone: 'consumed',
22+
providerAppiumVersion: 'consumed',
23+
providerLanguage: 'consumed',
24+
providerLocale: 'consumed',
25+
providerNetworkProfile: 'consumed',
26+
providerCustomNetwork: 'consumed',
27+
providerNoResignApp: 'refused',
28+
awsProjectArn: 'consumed',
29+
awsDeviceArn: 'consumed',
30+
awsAppArn: 'consumed',
31+
awsRegion: 'consumed',
32+
awsInteractionMode: 'consumed',
33+
},
34+
};
35+
36+
test('consumed, unset, empty, and false fields pass', () => {
37+
assert.doesNotThrow(() => rejectRefusedProviderProfileFields(undefined, DECLARATION));
38+
assert.doesNotThrow(() =>
39+
rejectRefusedProviderProfileFields(
40+
{
41+
providerApp: 'app',
42+
providerDeviceType: '',
43+
providerGeoLocation: undefined,
44+
providerNoResignApp: false,
45+
unrelated: 'x',
46+
},
47+
DECLARATION,
48+
),
49+
);
50+
});
51+
52+
test('a refused field fails with its flag and the provider named', () => {
53+
assert.throws(
54+
() => rejectRefusedProviderProfileFields({ providerDeviceType: 'real' }, DECLARATION),
55+
(error: unknown) =>
56+
error instanceof AppError &&
57+
error.code === 'INVALID_ARGS' &&
58+
error.message === '--provider-device-type is not supported by Fake Cloud.' &&
59+
error.details?.provider === 'fake' &&
60+
JSON.stringify(error.details?.flags) === '["--provider-device-type"]',
61+
);
62+
});
63+
64+
test('every refused field is reported at once', () => {
65+
assert.throws(
66+
() =>
67+
rejectRefusedProviderProfileFields(
68+
{ providerGeoLocation: 'US', providerNoResignApp: true, providerDeviceType: 'virtual' },
69+
DECLARATION,
70+
),
71+
/--provider-device-type, --provider-geo-location, --provider-no-resign-app are not supported by Fake Cloud\./,
72+
);
73+
});

‎packages/contracts/src/provider-profile-fields.ts‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
import { AppError } from '@agent-device/kernel/errors';
2+
import type { CloudProviderProfileFields } from './remote-config-fields.ts';
3+
4+
export type ProviderProfileField = keyof CloudProviderProfileFields;
5+
6+
/**
7+
* What one lease provider does with every Cloud provider profile field. The record is total, so a
8+
* field added to the profile fails to build until each provider says whether it consumes it; a
9+
* field a provider neither reads nor refuses would otherwise ride the profile and be dropped.
10+
*/
11+
export type ProviderProfileFieldDeclaration = Readonly<{
12+
provider: string;
13+
label: string;
14+
fields: Readonly<Record<ProviderProfileField, 'consumed' | 'refused'>>;
15+
}>;
16+
17+
const PROVIDER_PROFILE_FIELD_FLAGS: Readonly<Record<ProviderProfileField, string>> = {
18+
providerApp: '--provider-app',
19+
providerOsVersion: '--provider-os-version',
20+
providerDeviceType: '--provider-device-type',
21+
providerProject: '--provider-project',
22+
providerBuild: '--provider-build',
23+
providerSessionName: '--provider-session-name',
24+
providerDeviceOrientation: '--provider-device-orientation',
25+
providerGeoLocation: '--provider-geo-location',
26+
providerTimezone: '--provider-timezone',
27+
providerAppiumVersion: '--provider-appium-version',
28+
providerLanguage: '--provider-language',
29+
providerLocale: '--provider-locale',
30+
providerNetworkProfile: '--provider-network-profile',
31+
providerCustomNetwork: '--provider-custom-network',
32+
providerNoResignApp: '--provider-no-resign-app',
33+
awsProjectArn: '--aws-project-arn',
34+
awsDeviceArn: '--aws-device-arn',
35+
awsAppArn: '--aws-app-arn',
36+
awsRegion: '--aws-region',
37+
awsInteractionMode: '--aws-interaction-mode',
38+
};
39+
40+
/**
41+
* Fails when `flags` set a profile field the provider refuses. Every route to a provider — connect,
42+
* `leases.allocate`, a hand-authored remote-config profile — runs this against the same declaration.
43+
*/
44+
export function rejectRefusedProviderProfileFields(
45+
flags: Readonly<Record<string, unknown>> | undefined,
46+
declaration: ProviderProfileFieldDeclaration,
47+
): void {
48+
const refused = (Object.keys(declaration.fields) as ProviderProfileField[])
49+
.filter((field) => declaration.fields[field] === 'refused' && isSet(flags?.[field]))
50+
.map((field) => PROVIDER_PROFILE_FIELD_FLAGS[field]);
51+
if (refused.length === 0) return;
52+
const plural = refused.length !== 1;
53+
throw new AppError(
54+
'INVALID_ARGS',
55+
`${refused.join(', ')} ${plural ? 'are' : 'is'} not supported by ${declaration.label}.`,
56+
{
57+
hint: `Drop ${plural ? 'those flags' : 'the flag'} or use a provider that supports ${plural ? 'them' : 'it'}.`,
58+
provider: declaration.provider,
59+
flags: refused,
60+
},
61+
);
62+
}
63+
64+
function isSet(value: unknown): boolean {
65+
return value !== undefined && value !== null && value !== false && value !== '';
66+
}

‎packages/platform-apple/src/core/install-artifact.ts‎

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,11 @@ import path from 'node:path';
22
import type { LocalInstallSource } from '@agent-device/kernel/contracts';
33
import { readIosBundleInfo } from './bundle-info.ts';
44
import { AppError } from '@agent-device/kernel/errors';
5-
import { extractArchiveSafely, ArchiveBudget } from '@agent-device/host-kit/archive';
5+
import {
6+
archiveTypeFromPath,
7+
extractArchiveSafely,
8+
ArchiveBudget,
9+
} from '@agent-device/host-kit/archive';
610

711
import {
812
installArtifactArchiveBudget,
@@ -35,6 +39,7 @@ type IosPayloadAppBundle = {
3539
export type PreparedIosInstallArtifact = {
3640
archivePath?: string;
3741
installablePath: string;
42+
uploadPath?: string;
3843
bundleId?: string;
3944
appName?: string;
4045
cleanup: () => Promise<void>;
@@ -80,9 +85,11 @@ async function prepareIosInstallArtifactInScope(
8085
? materialized.installablePath
8186
: undefined);
8287

88+
const uploadPath = iosUploadPath(materialized);
8389
return {
8490
archivePath,
8591
installablePath: resolvedInstallable.installPath,
92+
...(uploadPath ? { uploadPath } : {}),
8693
bundleId: bundleInfo.bundleId,
8794
appName: bundleInfo.appName,
8895
cleanup: async () => {
@@ -102,6 +109,24 @@ async function prepareIosInstallArtifactInScope(
102109

103110
export { readIosBundleInfo } from './bundle-info.ts';
104111

112+
/**
113+
* The installable is an extracted `.app` directory, which no hosted upload API accepts. The file
114+
* that carries it is the `.ipa` it was unpacked from, or the zip the `.app` was extracted from
115+
* directly; an outer archive that merely wrapped either is never it.
116+
*/
117+
function iosUploadPath(materialized: {
118+
containingArchivePath?: string;
119+
installablePath: string;
120+
}): string | undefined {
121+
if (materialized.installablePath.toLowerCase().endsWith('.ipa')) {
122+
return materialized.installablePath;
123+
}
124+
const { containingArchivePath } = materialized;
125+
return containingArchivePath && archiveTypeFromPath(containingArchivePath) === 'zip'
126+
? containingArchivePath
127+
: undefined;
128+
}
129+
105130
async function resolveIosInstallablePath(
106131
appPath: string,
107132
options?: InstallIosArtifactOptions,

‎packages/provider-limrun/src/device.ts‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,39 @@
11
import type { DeviceLease } from '@agent-device/contracts/device';
22
import type { DeviceInfo } from '@agent-device/kernel/device';
3+
import type { ProviderProfileFieldDeclaration } from '@agent-device/contracts/provider-profile-fields';
34

45
export type LimrunPlatform = 'ios' | 'android';
56

67
export const LIMRUN_PROVIDER = 'limrun';
78

9+
/** Limrun reads only the app to preinstall; it picks the instance itself. */
10+
export const LIMRUN_PROFILE_FIELDS: ProviderProfileFieldDeclaration = {
11+
provider: LIMRUN_PROVIDER,
12+
label: 'Limrun',
13+
fields: {
14+
providerApp: 'consumed',
15+
providerOsVersion: 'refused',
16+
providerDeviceType: 'refused',
17+
providerProject: 'refused',
18+
providerBuild: 'refused',
19+
providerSessionName: 'refused',
20+
providerDeviceOrientation: 'refused',
21+
providerGeoLocation: 'refused',
22+
providerTimezone: 'refused',
23+
providerAppiumVersion: 'refused',
24+
providerLanguage: 'refused',
25+
providerLocale: 'refused',
26+
providerNetworkProfile: 'refused',
27+
providerCustomNetwork: 'refused',
28+
providerNoResignApp: 'refused',
29+
awsProjectArn: 'refused',
30+
awsDeviceArn: 'refused',
31+
awsAppArn: 'refused',
32+
awsRegion: 'refused',
33+
awsInteractionMode: 'refused',
34+
},
35+
};
36+
837
const LIMRUN_DEVICE_ID_PREFIX = LIMRUN_PROVIDER;
938

1039
export function platformForLimrunLeaseBackend(backend: string): LimrunPlatform | undefined {

‎packages/provider-limrun/src/index.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
export { LIMRUN_PROVIDER } from './device.ts';
1+
export { LIMRUN_PROFILE_FIELDS, LIMRUN_PROVIDER } from './device.ts';
22
export { createLimrunRuntime, type LimrunRuntime, type LimrunRuntimeOptions } from './runtime.ts';
33
export { verifyLimrunConnection } from './connection-verification.ts';
44

‎packages/provider-limrun/src/runtime.ts‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -231,15 +231,17 @@ class LimrunRuntimeImplementation implements ProviderDeviceRuntime {
231231
if (lease.leaseProvider !== this.provider) return undefined;
232232
const platform = platformForLimrunLeaseBackend(lease.backend);
233233
if (!platform) return undefined;
234-
const existing = this.sessions.get(lease.leaseId);
235-
if (existing) return { limrunInstanceId: existing.instanceId, device: existing.device };
236-
237234
const {
238235
allocateLimrunAndroidSession,
239236
allocateLimrunIosSession,
237+
rejectRefusedLimrunProfileFields,
240238
resolvePreinstalledAppId,
241239
resolveRequestedLimrunAppAsset,
242240
} = await import('./session-allocation.ts');
241+
// Before the reuse below: a repeat allocation of a live lease carries flags of its own.
242+
rejectRefusedLimrunProfileFields(context);
243+
const existing = this.sessions.get(lease.leaseId);
244+
if (existing) return { limrunInstanceId: existing.instanceId, device: existing.device };
243245
const requestedAsset = await resolveRequestedLimrunAppAsset(this.limrun, platform, context);
244246
const session =
245247
platform === 'ios'

‎packages/provider-limrun/src/session-allocation.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
import type Limrun from '@limrun/api';
22
import type { DeviceLease, LeaseLifecycleContext } from '@agent-device/contracts/device';
33
import { AppError } from '@agent-device/kernel/errors';
4+
import { rejectRefusedProviderProfileFields } from '@agent-device/contracts/provider-profile-fields';
45
import { createLimrunAndroidSession, type LimrunAndroidSession } from './android.ts';
5-
import { buildLimrunDevice } from './device.ts';
6+
import { buildLimrunDevice, LIMRUN_PROFILE_FIELDS } from './device.ts';
67
import { createLimrunIosSession, type LimrunIosSession } from './ios.ts';
78
import {
89
assertLimrunUploadedAppAccess,
@@ -34,6 +35,10 @@ type SessionAllocationParams = Readonly<{
3435
dependencies: LimrunRuntimeDependencies;
3536
}>;
3637

38+
export function rejectRefusedLimrunProfileFields(context?: LeaseLifecycleContext): void {
39+
rejectRefusedProviderProfileFields(context?.flags, LIMRUN_PROFILE_FIELDS);
40+
}
41+
3742
export async function resolveRequestedLimrunAppAsset(
3843
limrun: Limrun,
3944
platform: 'android' | 'ios',

0 commit comments

Comments
 (0)