Skip to content

Commit ddd1817

Browse files
thymikeethymikee
committed
fix(daemon): answer a named missing session on the session, not the device (#3674)
A command that names a session this daemon does not hold used to fall through to resolveTargetDevice({}) — the session-attached command carries no device selectors, so the device was picked from the host inventory. With two devices booted the caller met AMBIGUOUS_MATCH telling it to select a device; with one it met an app-session error. Either way the answer depended on how many devices happened to be up and sent the caller after a device selector when what it lost was its session. Refuse at the seam instead: requireNamedSessionHoldsDevice throws SESSION_NOT_FOUND with typed reason named_session_not_found and dispatched=no, wired into every resolution that would otherwise choose a device (snapshot, selector runtime, recording, sessionless close). A request that never named a session stays sessionless by design, and one that names a device selector as well keeps its stated target. Provenance is traced, not inferred: namesSessionExplicitly reads meta.sessionExplicit or a non-blank flags.session, replacing the two copies of hasExplicitSessionFlag. It lives in @agent-device/kernel/ contracts, beside the meta field it reads, so every consumer already evaluates it; the guard module reaches the hot seams through a function-scoped import, keeping the eager-closure gate at parity (ADR-0019 loading shape). The refusal is pinned by a router regression test, unit tests, and a new daemon.route.named-session-refusal dispatch-disclosure row.
1 parent bb05c63 commit ddd1817

21 files changed

Lines changed: 415 additions & 32 deletions

‎contracts/fixtures/dispatch-disclosure.json‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,12 @@
107107
"trigger": "a device-bound command with no active session and no device selector: requireSessionOrExplicitSelector refuses before any target resolves, carrying details.reason session_or_device_selector_required",
108108
"dispatched": "no"
109109
},
110+
{
111+
"id": "daemon.route.named-session-refusal",
112+
"producer": "daemon",
113+
"trigger": "a command naming a session this daemon does not hold and no device selector: requireNamedSessionHoldsDevice refuses with SESSION_NOT_FOUND and details.reason named_session_not_found before any device is listed or resolved",
114+
"dispatched": "no"
115+
},
110116
{
111117
"id": "daemon.route.find-type-refused-after-focus",
112118
"producer": "daemon",

‎packages/daemon/src/core/explicit-session-resolution.test.ts‎

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
import assert from 'node:assert/strict';
2+
import { test } from 'vitest';
3+
import type { DaemonWireRequest } from '@agent-device/contracts/command';
4+
import { PRE_DISPATCH_REFUSAL_REASONS, normalizeError } from '@agent-device/kernel/errors';
5+
import type { SessionState } from './session-state.ts';
6+
import {
7+
namedSessionNotFoundError,
8+
requireNamedSessionHoldsDevice,
9+
} from './explicit-session-resolution.ts';
10+
11+
function request(overrides: Partial<DaemonWireRequest> = {}): DaemonWireRequest {
12+
return {
13+
token: 't',
14+
session: 'e2e-ios-0',
15+
command: 'snapshot',
16+
positionals: [],
17+
...overrides,
18+
};
19+
}
20+
21+
function heldSession(address = 'e2e-ios-0'): SessionState {
22+
return {
23+
name: address,
24+
device: { platform: 'android', id: 'emulator-5554', name: 'Pixel 9', kind: 'emulator' },
25+
createdAt: 0,
26+
actions: [],
27+
};
28+
}
29+
30+
function normalizeRefusal(error: unknown) {
31+
assert.ok(error instanceof Error);
32+
return normalizeError(error);
33+
}
34+
35+
test('a named session the daemon does not hold is refused on the session, not the device', () => {
36+
assert.throws(
37+
() =>
38+
requireNamedSessionHoldsDevice({
39+
req: request({ meta: { sessionExplicit: true } }),
40+
sessionAddress: 'e2e-ios-0',
41+
session: undefined,
42+
}),
43+
(error: unknown) => {
44+
const refusal = normalizeRefusal(error);
45+
assert.equal(refusal.code, 'SESSION_NOT_FOUND');
46+
assert.equal(
47+
refusal.message,
48+
'No active session "e2e-ios-0". Run agent-device open <app> --session e2e-ios-0 first, ' +
49+
'or omit --session to run against a device selector.',
50+
);
51+
assert.equal(refusal.details?.reason, PRE_DISPATCH_REFUSAL_REASONS.namedSessionNotFound);
52+
assert.equal(refusal.details?.dispatched, 'no');
53+
assert.equal(refusal.details?.session, 'e2e-ios-0');
54+
assert.match(String(refusal.hint), /agent-device session list/);
55+
return true;
56+
},
57+
);
58+
});
59+
60+
test('the session flag alone names a session as firmly as the client meta does', () => {
61+
// The predicate itself is pinned in @agent-device/kernel/contracts; what the guard owes is
62+
// that BOTH carriers reach the refusal.
63+
const fromMeta = request({ meta: { sessionExplicit: true } });
64+
const fromFlag = request({ flags: { session: 'e2e-ios-0' } });
65+
for (const req of [fromMeta, fromFlag]) {
66+
assert.throws(
67+
() =>
68+
requireNamedSessionHoldsDevice({ req, sessionAddress: 'e2e-ios-0', session: undefined }),
69+
{ code: 'SESSION_NOT_FOUND' },
70+
);
71+
}
72+
});
73+
74+
test('a request that never named a session keeps its sessionless device resolution', () => {
75+
requireNamedSessionHoldsDevice({
76+
req: request({ session: 'cwd:1a2b3c4d5e6f7a8b:ios' }),
77+
sessionAddress: 'cwd:1a2b3c4d5e6f7a8b:ios',
78+
session: undefined,
79+
});
80+
requireNamedSessionHoldsDevice({
81+
req: request({ session: 'default', flags: {} }),
82+
sessionAddress: 'default',
83+
session: undefined,
84+
});
85+
});
86+
87+
test('a session the daemon holds never reaches the refusal', () => {
88+
requireNamedSessionHoldsDevice({
89+
req: request({ meta: { sessionExplicit: true } }),
90+
sessionAddress: 'e2e-ios-0',
91+
session: heldSession(),
92+
});
93+
});
94+
95+
test('a named session beside a device selector keeps the target the caller stated', () => {
96+
for (const flags of [
97+
{ udid: 'SIM-1' },
98+
{ serial: 'emulator-5554' },
99+
{ device: 'iPhone 17' },
100+
{ platform: 'ios' as const },
101+
{ target: 'tv' as const },
102+
]) {
103+
requireNamedSessionHoldsDevice({
104+
req: request({ flags, meta: { sessionExplicit: true } }),
105+
sessionAddress: 'e2e-ios-0',
106+
session: undefined,
107+
});
108+
}
109+
});
110+
111+
test('the recovery command echoes the address back, quoted only where the shell needs it', () => {
112+
// A scoped store key is already a safe shell word, so the hint hands back the exact address
113+
// `session list` printed; a name with a space has to arrive quoted to run as one argument.
114+
const scoped = normalizeRefusal(namedSessionNotFoundError('cwd:1a2b3c4d5e6f7a8b:ios'));
115+
assert.match(String(scoped.message), /--session cwd:1a2b3c4d5e6f7a8b:ios first/);
116+
assert.equal(scoped.details?.session, 'cwd:1a2b3c4d5e6f7a8b:ios');
117+
const spaced = normalizeRefusal(namedSessionNotFoundError('my session'));
118+
assert.match(String(spaced.message), /--session 'my session' first/);
119+
});

‎packages/daemon/src/core/explicit-session-resolution.ts‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
import type { DaemonWireRequest } from '@agent-device/contracts/command';
2+
import { shellQuoteIfNeeded } from '@agent-device/kernel/device-shell';
3+
import { namesSessionExplicitly } from '@agent-device/kernel/contracts';
4+
import {
5+
AppError,
6+
PRE_DISPATCH_REFUSAL_REASONS,
7+
type RefusalDetails,
8+
} from '@agent-device/kernel/errors';
9+
import { hasDeviceSelectionInput } from './device/device-selector-intent.ts';
10+
import type { SessionState } from './session-state.ts';
11+
12+
/**
13+
* The answer to a request that names a session this daemon does not hold: the session, not the
14+
* device, is what is missing, and the command stopped before anything could reach a device.
15+
*/
16+
export function namedSessionNotFoundError(sessionAddress: string): AppError {
17+
const sessionArg = shellQuoteIfNeeded(sessionAddress);
18+
return new AppError(
19+
'SESSION_NOT_FOUND',
20+
`No active session "${sessionAddress}". Run agent-device open <app> --session ${sessionArg} first, or omit --session to run against a device selector.`,
21+
{
22+
reason: PRE_DISPATCH_REFUSAL_REASONS.namedSessionNotFound,
23+
dispatched: 'no',
24+
session: sessionAddress,
25+
hint:
26+
'Run agent-device session list to see the sessions this daemon holds. ' +
27+
"A daemon that was replaced or restarted starts without its predecessor's sessions.",
28+
} satisfies RefusalDetails<(typeof PRE_DISPATCH_REFUSAL_REASONS)['namedSessionNotFound']>,
29+
);
30+
}
31+
32+
/**
33+
* Refuses a request whose session the daemon does not hold, at the seam where that request would
34+
* otherwise have had its device chosen from its own selectors.
35+
*
36+
* The fall-through this closes is what an unheld session leaves behind. A session-attached command
37+
* names no device, so resolving one from its flags meant picking from every device on the host:
38+
* with one booted the caller met an app-session error, with two an `AMBIGUOUS_MATCH` telling it to
39+
* select a device — an answer that depends on how many devices happen to be up, and that sends the
40+
* caller after a device selector when what it lost was its session (#3674). Refusing here also skips
41+
* the device listing the fall-through paid for.
42+
*
43+
* Two shapes keep the path they chose: a request that never named a session, which is using the
44+
* daemon sessionlessly by design, and one that names a device selector as well, which states its
45+
* target outright and needs no session to supply one. The predicate deciding the first is
46+
* `namesSessionExplicitly` (`@agent-device/kernel/contracts`), traced from what the request carries
47+
* rather than from the session string.
48+
*/
49+
export function requireNamedSessionHoldsDevice(
50+
params: Readonly<{
51+
req: DaemonWireRequest;
52+
sessionAddress: string;
53+
session: SessionState | undefined;
54+
}>,
55+
): void {
56+
if (params.session) return;
57+
if (!namesSessionExplicitly(params.req)) return;
58+
if (hasDeviceSelectionInput(params.req.flags)) return;
59+
throw namedSessionNotFoundError(params.sessionAddress);
60+
}

‎packages/daemon/src/core/snapshot-runtime-binding.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ export async function resolveBoundSnapshotCaptureRuntime(
154154
const { ref, session, device } = await resolveSessionDevice(
155155
sessionStore,
156156
sessionName,
157-
req.flags,
157+
req,
158158
params.sessionRef,
159159
);
160160
if (

‎packages/daemon/src/core/snapshot-session.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,14 @@ import { SessionStore } from './session-store.ts';
99
export async function resolveSessionDevice(
1010
sessionStore: SessionStore,
1111
sessionName: string,
12-
flags: DaemonRequest['flags'],
12+
req: DaemonRequest,
1313
boundRef?: SessionRef,
1414
) {
1515
const ref = boundRef ?? sessionStore.lookup(sessionName);
1616
const session = ref ? sessionStore.requireCurrent(ref) : undefined;
17-
const device = session?.device ?? (await resolveTargetDevice(flags ?? {}));
17+
const { requireNamedSessionHoldsDevice } = await import('./explicit-session-resolution.ts');
18+
requireNamedSessionHoldsDevice({ req, sessionAddress: sessionName, session });
19+
const device = session?.device ?? (await resolveTargetDevice(req.flags ?? {}));
1820
return { ref, session, device };
1921
}
2022

‎packages/daemon/src/execution/request-dispatch-disclosure.test.ts‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -273,6 +273,21 @@ async function sessionOrSelectorRefusedBeforeRouting(): Promise<unknown> {
273273
assert.fail(`expected the routing refusal, got ${String(failure)}`);
274274
}
275275

276+
/**
277+
* The store holds `SESSION` and the request names another address, which is what a run against a
278+
* replaced daemon looks like. The row is about the refusal ordering: `snapshot` would otherwise
279+
* have chosen a device from the host inventory and answered with that.
280+
*/
281+
async function namedSessionRefusedBeforeDeviceSelection(): Promise<unknown> {
282+
const failure = await route(makeSession(SESSION), {
283+
command: 'snapshot',
284+
positionals: [],
285+
session: 'session-never-opened',
286+
meta: { requestId: 'named-session-refusal', sessionExplicit: true },
287+
});
288+
assert.fail(`expected the named-session refusal, got ${String(failure)}`);
289+
}
290+
276291
function captureNodes(nodes: readonly unknown[]): void {
277292
gestureRuntimeSpies.captureSnapshot.mockResolvedValue({
278293
backend: 'xctest',
@@ -354,6 +369,7 @@ const DRIVERS: Record<string, () => Promise<unknown>> = {
354369
'daemon.route.scroll-then-dialog-read-refused': androidScrollThenDialogReadRefused,
355370
'daemon.route.read-only-get': readOnlyGet,
356371
'daemon.route.session-or-selector-refusal': sessionOrSelectorRefusedBeforeRouting,
372+
'daemon.route.named-session-refusal': namedSessionRefusedBeforeDeviceSelection,
357373
};
358374

359375
const ROWS = dispatchDisclosureRowsOwnedBy(

‎packages/daemon/src/execution/selector-runtime-backend.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,12 @@ async function resolveSelectorRuntimeDevice(
116116
const ref = params.sessionStore.lookup(params.sessionName);
117117
const session = ref?.session;
118118
if (!session && requireSession) return { ok: false, response: noActiveSessionError() };
119+
const { requireNamedSessionHoldsDevice } = await import('../core/explicit-session-resolution.ts');
120+
requireNamedSessionHoldsDevice({
121+
req: params.req,
122+
sessionAddress: params.sessionName,
123+
session,
124+
});
119125
const device = session?.device ?? (await resolveTargetDevice(params.req.flags ?? {}));
120126
return { ok: true, ref, session, device };
121127
}

‎packages/daemon/src/execution/wait-runtime.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ export async function dispatchWaitViaRuntime(params: DispatchWaitParams): Promis
4747
const parsedOrResponse = parseWaitRequest(req);
4848
if ('ok' in parsedOrResponse) return parsedOrResponse;
4949
const parsed = parsedOrResponse;
50-
const { ref, session, device } = await resolveSessionDevice(sessionStore, sessionName, req.flags);
50+
const { ref, session, device } = await resolveSessionDevice(sessionStore, sessionName, req);
5151
// ADR 0019: facts are the only support authority, through the selector family's one
5252
// admit-then-bind entry. A duration wait observes nothing, so it never asks for a binding —
5353
// exactly the cell legacy admission skipped by testing `parsed.kind !== 'sleep'`.

‎packages/daemon/src/resources/session-device-resolution.ts‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,6 @@ export function requireSessionOrExplicitSelector(
2828
);
2929
}
3030

31-
export function hasExplicitSessionFlag(flags: DaemonRequest['flags'] | undefined): boolean {
32-
return typeof flags?.session === 'string' && flags.session.trim().length > 0;
33-
}
34-
3531
export async function resolveCommandDevice(params: {
3632
session: SessionState | undefined;
3733
flags: DaemonRequest['flags'] | undefined;

0 commit comments

Comments
 (0)