Skip to content

Commit 9ba53df

Browse files
committed
fix(daemon): key the implicit session by workspace and platform
An implicit session was addressed by `cwd:<workspace>:default`, one slot per checkout, so a repo driving both platforms could not open the second one: after `open --platform ios`, `open --platform android` and `boot --platform android` from the same directory failed with INVALID_ARGS against a session they never asked for. The workaround named a session by hand on every command of both legs. `--platform` now selects the implicit session, so `cwd:<workspace>:ios` and `cwd:<workspace>:android` coexist. A platform-naming request joins a session it genuinely agrees with, so a session opened without `--platform` stays reachable and keeps its own artifact directory; a same-platform device or target mismatch still gets the existing refusal instead of forking a twin. A request naming no platform joins the workspace's only implicit session and refuses with AMBIGUOUS_MATCH when several exist, while `sessionKind: 'inventory'` commands keep routing so `session list` can resolve the ambiguity. Two adjacent repairs were load-bearing: a settling interaction published the live session under `SessionState.name` rather than the address the store owns, which `session list` already reported as a phantom second row; and `--session-lock`'s platform reaches `flags` only after the key is chosen, so routing now reads `meta.lockPlatform` too.
1 parent 775163b commit 9ba53df

12 files changed

Lines changed: 726 additions & 16 deletions

‎CHANGELOG.md‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,46 @@
22

33
## Unreleased
44

