Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/cli-schema/cli-help-topics.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ test('usageForCommand resolves workflow help topic', async () => {
assert.match(help, /Shapes and platform quirks: help gestures/);
assert.match(
help,
/open --foreground -> snapshot\. Selection: explicit --device\/--udid\/--serial, then session, booted\/bootable local, or one provider; --platform\/--target only filter\./,
/open --foreground -> snapshot\. Selection: explicit --device\/--udid\/--serial, then session, booted\/bootable local, or provider; --platform\/--target only filter\./,
);
assert.match(help, /Never open artifact paths or invent package ids/);
assert.match(
Expand Down
2 changes: 1 addition & 1 deletion src/cli-schema/cli-help.ts
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ Command shape:
Gestures: scroll/swipe for lists/flicks; gesture pan|fling|pinch|rotate|transform|drag for multi-touch. Shapes and platform quirks: help gestures.

Bootstrap:
open --foreground -> snapshot. Selection: explicit --device/--udid/--serial, then session, booted/bootable local, or one provider; --platform/--target only filter. Ambiguous/empty fails with bounded retry selectors; providers never fall back.
open --foreground -> snapshot. Selection: explicit --device/--udid/--serial, then session, booted/bootable local, or provider; --platform/--target only filter. Ambiguous/empty fails with bounded retry selectors; no provider fallback.
Install arguments are app/package id then artifact path: agent-device install com.example.app ./dist/app.apk --platform android, then open <id> --relaunch for fresh state. Use reinstall only when explicitly requested.
Unknown app id: devices, then apps, then open <discovered-app-id>. Never open artifact paths or invent package ids; ask if lookup misses the target.
Apple CI: prepare ios-runner after boot/install, before replay/test (help prepare). Remote/cloud: connect -> open -> commands -> close -> disconnect (help remote). Reusable scripts, secret-safe fills, replay repair: help scripting.
Expand Down
9 changes: 9 additions & 0 deletions src/core/command-descriptor/registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,14 @@ const isShardedTestRequest = (req: DispatchedCommand): boolean =>
req.command === 'test' &&
(typeof req.flags?.shardAll === 'number' || typeof req.flags?.shardSplit === 'number');

// #2016: a plain `close` (no app-target positional) has nothing to close via
// flags, so it's the only close shape eligible for the sessionless
// no-lease-anywhere admission bypass in request-admission.ts. `close <app>`
// resolves its device straight from flags when there's no session and must
// stay behind full lease/tenant admission.
const isPlainCloseRequest = (req: DispatchedCommand): boolean =>
(req.positionals?.length ?? 0) === 0;

// ADR 0014 request-sensitive ref-frame resolvers. The action is the leading
// positional (see keyboard/alert daemon writers in src/commands/system/index.ts
// and src/commands/capture/alert.ts). Only the read-only status probes preserve
Expand Down Expand Up @@ -922,6 +930,7 @@ export const RAW_COMMAND_DESCRIPTORS = [
refFrameEffect: 'may-invalidate',
allowInvalidRecording: true,
saveScriptFlagOwner: true,
sessionlessPlainCloseAdmissionExempt: isPlainCloseRequest,
},
timeoutPolicy: DEFAULT_TIMEOUT_POLICY,
batchable: true,
Expand Down
149 changes: 149 additions & 0 deletions src/daemon/__tests__/request-admission.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
import { test } from 'vitest';
import assert from 'node:assert/strict';
import { makeIosSession } from '../../__tests__/test-utils/session-factories.ts';
import { LeaseRegistry } from '../lease-registry.ts';
import { assertRequestLeaseAdmission } from '../request-admission.ts';
import type { DaemonRequest } from '../types.ts';

function makeRequest(overrides: Partial<DaemonRequest> = {}): DaemonRequest {
return {
token: 'token',
session: 'default',
command: 'close',
positionals: [],
flags: {},
...overrides,
};
}

// #2016: a tenant-isolated connection (e.g. BrowserStack) whose lease was
// never allocated (`open` was never called) must not surface the generic
// tenant/run/lease admission error when `close` tries to clean it up.
test('close on a tenant-isolated session with no lease ever allocated admits without throwing', () => {
const registry = new LeaseRegistry();

const result = assertRequestLeaseAdmission(
makeRequest({
command: 'close',
meta: { tenantId: 'tenant-a', runId: 'run-1', sessionIsolation: 'tenant' },
}),
registry,
undefined,
);

assert.equal(result, undefined);
});

test('close on a lease-less but stored tenant-isolated session still requires a lease id', () => {
// #2016 follow-up: a *stored* session under tenant isolation is keyed by
// tenant, not by run, so a lease-less stored session could belong to
// another run in the same tenant. The bypass must not treat "this session
// record has no lease field" as proof there's nothing to protect — only
// "no session record exists at all" (the actual deferred-connect case)
// qualifies.
const registry = new LeaseRegistry();
const session = makeIosSession('default');

assert.throws(
() =>
assertRequestLeaseAdmission(
makeRequest({
command: 'close',
meta: { tenantId: 'tenant-a', runId: 'run-1', sessionIsolation: 'tenant' },
}),
registry,
session,
),
/tenant isolation requires lease id/,
);
});

test('close with an app target still requires a lease id even with no lease anywhere', () => {
// #2016 follow-up: `close <app>` with no session resolves its device
// straight from flags (`closeWithoutSession`), so it must not bypass
// tenant/lease admission the way a plain `close` (nothing to close) does —
// otherwise any caller could close an arbitrary flag-selected device on a
// tenant-isolated fleet without ever presenting a lease.
const registry = new LeaseRegistry();

assert.throws(
() =>
assertRequestLeaseAdmission(
makeRequest({
command: 'close',
positionals: ['com.example.app'],
meta: { tenantId: 'tenant-a', runId: 'run-1', sessionIsolation: 'tenant' },
}),
registry,
undefined,
),
/tenant isolation requires lease id/,
);
});

test('close with an explicit lease id but no session lease still requires an active lease', () => {
const registry = new LeaseRegistry();

assert.throws(
() =>
assertRequestLeaseAdmission(
makeRequest({
command: 'close',
meta: {
tenantId: 'tenant-a',
runId: 'run-1',
leaseId: 'a'.repeat(32),
sessionIsolation: 'tenant',
},
}),
registry,
undefined,
),
/Lease is not active/,
);
});

test('non-close commands on a tenant-isolated session still require a lease id', () => {
const registry = new LeaseRegistry();

assert.throws(
() =>
assertRequestLeaseAdmission(
makeRequest({
command: 'snapshot',
meta: { tenantId: 'tenant-a', runId: 'run-1', sessionIsolation: 'tenant' },
}),
registry,
undefined,
),
/tenant isolation requires lease id/,
);
});

test('close still admits and heartbeats a real active lease', () => {
let now = 1_000;
const registry = new LeaseRegistry({ now: () => now });
const lease = registry.allocateLease({ tenantId: 'tenant-a', runId: 'run-1' });
const session = makeIosSession('default', {
lease: {
leaseId: lease.leaseId,
tenantId: lease.tenantId,
runId: lease.runId,
leaseBackend: lease.backend,
expiresAt: lease.expiresAt,
},
});
now = 2_000;

const result = assertRequestLeaseAdmission(
makeRequest({
command: 'close',
meta: { tenantId: 'tenant-a', runId: 'run-1', sessionIsolation: 'tenant' },
}),
registry,
session,
);

assert.equal(result?.leaseId, lease.leaseId);
assert.equal(result?.heartbeatAt, 2_000);
});
92 changes: 91 additions & 1 deletion src/daemon/__tests__/request-execution-scope.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { afterAll, test, expect } from 'vitest';
import { afterAll, test, expect, vi } from 'vitest';
import fs from 'node:fs';
import path from 'node:path';
import {
Expand All @@ -23,6 +23,7 @@ import { resolveSessionRequestLogPath } from '../session-store.ts';
import type { DaemonRequest } from '../types.ts';
import { mkdtempForTestSync } from '../../__tests__/test-utils/tmp-dir.ts';
import { makeTestScreenRecordingResource } from '../../__tests__/test-utils/screen-recording-live-handle.ts';
import { handleCloseCommand } from '../handlers/session-close.ts';

const TEST_ROOT = mkdtempForTestSync('agent-device-request-execution-scope-');
const LOG_PATH = path.join(TEST_ROOT, 'diagnostics.log');
Expand Down Expand Up @@ -754,6 +755,95 @@ test('runLocked rejects a request canceled while waiting for its execution lock'
}
});

// #2016 router-level regression: a deferred remote connection (`connect`
// succeeded, `open` never ran, so the daemon never allocated a lease or
// created a session) reaches `close`'s own SESSION_NOT_FOUND outcome
// through the real tenant-scoping + locked-admission pipeline, rather than
// throwing the generic tenant-isolation error before `handleCloseCommand`
// ever runs. No provider is touched — there is nothing to release.
test('router: deferred tenant connect with no daemon session closes as SESSION_NOT_FOUND without touching the provider', async () => {
const sessionStore = makeSessionStore('agent-device-request-scope-');
const leaseRegistry = new LeaseRegistry();
const release = vi.fn(async () => ({}));

const scope = await createRequestExecutionScope({
req: makeRequest({
session: 'default',
command: 'close',
meta: { tenantId: 'tenant-a', runId: 'run-1', sessionIsolation: 'tenant' },
}),
sessionStore,
leaseRegistry,
});
expect(scope.sessionName).toBe('tenant-a:default');

const response = await scope.runLocked(async () =>
handleCloseCommand({
req: scope.req,
sessionName: scope.sessionName,
logPath: scope.requestLogPath,
sessionStore,
leaseRegistry,
leaseLifecycleProvider: { release },
}),
);

expect(response.ok).toBe(false);
if (!response.ok) {
expect(response.error.code).toBe('SESSION_NOT_FOUND');
}
expect(release).not.toHaveBeenCalled();
});

// The same deferred connection, but with an app-target `close <app>`. This
// must not reach `handleCloseCommand` at all — an app-target close with no
// session resolves its device straight from flags, so it stays behind full
// lease/tenant admission (the router rejects it before dispatch).
test('router: deferred tenant connect still refuses an app-target close before dispatch', async () => {
const sessionStore = makeSessionStore('agent-device-request-scope-');
const leaseRegistry = new LeaseRegistry();

const scope = await createRequestExecutionScope({
req: makeRequest({
session: 'default',
command: 'close',
positionals: ['com.example.app'],
meta: { tenantId: 'tenant-a', runId: 'run-1', sessionIsolation: 'tenant' },
}),
sessionStore,
leaseRegistry,
});

await expect(scope.runLocked(async () => 'unreachable')).rejects.toThrow(
/tenant isolation requires lease id/,
);
});

// A stored session that already exists under this tenant-scoped name but
// happens to carry no lease must still be refused: it could belong to a
// different run in the same tenant (sessions are tenant-scoped, not
// run-scoped), so a missing lease field alone is not proof of ownership.
test('router: an existing lease-less session under tenant isolation still refuses close', async () => {
const sessionStore = makeSessionStore('agent-device-request-scope-');
sessionStore.set('tenant-a:default', makeIosSession('tenant-a:default'));
const leaseRegistry = new LeaseRegistry();

const scope = await createRequestExecutionScope({
req: makeRequest({
session: 'default',
command: 'close',
meta: { tenantId: 'tenant-a', runId: 'run-1', sessionIsolation: 'tenant' },
}),
sessionStore,
leaseRegistry,
});
expect(scope.sessionName).toBe('tenant-a:default');

await expect(scope.runLocked(async () => 'unreachable')).rejects.toThrow(
/tenant isolation requires lease id/,
);
});

function makeRequest(overrides: Partial<DaemonRequest> = {}): DaemonRequest {
return {
token: 'test-token',
Expand Down
17 changes: 17 additions & 0 deletions src/daemon/daemon-command-registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,17 @@ export type DaemonCommandDescriptor = {
preferExplicitDeviceOverExistingSession?: boolean;
allowSessionlessDefaultDevice?: (req: DaemonRequest) => boolean;
skipSessionlessProviderDevice?: (req: DaemonRequest) => boolean;
/**
* #2016: this request shape is eligible for the sessionless,
* no-lease-anywhere lease-admission bypass — a session that was never
* created (deferred `connect`, `open` never ran) has no lease to admit or
* release. Only `close` declares it, and only for the plain-close shape
* (no app-target positional): `close <app>` resolves its device straight
* from flags when there's no session, so it must stay behind full
* lease/tenant admission. Declared here so `request-admission.ts` asks the
* registry instead of reclassifying `req.command`/`req.positionals` itself.
*/
sessionlessPlainCloseAdmissionExempt?: (req: DaemonRequest) => boolean;
};

export type DaemonProviderDeviceResolutionIntent =
Expand Down Expand Up @@ -121,6 +132,12 @@ export function usesSessionlessDefaultProviderDevice(req: DaemonRequest): boolea
return typeof allow === 'function' ? allow(req) : false;
}

/** #2016: whether this request qualifies for the sessionless plain-close lease-admission bypass. */
export function isSessionlessPlainCloseAdmissionExempt(req: DaemonRequest): boolean {
const exempt = getDaemonCommandDescriptor(req.command)?.sessionlessPlainCloseAdmissionExempt;
return typeof exempt === 'function' ? exempt(req) : false;
}

/**
* ADR 0014: the ref-frame effect a request resolves to, honoring the
* request-sensitive resolver form. Returns `undefined` for commands with no
Expand Down
24 changes: 23 additions & 1 deletion src/daemon/request-admission.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
import { AppError } from '@agent-device/kernel/errors';
import { normalizeTenantId, resolveSessionIsolationMode } from './config.ts';
import { isTenantOwnedSessionName, tenantScopedSessionName } from './session-tenant-scope.ts';
import { isLeaseAdmissionExempt } from './daemon-command-registry.ts';
import {
isLeaseAdmissionExempt,
isSessionlessPlainCloseAdmissionExempt,
} from './daemon-command-registry.ts';
import {
DEFAULT_PROXY_LEASE_TTL_MS,
findMissingProxyLeaseFields,
Expand Down Expand Up @@ -69,6 +72,25 @@ export function assertRequestLeaseAdmission(
const requestLeaseScope = resolveLeaseScope(req);
assertProxyOpenLeaseMetadata(req, requestLeaseScope);
const sessionLease = session?.lease;
// #2016: a tenant-isolated connection that never reached `open` has no
// daemon session and no lease to admit or release. Falling through would
// make the generic tenant/run/lease check below throw "tenant isolation
// requires lease id.", which reads as an access-control failure instead of
// "nothing to close". Let the close handler's own session lookup return
// its SESSION_NOT_FOUND response instead. Requires `session === undefined`,
// not just a lease-less session: a *stored* session under tenant isolation
// is keyed by tenant, not by run, so a lease-less stored session could
// belong to another run in the same tenant — admission must still verify a
// matching lease before that run's session can be torn down. Which request
// shape qualifies (plain `close`, not an app-target `close <app>`) is the
// registry's call, not this module's — see `sessionlessPlainCloseAdmissionExempt`.
if (
session === undefined &&
!requestLeaseScope.leaseId &&
isSessionlessPlainCloseAdmissionExempt(req)
) {
return undefined;
}
if (!sessionLease && req.meta?.sessionIsolation !== 'tenant') {
if (!requestLeaseScope.leaseId) return undefined;
if (!requestLeaseScope.tenantId && !requestLeaseScope.runId) return undefined;
Expand Down
Loading