Skip to content

Commit fc2f01a

Browse files
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 87fe3ea commit fc2f01a

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",
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+
});
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/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;
Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
import { createTestDeviceInventoryGateways } from '@agent-device/device-selection/inventory-gateways-fixtures';
2+
import { test, expect } from 'vitest';
3+
import path from 'node:path';
4+
import { fakeCreateCommandSurface } from '../../__tests__/support/command-surface-fake.ts';
5+
import {
6+
createRequestHandler,
7+
lifecycleDeviceRuntimeGateway,
8+
} from '../../__tests__/support/test-device-runtime-gateway.ts';
9+
import type { DaemonRequest } from '../../core/daemon-request.ts';
10+
import { LeaseRegistry } from '../../resources/lease-registry.ts';
11+
import { makeSessionStore } from '../../__tests__/support/store-factory.ts';
12+
import type { DeviceInfo } from '@agent-device/kernel/device';
13+
import { PRE_DISPATCH_REFUSAL_REASONS } from '@agent-device/kernel/errors';
14+
import { mkdtempForTestSync } from '@agent-device/test-kit';
15+
16+
// #3674: a run whose daemon was replaced mid-test keeps sending commands to the session it opened.
17+
// The answer has to name the session that went missing, and must not depend on how many devices
18+
// happen to be booted on the host.
19+
20+
const LOST_SESSION = 'e2e-ios-0';
21+
22+
function bootedSimulator(id: string, name: string): DeviceInfo {
23+
return {
24+
platform: 'apple',
25+
target: 'mobile',
26+
id,
27+
name,
28+
kind: 'simulator',
29+
booted: true,
30+
};
31+
}
32+
33+
const BOOTED_SIMULATORS: DeviceInfo[] = [
34+
bootedSimulator('SIM-A', 'TinyConcepts E2E'),
35+
bootedSimulator('SIM-B', 'Perf iPhone 17'),
36+
];
37+
38+
function makeHandler(listings: { count: number }) {
39+
const sessionStore = makeSessionStore('agent-device-router-named-session-');
40+
const handler = createRequestHandler({
41+
createCommandSurface: fakeCreateCommandSurface,
42+
logPath: path.join(mkdtempForTestSync('daemon'), 'daemon.log'),
43+
token: 'test-token',
44+
sessionStore,
45+
leaseRegistry: new LeaseRegistry(),
46+
deviceRuntimeGateway: lifecycleDeviceRuntimeGateway,
47+
deviceInventoryGateways: createTestDeviceInventoryGateways({
48+
local: async () => {
49+
listings.count += 1;
50+
return BOOTED_SIMULATORS;
51+
},
52+
}),
53+
trackDownloadableArtifact: () => 'artifact-id',
54+
});
55+
return { handler, sessionStore };
56+
}
57+
58+
function request(overrides: Partial<DaemonRequest> = {}): DaemonRequest {
59+
return {
60+
token: 'test-token',
61+
session: LOST_SESSION,
62+
command: 'snapshot',
63+
positionals: [],
64+
flags: {},
65+
meta: { requestId: 'named-session-request', sessionExplicit: true },
66+
...overrides,
67+
};
68+
}
69+
70+
test('a command naming a session this daemon does not hold answers on the session, not the device', async () => {
71+
const { handler } = makeHandler({ count: 0 });
72+
73+
const response = await handler(request());
74+
75+
expect(response.ok).toBe(false);
76+
if (response.ok) return;
77+
expect(response.error.code).toBe('SESSION_NOT_FOUND');
78+
expect(response.error.message).toContain(`No active session "${LOST_SESSION}"`);
79+
expect(response.error.details?.reason).toBe(PRE_DISPATCH_REFUSAL_REASONS.namedSessionNotFound);
80+
expect(response.error.details?.dispatched).toBe('no');
81+
expect(response.error.details?.session).toBe(LOST_SESSION);
82+
});
83+
84+
test('the refusal answers without listing the host devices at all', async () => {
85+
const listings = { count: 0 };
86+
const { handler } = makeHandler(listings);
87+
88+
await handler(request());
89+
90+
expect(listings.count).toBe(0);
91+
});
92+
93+
test('the same command with no session named still resolves a device sessionlessly', async () => {
94+
// The closest negative: same daemon, same two booted simulators, same command. What changes is
95+
// only whether the caller named a session, and that is what the answer must turn on.
96+
const listings = { count: 0 };
97+
const { handler } = makeHandler(listings);
98+
99+
const response = await handler(
100+
request({ session: 'default', meta: { requestId: 'sessionless-request' } }),
101+
);
102+
103+
expect(response.ok).toBe(false);
104+
if (response.ok) return;
105+
expect(response.error.code).toBe('AMBIGUOUS_MATCH');
106+
expect(listings.count).toBeGreaterThan(0);
107+
});
108+
109+
test('a named session beside a device selector keeps honouring the device it names', async () => {
110+
// The other closest negative: the same missing session, but the request states its target
111+
// outright. Then the session is not what is missing and the refusal must stay out of the way.
112+
const listings = { count: 0 };
113+
const { handler } = makeHandler(listings);
114+
115+
const response = await handler(request({ flags: { platform: 'ios', udid: 'SIM-A' } }));
116+
117+
if (!response.ok) {
118+
expect(response.error.details?.reason).not.toBe(
119+
PRE_DISPATCH_REFUSAL_REASONS.namedSessionNotFound,
120+
);
121+
}
122+
expect(listings.count).toBeGreaterThan(0);
123+
});
124+
125+
test('a sessionless close naming a missing session answers on the session', async () => {
126+
const { handler } = makeHandler({ count: 0 });
127+
128+
const response = await handler(request({ command: 'close', positionals: ['com.example.app'] }));
129+
130+
expect(response.ok).toBe(false);
131+
if (response.ok) return;
132+
expect(response.error.code).toBe('SESSION_NOT_FOUND');
133+
expect(response.error.message).toContain(`No active session "${LOST_SESSION}"`);
134+
});

0 commit comments

Comments
 (0)