5+
- Changed (sessions): the implicit session is now keyed by workspace **and platform**, so one checkout
6+
can drive iOS and Android without inventing a `--session` name for every command (#2580). An
7+
implicit session was addressed by `cwd:<workspace>:default`, one slot per checkout, and it stayed
8+
bound to the first device it touched. A repo that tests both platforms — a visual-regression run
9+
across `react-native-paper` components, for example — could not open the second one at all: after
10+
`open --platform ios`, `open --platform android` from the same directory failed with `INVALID_ARGS`
11+
("already bound to apple device \"iPhone 17\""), and so did `boot --platform android`, which binds
12+
nothing and only needed a device to start. The hint was correct (`--session android` works), but
13+
following it means threading a hand-written name through every command of both legs, and a script
14+
that drops one flag silently talks to the other platform's device. `--platform` now selects the
15+
implicit session, so the platform a script already declares is the only handle it needs:
16+
`cwd:<workspace>:ios` and `cwd:<workspace>:android` coexist, each with its own device, app, and
17+
artifact directory. Three rules keep the behavior that was already earned.
18+
1. A platform-naming request joins the workspace's session only when it genuinely agrees with it,
19+
so a session opened without `--platform` — which keeps the platform-less `default` leaf — stays
20+
reachable from commands that do name a platform and keeps writing to its own artifact directory
21+
instead of growing a twin beside it. A request that agrees on platform but names a different
22+
device or target still gets the existing `INVALID_ARGS` refusal; it does not silently fork a
23+
second same-platform session that one `--platform` would then have to disambiguate.
24+
2. A request naming no platform joins the workspace's only implicit session — what every
25+
single-platform script does today, so those are unaffected — and refuses with `AMBIGUOUS_MATCH`
26+
when the workspace holds several, naming every address and the `--platform`/`--session` flag
27+
that selects it, instead of choosing a platform by open order. Same for a broad selector such as
28+
`--platform apple` that matches both this workspace's iPhone and its Mac.
29+
3. Inventory commands — `session list`, `devices`, `doctor`, `capabilities`, `apps`, which the
30+
registry classifies as `sessionKind: 'inventory'` — never claim a session and keep resolving an
31+
address through the ambiguity. Refusing those would refuse the command an agent runs to resolve
32+
it. `events` and `close` are deliberately not in that set: each reads or tears down one session,
33+
and a silently-empty `events` or a `close` that closed nothing would be worse than the refusal.
34+
Two adjacent repairs were required. A settling interaction published the live session under
35+
`SessionState.name`, which for an implicit session is only `default` and not the address it answers
36+
to, so `agent-device session list` already reported a phantom second row beside the real session;
37+
under platform keying that phantom would also have read as a second session the caller has to
38+
disambiguate, so the publisher now stores under the key the store itself owns, and workspace-session
39+
counting collapses one session reachable under two addresses regardless of writer. And
40+
`--session-lock`'s platform reaches `flags` only
41+
after routing has chosen the key, so routing now reads `meta.lockPlatform` too — otherwise a
42+
lock-policy caller, the exact audience that avoids naming sessions, would land on the wrong leaf.
43+
Explicit `--session <name>` is untouched and still addresses one session verbatim.
44+
545
- Fixed: an iOS capture that no backend could read now says which backend was asked and what was on
646
screen, instead of failing on the internal `regular iOS snapshot presentation requires a valid
747
viewport` invariant alone (#2560). The runner declares such a payload sparse — backend, reason code,

‎packages/command-registry/src/flag-definitions-target.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -417,7 +417,7 @@ export const TARGET_FLAG_DEFINITIONS: readonly FlagDefinition[] = [
417417
names: ['--session'],
418418
type: 'string',
419419
usageLabel: '--session <name>',
420-
usageDescription: 'Named session',
420+
usageDescription: 'Named session (implicit default is scoped to this workspace + platform)',
421421
projectConfig: true,
422422
recorded: false,
423423
},

‎src/daemon/__tests__/request-execution-scope.test.ts‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import {
2020
prepareLockedRequestScope,
2121
} from '../request-execution-scope.ts';
2222
import { resolveSessionRequestLogPath } from '../session-artifact-paths.ts';
23+
import { resolveSessionScope } from '../session-routing.ts';
2324
import type { DaemonRequest } from '../daemon-request.ts';
2425
import { mkdtempForTestSync } from '../../__tests__/test-utils/tmp-dir.ts';
2526
import { makeTestScreenRecordingResource } from '../../__tests__/test-utils/screen-recording-live-handle.ts';
@@ -935,3 +936,41 @@ function makeRequest(overrides: Partial<DaemonRequest> = {}): DaemonRequest {
935936
...overrides,
936937
};
937938
}
939+
940+
// The `attachesToSession` routing option is derived from the command's own registry classification,
941+
// so a swapped derivation has to be visible somewhere: an inventory command must keep resolving an
942+
// address across implicit session ambiguity, and a session command must refuse instead of picking
943+
// one workspace session by open order.
944+
async function createScopeAcrossTwoImplicitWorkspaceSessions(command: string) {
945+
const root = mkdtempForTestSync('agent-device-request-scope-ambiguity-');
946+
fs.mkdirSync(path.join(root, '.git'));
947+
const scope = resolveSessionScope({ ...makeRequest({ command }), meta: { cwd: root } });
948+
if (scope.kind !== 'cwd') throw new Error('expected a cwd session scope');
949+
const sessionStore = makeSessionStore('agent-device-request-scope-ambiguity-');
950+
sessionStore.set(
951+
`cwd:${scope.id}:ios`,
952+
makeIosSession('default', { sessionScope: { kind: 'cwd', id: scope.id } }),
953+
);
954+
sessionStore.set(
955+
`cwd:${scope.id}:android`,
956+
makeAndroidSession('default', { sessionScope: { kind: 'cwd', id: scope.id } }),
957+
);
958+
return await createRequestExecutionScope({
959+
req: makeRequest({ command, meta: { cwd: root, requestId: `ambiguity-${command}` } }),
960+
sessionStore,
961+
leaseRegistry: new LeaseRegistry(),
962+
});
963+
}
964+
965+
test('createRequestExecutionScope lets session list route across implicit session ambiguity', async () => {
966+
const scope = await createScopeAcrossTwoImplicitWorkspaceSessions('session_list');
967+
968+
expect(scope.sessionName).toMatch(/^cwd:[a-f0-9]{16}:default$/);
969+
await scope[Symbol.asyncDispose]();
970+
});
971+
972+
test('createRequestExecutionScope refuses a session command across implicit session ambiguity', async () => {
973+
await expect(createScopeAcrossTwoImplicitWorkspaceSessions('press')).rejects.toMatchObject({
974+
code: 'AMBIGUOUS_MATCH',
975+
});
976+
});

‎src/daemon/__tests__/request-router-replay-scope.test.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,8 @@ test('session list includes a cwd-scoped session opened by replay', async () =>
179179
data: {
180180
sessions: [
181181
expect.objectContaining({
182-
name: expect.stringMatching(/^cwd:[a-f0-9]+:default$/),
182+
// A platform-selecting request addresses that platform's own implicit session (#2580).
183+
name: expect.stringMatching(/^cwd:[a-f0-9]+:ios$/),
183184
}),
184185
],
185186
},

0 commit comments

Comments
 (0)