From 80c4f4a6714f2eb88ade1b052f9591426a085d3a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Tue, 25 Aug 2026 14:46:44 +0200 Subject: [PATCH 1/4] fix(daemon): close no longer throws tenant-isolation error on a never-allocated lease MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `close` on a tenant-isolated connection (e.g. BrowserStack) whose lease was never allocated (`connect` succeeded but `open` never reached `lease_allocate`) threw a generic "tenant isolation requires lease id." error from the lease-admission gate before `close`'s own handler ever got a chance to run — masking the much clearer SESSION_NOT_FOUND / lease-less teardown path it already supports. Scoped narrowly to plain `close` (no app-target positional) so `close `, which resolves its device straight from flags when there's no session, still goes through full lease/tenant admission. Fixes #2016 --- .../__tests__/request-admission.test.ts | 141 ++++++++++++++++++ src/daemon/request-admission.ts | 18 +++ 2 files changed, 159 insertions(+) create mode 100644 src/daemon/__tests__/request-admission.test.ts diff --git a/src/daemon/__tests__/request-admission.test.ts b/src/daemon/__tests__/request-admission.test.ts new file mode 100644 index 0000000000..0b6cde4e57 --- /dev/null +++ b/src/daemon/__tests__/request-admission.test.ts @@ -0,0 +1,141 @@ +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 { + 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 tenant-isolated session record with no lease field admits without throwing', () => { + const registry = new LeaseRegistry(); + const session = makeIosSession('default'); + + const result = assertRequestLeaseAdmission( + makeRequest({ + command: 'close', + meta: { tenantId: 'tenant-a', runId: 'run-1', sessionIsolation: 'tenant' }, + }), + registry, + session, + ); + + assert.equal(result, undefined); +}); + +test('close with an app target still requires a lease id even with no lease anywhere', () => { + // #2016 follow-up: `close ` 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); +}); diff --git a/src/daemon/request-admission.ts b/src/daemon/request-admission.ts index a4f302173e..cfa0f7d719 100644 --- a/src/daemon/request-admission.ts +++ b/src/daemon/request-admission.ts @@ -69,6 +69,24 @@ export function assertRequestLeaseAdmission( const requestLeaseScope = resolveLeaseScope(req); assertProxyOpenLeaseMetadata(req, requestLeaseScope); const sessionLease = session?.lease; + // #2016: plain `close` (no app target) on a tenant-isolated connection + // whose lease was never allocated (e.g. `open` was never called) has 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 (SESSION_NOT_FOUND, + // or a lease-less teardown) decide instead. Scoped to zero positionals so + // an app-target close (`close `, which resolves its device straight + // from flags when there's no session) still goes through the normal + // lease/tenant check rather than closing an arbitrary device for free. + if ( + !sessionLease && + !requestLeaseScope.leaseId && + req.command === 'close' && + (req.positionals?.length ?? 0) === 0 + ) { + return undefined; + } if (!sessionLease && req.meta?.sessionIsolation !== 'tenant') { if (!requestLeaseScope.leaseId) return undefined; if (!requestLeaseScope.tenantId && !requestLeaseScope.runId) return undefined; From c244bee0109ada5af58cfbbf5c7766e72f8f528d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Tue, 25 Aug 2026 15:27:21 +0200 Subject: [PATCH 2/4] fix(daemon): require session===undefined for the close lease bypass, not just a missing lease field MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review on #2029 (P1): the bypass fired on any lease-less session, including a *stored* SessionState. Tenant-scoped session names are keyed by tenant, not by run, so a lease-less stored session could belong to a different run in the same tenant — a caller without a matching lease could tear it down. Narrow the bypass to session === undefined (no daemon session record at all), which is the actual deferred-connect case from #2016: `open` never ran, so the daemon never created a session to protect. Adds router-level regression tests through the real session-scoping/locked-admission/close-handler pipeline (request-execution-scope.test.ts): deferred connect with no session closes as SESSION_NOT_FOUND without ever calling the lease provider, while an app-target close and an existing lease-less stored session both still require a real lease. --- .../__tests__/request-admission.test.ts | 28 ++++-- .../__tests__/request-execution-scope.test.ts | 92 ++++++++++++++++++- src/daemon/request-admission.ts | 21 +++-- 3 files changed, 120 insertions(+), 21 deletions(-) diff --git a/src/daemon/__tests__/request-admission.test.ts b/src/daemon/__tests__/request-admission.test.ts index 0b6cde4e57..9cbe7c1b5c 100644 --- a/src/daemon/__tests__/request-admission.test.ts +++ b/src/daemon/__tests__/request-admission.test.ts @@ -34,20 +34,28 @@ test('close on a tenant-isolated session with no lease ever allocated admits wit assert.equal(result, undefined); }); -test('close on a tenant-isolated session record with no lease field admits without throwing', () => { +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'); - const result = assertRequestLeaseAdmission( - makeRequest({ - command: 'close', - meta: { tenantId: 'tenant-a', runId: 'run-1', sessionIsolation: 'tenant' }, - }), - registry, - session, + assert.throws( + () => + assertRequestLeaseAdmission( + makeRequest({ + command: 'close', + meta: { tenantId: 'tenant-a', runId: 'run-1', sessionIsolation: 'tenant' }, + }), + registry, + session, + ), + /tenant isolation requires lease id/, ); - - assert.equal(result, undefined); }); test('close with an app target still requires a lease id even with no lease anywhere', () => { diff --git a/src/daemon/__tests__/request-execution-scope.test.ts b/src/daemon/__tests__/request-execution-scope.test.ts index bea1818b6f..21cf59635d 100644 --- a/src/daemon/__tests__/request-execution-scope.test.ts +++ b/src/daemon/__tests__/request-execution-scope.test.ts @@ -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 { @@ -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'); @@ -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 `. 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 { return { token: 'test-token', diff --git a/src/daemon/request-admission.ts b/src/daemon/request-admission.ts index cfa0f7d719..0da9e7ace7 100644 --- a/src/daemon/request-admission.ts +++ b/src/daemon/request-admission.ts @@ -70,17 +70,18 @@ export function assertRequestLeaseAdmission( assertProxyOpenLeaseMetadata(req, requestLeaseScope); const sessionLease = session?.lease; // #2016: plain `close` (no app target) on a tenant-isolated connection - // whose lease was never allocated (e.g. `open` was never called) has 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 (SESSION_NOT_FOUND, - // or a lease-less teardown) decide instead. Scoped to zero positionals so - // an app-target close (`close `, which resolves its device straight - // from flags when there's no session) still goes through the normal - // lease/tenant check rather than closing an arbitrary device for free. + // 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. if ( - !sessionLease && + session === undefined && !requestLeaseScope.leaseId && req.command === 'close' && (req.positionals?.length ?? 0) === 0 From 42627d8a1ff966db992b837b8a89eadd5dbba007 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Tue, 25 Aug 2026 15:38:18 +0200 Subject: [PATCH 3/4] test: bump workflow help-card byte budget after merging main MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Merging main (#2020) grew the workflow help card's "Bootstrap" line past the existing 9000-byte budget by a few bytes — unrelated to this PR's fix, just picked up by merging latest main. Bumping the budget rather than trimming #2020's recently-rewritten help copy. --- src/__tests__/cli-help.test.ts | 2 +- src/cli-schema/cli-help-topics.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/__tests__/cli-help.test.ts b/src/__tests__/cli-help.test.ts index 1a9d9a3a19..f3ffea0dc6 100644 --- a/src/__tests__/cli-help.test.ts +++ b/src/__tests__/cli-help.test.ts @@ -75,7 +75,7 @@ test('help workflow prints the compact workflow card with a version header and s assert.equal(result.calls.length, 0); assert.match(result.stdout, /^agent-device \S+ — workflow/); assert.ok( - Buffer.byteLength(result.stdout, 'utf8') < 9000, + Buffer.byteLength(result.stdout, 'utf8') < 9100, `help workflow should stay close to the compact-card size target, was ${Buffer.byteLength(result.stdout, 'utf8')} bytes`, ); assert.match(result.stdout, /open -> snapshot -i -> settle -> verify -> close loop/); diff --git a/src/cli-schema/cli-help-topics.test.ts b/src/cli-schema/cli-help-topics.test.ts index fcd23401a6..c8a76d91cc 100644 --- a/src/cli-schema/cli-help-topics.test.ts +++ b/src/cli-schema/cli-help-topics.test.ts @@ -119,7 +119,7 @@ test('usageForCommand resolves workflow help topic', async () => { if (help === null) throw new Error('Expected workflow help text'); assert.match(help, /^agent-device \S+ — workflow/); assert.ok( - Buffer.byteLength(help, 'utf8') < 9000, + Buffer.byteLength(help, 'utf8') < 9100, `workflow help topic should stay close to the compact-card size target, was ${Buffer.byteLength(help, 'utf8')} bytes`, ); assert.match(help, /open -> snapshot -i -> settle -> verify -> close loop/); From a091770c48d4dcf38e166bec58ebecd796be919c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Tue, 25 Aug 2026 17:33:59 +0200 Subject: [PATCH 4/4] fix(daemon): move close's admission bypass into the command-descriptor registry Review on #2029, P1: request-admission.ts was reclassifying req.command and req.positionals inline instead of consuming a registry predicate (ADR-0003). Added a request-sensitive DaemonCommandDescriptor trait, sessionlessPlainCloseAdmissionExempt, declared on close's descriptor via a named predicate (isPlainCloseRequest); request-admission.ts now asks the registry (isSessionlessPlainCloseAdmissionExempt) instead of matching req.command === 'close' and req.positionals.length itself. P2: reverted the workflow help-card byte-budget bump from #2030bd (9000 -> 9100) back to 9000, and instead trimmed the "Bootstrap" help line by the minimum amount ("or one provider" -> "or provider", "providers never fall back" -> "no provider fallback") to fit the existing budget with a small margin (8994 bytes). That budget regression came from #2020 on main, unrelated to this PR's close/lease-admission fix. --- src/__tests__/cli-help.test.ts | 2 +- src/cli-schema/cli-help-topics.test.ts | 4 ++-- src/cli-schema/cli-help.ts | 2 +- src/core/command-descriptor/registry.ts | 9 +++++++ src/daemon/daemon-command-registry.ts | 17 ++++++++++++++ src/daemon/request-admission.ts | 31 ++++++++++++++----------- 6 files changed, 47 insertions(+), 18 deletions(-) diff --git a/src/__tests__/cli-help.test.ts b/src/__tests__/cli-help.test.ts index f3ffea0dc6..1a9d9a3a19 100644 --- a/src/__tests__/cli-help.test.ts +++ b/src/__tests__/cli-help.test.ts @@ -75,7 +75,7 @@ test('help workflow prints the compact workflow card with a version header and s assert.equal(result.calls.length, 0); assert.match(result.stdout, /^agent-device \S+ — workflow/); assert.ok( - Buffer.byteLength(result.stdout, 'utf8') < 9100, + Buffer.byteLength(result.stdout, 'utf8') < 9000, `help workflow should stay close to the compact-card size target, was ${Buffer.byteLength(result.stdout, 'utf8')} bytes`, ); assert.match(result.stdout, /open -> snapshot -i -> settle -> verify -> close loop/); diff --git a/src/cli-schema/cli-help-topics.test.ts b/src/cli-schema/cli-help-topics.test.ts index c8a76d91cc..35d5f1c0aa 100644 --- a/src/cli-schema/cli-help-topics.test.ts +++ b/src/cli-schema/cli-help-topics.test.ts @@ -119,7 +119,7 @@ test('usageForCommand resolves workflow help topic', async () => { if (help === null) throw new Error('Expected workflow help text'); assert.match(help, /^agent-device \S+ — workflow/); assert.ok( - Buffer.byteLength(help, 'utf8') < 9100, + Buffer.byteLength(help, 'utf8') < 9000, `workflow help topic should stay close to the compact-card size target, was ${Buffer.byteLength(help, 'utf8')} bytes`, ); assert.match(help, /open -> snapshot -i -> settle -> verify -> close loop/); @@ -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( diff --git a/src/cli-schema/cli-help.ts b/src/cli-schema/cli-help.ts index 2da57dc224..e1d70f63e8 100644 --- a/src/cli-schema/cli-help.ts +++ b/src/cli-schema/cli-help.ts @@ -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 --relaunch for fresh state. Use reinstall only when explicitly requested. Unknown app id: devices, then apps, then open . 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. diff --git a/src/core/command-descriptor/registry.ts b/src/core/command-descriptor/registry.ts index 093feb946a..62d1523a6f 100644 --- a/src/core/command-descriptor/registry.ts +++ b/src/core/command-descriptor/registry.ts @@ -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 ` +// 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 @@ -922,6 +930,7 @@ export const RAW_COMMAND_DESCRIPTORS = [ refFrameEffect: 'may-invalidate', allowInvalidRecording: true, saveScriptFlagOwner: true, + sessionlessPlainCloseAdmissionExempt: isPlainCloseRequest, }, timeoutPolicy: DEFAULT_TIMEOUT_POLICY, batchable: true, diff --git a/src/daemon/daemon-command-registry.ts b/src/daemon/daemon-command-registry.ts index 0e76ca9358..0229520bbb 100644 --- a/src/daemon/daemon-command-registry.ts +++ b/src/daemon/daemon-command-registry.ts @@ -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 ` 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 = @@ -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 diff --git a/src/daemon/request-admission.ts b/src/daemon/request-admission.ts index 0da9e7ace7..655350751c 100644 --- a/src/daemon/request-admission.ts +++ b/src/daemon/request-admission.ts @@ -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, @@ -69,22 +72,22 @@ export function assertRequestLeaseAdmission( const requestLeaseScope = resolveLeaseScope(req); assertProxyOpenLeaseMetadata(req, requestLeaseScope); const sessionLease = session?.lease; - // #2016: plain `close` (no app target) on 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. + // #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 `) is the + // registry's call, not this module's — see `sessionlessPlainCloseAdmissionExempt`. if ( session === undefined && !requestLeaseScope.leaseId && - req.command === 'close' && - (req.positionals?.length ?? 0) === 0 + isSessionlessPlainCloseAdmissionExempt(req) ) { return undefined; }