Skip to content

Commit a091770

Browse files
committed
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.
1 parent 42627d8 commit a091770

6 files changed

Lines changed: 47 additions & 18 deletions

File tree

src/__tests__/cli-help.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ test('help workflow prints the compact workflow card with a version header and s
7575
assert.equal(result.calls.length, 0);
7676
assert.match(result.stdout, /^agent-device \S+ workflow/);
7777
assert.ok(
78-
Buffer.byteLength(result.stdout, 'utf8') < 9100,
78+
Buffer.byteLength(result.stdout, 'utf8') < 9000,
7979
`help workflow should stay close to the compact-card size target, was ${Buffer.byteLength(result.stdout, 'utf8')} bytes`,
8080
);
8181
assert.match(result.stdout, /open -> snapshot -i -> settle -> verify -> close loop/);

src/cli-schema/cli-help-topics.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ test('usageForCommand resolves workflow help topic', async () => {
119119
if (help === null) throw new Error('Expected workflow help text');
120120
assert.match(help, /^agent-device \S+ workflow/);
121121
assert.ok(
122-
Buffer.byteLength(help, 'utf8') < 9100,
122+
Buffer.byteLength(help, 'utf8') < 9000,
123123
`workflow help topic should stay close to the compact-card size target, was ${Buffer.byteLength(help, 'utf8')} bytes`,
124124
);
125125
assert.match(help, /open -> snapshot -i -> settle -> verify -> close loop/);
@@ -138,7 +138,7 @@ test('usageForCommand resolves workflow help topic', async () => {
138138
assert.match(help, /Shapes and platform quirks: help gestures/);
139139
assert.match(
140140
help,
141-
/open --foreground -> snapshot\. Selection: explicit --device\/--udid\/--serial, then session, booted\/bootable local, or one provider; --platform\/--target only filter\./,
141+
/open --foreground -> snapshot\. Selection: explicit --device\/--udid\/--serial, then session, booted\/bootable local, or provider; --platform\/--target only filter\./,
142142
);
143143
assert.match(help, /Never open artifact paths or invent package ids/);
144144
assert.match(

src/cli-schema/cli-help.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ Command shape:
134134
Gestures: scroll/swipe for lists/flicks; gesture pan|fling|pinch|rotate|transform|drag for multi-touch. Shapes and platform quirks: help gestures.
135135
136136
Bootstrap:
137-
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.
137+
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.
138138
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.
139139
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.
140140
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.

src/core/command-descriptor/registry.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,14 @@ const isShardedTestRequest = (req: DispatchedCommand): boolean =>
170170
req.command === 'test' &&
171171
(typeof req.flags?.shardAll === 'number' || typeof req.flags?.shardSplit === 'number');
172172

173+
// #2016: a plain `close` (no app-target positional) has nothing to close via
174+
// flags, so it's the only close shape eligible for the sessionless
175+
// no-lease-anywhere admission bypass in request-admission.ts. `close <app>`
176+
// resolves its device straight from flags when there's no session and must
177+
// stay behind full lease/tenant admission.
178+
const isPlainCloseRequest = (req: DispatchedCommand): boolean =>
179+
(req.positionals?.length ?? 0) === 0;
180+
173181
// ADR 0014 request-sensitive ref-frame resolvers. The action is the leading
174182
// positional (see keyboard/alert daemon writers in src/commands/system/index.ts
175183
// and src/commands/capture/alert.ts). Only the read-only status probes preserve
@@ -922,6 +930,7 @@ export const RAW_COMMAND_DESCRIPTORS = [
922930
refFrameEffect: 'may-invalidate',
923931
allowInvalidRecording: true,
924932
saveScriptFlagOwner: true,
933+
sessionlessPlainCloseAdmissionExempt: isPlainCloseRequest,
925934
},
926935
timeoutPolicy: DEFAULT_TIMEOUT_POLICY,
927936
batchable: true,

src/daemon/daemon-command-registry.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,17 @@ export type DaemonCommandDescriptor = {
4444
preferExplicitDeviceOverExistingSession?: boolean;
4545
allowSessionlessDefaultDevice?: (req: DaemonRequest) => boolean;
4646
skipSessionlessProviderDevice?: (req: DaemonRequest) => boolean;
47+
/**
48+
* #2016: this request shape is eligible for the sessionless,
49+
* no-lease-anywhere lease-admission bypass — a session that was never
50+
* created (deferred `connect`, `open` never ran) has no lease to admit or
51+
* release. Only `close` declares it, and only for the plain-close shape
52+
* (no app-target positional): `close <app>` resolves its device straight
53+
* from flags when there's no session, so it must stay behind full
54+
* lease/tenant admission. Declared here so `request-admission.ts` asks the
55+
* registry instead of reclassifying `req.command`/`req.positionals` itself.
56+
*/
57+
sessionlessPlainCloseAdmissionExempt?: (req: DaemonRequest) => boolean;
4758
};
4859

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

135+
/** #2016: whether this request qualifies for the sessionless plain-close lease-admission bypass. */
136+
export function isSessionlessPlainCloseAdmissionExempt(req: DaemonRequest): boolean {
137+
const exempt = getDaemonCommandDescriptor(req.command)?.sessionlessPlainCloseAdmissionExempt;
138+
return typeof exempt === 'function' ? exempt(req) : false;
139+
}
140+
124141
/**
125142
* ADR 0014: the ref-frame effect a request resolves to, honoring the
126143
* request-sensitive resolver form. Returns `undefined` for commands with no

src/daemon/request-admission.ts

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
import { AppError } from '@agent-device/kernel/errors';
22
import { normalizeTenantId, resolveSessionIsolationMode } from './config.ts';
33
import { isTenantOwnedSessionName, tenantScopedSessionName } from './session-tenant-scope.ts';
4-
import { isLeaseAdmissionExempt } from './daemon-command-registry.ts';
4+
import {
5+
isLeaseAdmissionExempt,
6+
isSessionlessPlainCloseAdmissionExempt,
7+
} from './daemon-command-registry.ts';
58
import {
69
DEFAULT_PROXY_LEASE_TTL_MS,
710
findMissingProxyLeaseFields,
@@ -69,22 +72,22 @@ export function assertRequestLeaseAdmission(
6972
const requestLeaseScope = resolveLeaseScope(req);
7073
assertProxyOpenLeaseMetadata(req, requestLeaseScope);
7174
const sessionLease = session?.lease;
72-
// #2016: plain `close` (no app target) on a tenant-isolated connection
73-
// that never reached `open` has no daemon session and no lease to admit
74-
// or release. Falling through would make the generic tenant/run/lease
75-
// check below throw "tenant isolation requires lease id.", which reads as
76-
// an access-control failure instead of "nothing to close". Let the close
77-
// handler's own session lookup return its SESSION_NOT_FOUND response
78-
// instead. Requires `session === undefined`, not just a lease-less
79-
// session: a *stored* session under tenant isolation is keyed by tenant,
80-
// not by run, so a lease-less stored session could belong to another run
81-
// in the same tenant — admission must still verify a matching lease
82-
// before that run's session can be torn down.
75+
// #2016: a tenant-isolated connection that never reached `open` has no
76+
// daemon session and no lease to admit or release. Falling through would
77+
// make the generic tenant/run/lease check below throw "tenant isolation
78+
// requires lease id.", which reads as an access-control failure instead of
79+
// "nothing to close". Let the close handler's own session lookup return
80+
// its SESSION_NOT_FOUND response instead. Requires `session === undefined`,
81+
// not just a lease-less session: a *stored* session under tenant isolation
82+
// is keyed by tenant, not by run, so a lease-less stored session could
83+
// belong to another run in the same tenant — admission must still verify a
84+
// matching lease before that run's session can be torn down. Which request
85+
// shape qualifies (plain `close`, not an app-target `close <app>`) is the
86+
// registry's call, not this module's — see `sessionlessPlainCloseAdmissionExempt`.
8387
if (
8488
session === undefined &&
8589
!requestLeaseScope.leaseId &&
86-
req.command === 'close' &&
87-
(req.positionals?.length ?? 0) === 0
90+
isSessionlessPlainCloseAdmissionExempt(req)
8891
) {
8992
return undefined;
9093
}

0 commit comments

Comments
 (0)