mirror of
https://github.com/callstack/agent-device.git
synced 2026-09-14 20:06:34 +08:00
fix(daemon): open --foreground P1 hotfix — selector rejection, interactive snapshot, capture-failure masking (#1671)
* fix(daemon): open --foreground P1 hotfix — selector rejection, interactive snapshot, capture-failure masking Three P1s from the post-merge review of #1670, all at the session-open-foreground dispatch seam: 1. Explicit device selectors were silently overwritten: the resolved-device rewrite pinned --udid/--platform over whatever the caller passed, so `open --foreground --udid B` with sim A sole-booted silently opened A. Now fails fast with INVALID_ARGS (matching the existing app-positional rejection) on --udid/--device, and on --platform other than ios; an explicit --platform ios passes through. 2. The promised interactive snapshot was never requested: the composed snapshot dispatch forwarded the open request's flags untouched, without snapshotInteractiveOnly — so the capture was NOT the `snapshot -i` path the doc comment promised and returned no interactive presentation. The composed request now sets snapshotInteractiveOnly: true (the exact key the CLI maps -i to and the snapshot runtime reads as interactiveOnly). 3. A capture failure masked the successful open: returning the snapshot error discarded openResponse even though the session exists, so a retry of `open --foreground` failed with "close the current session first". Open success + snapshot failure now returns ok with an explicit initialSnapshotError {code, message} detail and a rendered warning that the session IS open and how to capture manually (snapshot -i). Regressions added for all three: explicit-selector rejection (udid/device/both/non-iOS platform + ios pass-through), the composed dispatch carrying snapshotInteractiveOnly, and the snapshot-failure path returning ok + warning + usable session. * fix(cli): render the composed open --foreground snapshot on default stdout and project initialSnapshotError through the public surfaces Post-merge review on #1671 found the daemon fixes never reached the public boundaries: openCliOutput ignored the nested snapshot (the one-call promise held only under --json), and initialSnapshotError was daemon-only — absent from AppOpenResult, Node normalization, and serializeOpenResult, with the normalized shape truncated to code+message. - default open output now renders the composed interactive tree through the same snapshotCliOutput path snapshot -i uses - AppOpenResult carries initialSnapshotError as the FULL daemon error (hint/details/diagnosticId/logPath preserved) through normalization and serialization Worker-authored; committed by the coordinating session after the worker stalled twice mid-push. Tests: output.test.ts + session-open-foreground (26 pass), typecheck, oxfmt. * refactor(client): one daemon-error normalizer + client-route regressions for initialSnapshotError Review follow-ups on #1671: normalizeInitialSnapshotError duplicated the target-shutdown error normalization and pushed the module over the fallow complexity threshold; consolidated into a single internal normalizeDaemonError (table-driven, full shape incl. retriable/supportedOn) used by both result paths, projected through normalizeOpenForegroundComposition. Client-route regressions: createAgentDeviceClient().apps.open now proves the full initialSnapshotError shape (hint/details/diagnosticId/logPath/retriable) survives normalization, and that a malformed one is dropped — deleting the boundary normalization fails these tests. Also rebased onto current main. * fix(daemon): a thrown initial-snapshot capture failure gets the same successful-open contract Review P1 on #1671: dispatchSnapshotViaRuntime rethrows ordinary capture/runner exceptions; the composition only handled a returned { ok: false }, so a thrown failure escaped to the router and failed the whole open after the session was created — retrying then wedged on the existing session. The catch normalizes the rejection (kernel normalizeError, same conversion the router applies) into the shared openWithInitialSnapshotFailure path: ok response, full-shape initialSnapshotError, session-usable warning. Rejecting-mock regression added alongside the returned-failure case.
This commit is contained in:
committed by
GitHub
parent
13bc70f24f
commit
4b89482a06
@@ -5,6 +5,7 @@ import type { JsonObject } from './json.ts';
|
||||
import type { SessionSurface } from './session-surface.ts';
|
||||
import type { TargetShutdownResult } from './target-shutdown-contract.ts';
|
||||
import type { DaemonInstallSource, SessionRuntimeHints } from '@agent-device/kernel/contracts';
|
||||
import type { DaemonError } from '@agent-device/kernel/errors';
|
||||
import type { PublicPlatform } from '@agent-device/kernel/device';
|
||||
import type {
|
||||
AgentDeviceIdentifiers,
|
||||
@@ -84,6 +85,14 @@ export type AppOpenResult = {
|
||||
* fields like `identifiers`.
|
||||
*/
|
||||
snapshot?: Record<string, unknown>;
|
||||
/**
|
||||
* open --foreground: present when the session opened successfully but the
|
||||
* composed initial snapshot capture failed. Carries the FULL daemon error
|
||||
* shape (code, message, hint, details, diagnosticId, logPath) so recovery
|
||||
* guidance survives to the caller; the session itself is open and usable —
|
||||
* a `warnings` entry says so and points at `snapshot -i`.
|
||||
*/
|
||||
initialSnapshotError?: DaemonError;
|
||||
identifiers: AgentDeviceIdentifiers;
|
||||
};
|
||||
|
||||
|
||||
@@ -260,6 +260,71 @@ test('apps.open resolves session device identifiers from open response', async (
|
||||
]);
|
||||
});
|
||||
|
||||
test('apps.open preserves the full initialSnapshotError shape through client normalization', async () => {
|
||||
// open --foreground: open succeeded, composed snapshot did not. The public
|
||||
// client result must carry the FULL daemon error — dropping the boundary
|
||||
// normalization (or truncating to code+message) must fail here.
|
||||
const initialSnapshotError = {
|
||||
code: 'COMMAND_FAILED',
|
||||
message: 'capture failed',
|
||||
hint: 'Run: agent-device snapshot -i',
|
||||
details: { reason: 'runner_capture_failed' },
|
||||
diagnosticId: 'ms-diag-1234',
|
||||
logPath: '/tmp/agent-device/sessions/qa/requests/snap.ndjson',
|
||||
retriable: true,
|
||||
};
|
||||
const setup = createTransport(async (req) => {
|
||||
if (req.command === 'open') {
|
||||
return {
|
||||
ok: true,
|
||||
data: {
|
||||
session: 'qa',
|
||||
appName: 'Settings',
|
||||
appBundleId: 'com.apple.Preferences',
|
||||
platform: 'ios',
|
||||
target: 'mobile',
|
||||
device: 'iPhone 16',
|
||||
id: 'SIM-001',
|
||||
kind: 'simulator',
|
||||
device_udid: 'SIM-001',
|
||||
warnings: ['The session is open, but the initial interactive snapshot failed.'],
|
||||
initialSnapshotError,
|
||||
},
|
||||
};
|
||||
}
|
||||
throw new Error(`Unexpected command: ${req.command}`);
|
||||
});
|
||||
const client = createAgentDeviceClient(setup.config, { transport: setup.transport });
|
||||
|
||||
const result = await client.apps.open({ app: 'Settings', platform: 'ios', foreground: true });
|
||||
|
||||
assert.deepEqual(result.initialSnapshotError, initialSnapshotError);
|
||||
assert.equal(result.snapshot, undefined);
|
||||
});
|
||||
|
||||
test('apps.open drops a malformed initialSnapshotError instead of projecting garbage', async () => {
|
||||
const setup = createTransport(async () => ({
|
||||
ok: true,
|
||||
data: {
|
||||
session: 'qa',
|
||||
appName: 'Settings',
|
||||
appBundleId: 'com.apple.Preferences',
|
||||
platform: 'ios',
|
||||
target: 'mobile',
|
||||
device: 'iPhone 16',
|
||||
id: 'SIM-001',
|
||||
kind: 'simulator',
|
||||
device_udid: 'SIM-001',
|
||||
initialSnapshotError: { code: 'COMMAND_FAILED' },
|
||||
},
|
||||
}));
|
||||
const client = createAgentDeviceClient(setup.config, { transport: setup.transport });
|
||||
|
||||
const result = await client.apps.open({ app: 'Settings', platform: 'ios' });
|
||||
|
||||
assert.equal(result.initialSnapshotError, undefined);
|
||||
});
|
||||
|
||||
test('apps.open forwards explicit runtime hints through the daemon request', async () => {
|
||||
const setup = createTransport(async () => ({
|
||||
ok: true,
|
||||
|
||||
@@ -42,6 +42,7 @@ import {
|
||||
buildMeta,
|
||||
normalizeDeployResult,
|
||||
normalizeDevice,
|
||||
normalizeOpenForegroundComposition,
|
||||
normalizeInstallFromSourceResult,
|
||||
normalizeMaterializationReleaseResult,
|
||||
normalizeOpenDevice,
|
||||
@@ -279,11 +280,7 @@ export function createAgentDeviceClient(
|
||||
startup: normalizeStartupSample(data.startup),
|
||||
runtime: normalizeRuntimeHints(data.runtime),
|
||||
device,
|
||||
// RFC prototype (open --foreground): only present when the daemon's
|
||||
// foreground-attach composition captured an initial snapshot.
|
||||
...(data.snapshot && typeof data.snapshot === 'object'
|
||||
? { snapshot: data.snapshot as Record<string, unknown> }
|
||||
: {}),
|
||||
...normalizeOpenForegroundComposition(data),
|
||||
identifiers: {
|
||||
session,
|
||||
deviceId: device?.id,
|
||||
|
||||
@@ -16,7 +16,7 @@ import {
|
||||
isSerialAddressablePlatform,
|
||||
type AppleOS,
|
||||
} from '@agent-device/kernel/device';
|
||||
import { AppError, type NormalizedError } from '@agent-device/kernel/errors';
|
||||
import { AppError, type DaemonError } from '@agent-device/kernel/errors';
|
||||
import type { SnapshotNode } from '@agent-device/kernel/snapshot';
|
||||
import { leaseScopeFromOptions, leaseScopeToRequestMeta } from '../core/lease-scope.ts';
|
||||
import type { DaemonRequest, SessionRuntimeHints } from '../daemon/types.ts';
|
||||
@@ -253,7 +253,7 @@ export function normalizeTargetShutdownResult(value: unknown): TargetShutdownRes
|
||||
) {
|
||||
return undefined;
|
||||
}
|
||||
const error = normalizeTargetShutdownError(value.error);
|
||||
const error = normalizeDaemonError(value.error);
|
||||
return {
|
||||
success: value.success,
|
||||
exitCode: value.exitCode,
|
||||
@@ -263,16 +263,44 @@ export function normalizeTargetShutdownResult(value: unknown): TargetShutdownRes
|
||||
};
|
||||
}
|
||||
|
||||
function normalizeTargetShutdownError(value: unknown): NormalizedError | undefined {
|
||||
/**
|
||||
* The one normalizer for daemon errors riding inside otherwise-ok results
|
||||
* (target-shutdown reports, the open --foreground initial-snapshot failure).
|
||||
* Preserves the FULL shape — hint/details/diagnosticId/logPath plus the
|
||||
* additive retriable/supportedOn signals — never a code+message truncation,
|
||||
* so recovery guidance survives to Node/CLI JSON callers.
|
||||
*/
|
||||
const DAEMON_ERROR_STRING_FIELDS = ['hint', 'diagnosticId', 'logPath', 'supportedOn'] as const;
|
||||
|
||||
function normalizeDaemonError(value: unknown): DaemonError | undefined {
|
||||
if (!isRecord(value)) return undefined;
|
||||
if (typeof value.code !== 'string' || typeof value.message !== 'string') return undefined;
|
||||
const error: DaemonError = { code: value.code, message: value.message };
|
||||
for (const field of DAEMON_ERROR_STRING_FIELDS) {
|
||||
const candidate = value[field];
|
||||
if (typeof candidate === 'string') error[field] = candidate;
|
||||
}
|
||||
if (isRecord(value.details)) error.details = value.details;
|
||||
if (typeof value.retriable === 'boolean') error.retriable = value.retriable;
|
||||
return error;
|
||||
}
|
||||
|
||||
/**
|
||||
* open --foreground composition extras on an ok open response: the initial
|
||||
* snapshot when the foreground-attach capture succeeded, or the FULL capture
|
||||
* error (never a code+message truncation) when open succeeded and the
|
||||
* composed snapshot did not — the session is open and usable either way.
|
||||
*/
|
||||
export function normalizeOpenForegroundComposition(data: Record<string, unknown>): {
|
||||
snapshot?: Record<string, unknown>;
|
||||
initialSnapshotError?: DaemonError;
|
||||
} {
|
||||
const initialSnapshotError = normalizeDaemonError(data.initialSnapshotError);
|
||||
return {
|
||||
code: value.code,
|
||||
message: value.message,
|
||||
...(typeof value.hint === 'string' ? { hint: value.hint } : {}),
|
||||
...(typeof value.diagnosticId === 'string' ? { diagnosticId: value.diagnosticId } : {}),
|
||||
...(typeof value.logPath === 'string' ? { logPath: value.logPath } : {}),
|
||||
...(isRecord(value.details) ? { details: value.details } : {}),
|
||||
...(data.snapshot && typeof data.snapshot === 'object'
|
||||
? { snapshot: data.snapshot as Record<string, unknown> }
|
||||
: {}),
|
||||
...(initialSnapshotError ? { initialSnapshotError } : {}),
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
@@ -45,6 +45,86 @@ describe('openCliOutput', () => {
|
||||
expect(output.data).toMatchObject({ warnings: [warning] });
|
||||
expect(output.text).toBe(`Opened: authoring\nWarning: ${warning}`);
|
||||
});
|
||||
|
||||
// #1671 P1: the composed open --foreground snapshot must render on default
|
||||
// stdout through the PRODUCTION formatter route (the family-registry entry
|
||||
// the CLI resolves, not a helper mock), printing the same interactive tree
|
||||
// lines `snapshot -i` prints after the open confirmation.
|
||||
test('production formatter route renders the composed --foreground snapshot tree on default output', async () => {
|
||||
const { formatCliOutput } = await import('../cli-output.ts');
|
||||
const { attachRefs } = await import('@agent-device/kernel/snapshot');
|
||||
const nodes = attachRefs([
|
||||
{ index: 0, type: 'Button', label: 'Sign In', depth: 0, hittable: true },
|
||||
{ index: 1, type: 'TextField', label: 'Email', depth: 0, hittable: true },
|
||||
]);
|
||||
const openResult: AppOpenResult = {
|
||||
session: 'default',
|
||||
sessionStateDir: '/tmp/agent-device/sessions/cwd_123_default',
|
||||
appBundleId: 'com.apple.Preferences',
|
||||
snapshot: { nodes, truncated: false, backend: 'xctest', refsGeneration: 1 },
|
||||
identifiers: { session: 'default' },
|
||||
};
|
||||
|
||||
const openOutput = withNoColor(() =>
|
||||
formatCliOutput({ name: 'open', input: {}, result: openResult }),
|
||||
);
|
||||
const snapshotOutput = withNoColor(() =>
|
||||
formatCliOutput({
|
||||
name: 'snapshot',
|
||||
input: { interactiveOnly: true },
|
||||
result: { nodes, truncated: false, backend: 'xctest', refsGeneration: 1 },
|
||||
}),
|
||||
);
|
||||
|
||||
if (!openOutput?.text || !snapshotOutput?.text) {
|
||||
throw new Error('production formatter route returned no text output');
|
||||
}
|
||||
// The tree lines are exactly what `snapshot -i` would print, appended
|
||||
// after the open confirmation lines.
|
||||
expect(openOutput.text).toBe(
|
||||
[
|
||||
'Opened: com.apple.Preferences',
|
||||
'Session state: /tmp/agent-device/sessions/cwd_123_default',
|
||||
snapshotOutput.text,
|
||||
].join('\n'),
|
||||
);
|
||||
expect(openOutput.text).toContain('Sign In');
|
||||
expect(openOutput.text).toContain('Email');
|
||||
// --json carries the same presented snapshot payload snapshot -i emits.
|
||||
const openJsonSnapshot = (openOutput.data as { snapshot?: unknown }).snapshot;
|
||||
expect(openJsonSnapshot).toEqual(snapshotOutput.jsonData ?? snapshotOutput.data);
|
||||
});
|
||||
|
||||
test('renders the initialSnapshotError warning without a tree when the composed capture failed', () => {
|
||||
const warning =
|
||||
'The session is open, but the initial interactive snapshot failed (COMMAND_FAILED: capture failed). Run: agent-device snapshot -i';
|
||||
const output = openCliOutput({
|
||||
session: 'default',
|
||||
warnings: [warning],
|
||||
initialSnapshotError: {
|
||||
code: 'COMMAND_FAILED',
|
||||
message: 'capture failed',
|
||||
hint: 'Retry with --debug and inspect diagnostics log for details.',
|
||||
diagnosticId: 'diag-1',
|
||||
logPath: '/tmp/req.ndjson',
|
||||
details: { reason: 'runner_unavailable' },
|
||||
},
|
||||
identifiers: { session: 'default' },
|
||||
});
|
||||
|
||||
expect(output.text).toBe(`Opened: default\nWarning: ${warning}`);
|
||||
// The FULL error shape survives into public JSON output.
|
||||
expect(output.data).toMatchObject({
|
||||
initialSnapshotError: {
|
||||
code: 'COMMAND_FAILED',
|
||||
message: 'capture failed',
|
||||
hint: 'Retry with --debug and inspect diagnostics log for details.',
|
||||
diagnosticId: 'diag-1',
|
||||
logPath: '/tmp/req.ndjson',
|
||||
details: { reason: 'runner_unavailable' },
|
||||
},
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('artifactsCliOutput', () => {
|
||||
|
||||
@@ -29,6 +29,7 @@ import {
|
||||
serializeSessionListEntry,
|
||||
} from '../../utils/result-serialization.ts';
|
||||
import { readCommandMessage } from '../../utils/success-text.ts';
|
||||
import { snapshotCliOutput } from '../capture/output.ts';
|
||||
import type { CliOutput } from '../command-contract.ts';
|
||||
import {
|
||||
messageCliOutput,
|
||||
@@ -101,7 +102,28 @@ export function openCliOutput(result: AppOpenResult): CliOutput {
|
||||
for (const warning of result.warnings ?? []) {
|
||||
lines.push(`Warning: ${warning}`);
|
||||
}
|
||||
return { data, text: lines.join('\n') || null };
|
||||
// open --foreground: the composed initial snapshot renders through the SAME
|
||||
// path `snapshot -i` uses (label dedupe + interactive tree text), after the
|
||||
// open confirmation — the one-call promise holds on default stdout, not just
|
||||
// --json. The composed capture is interactive-only by construction.
|
||||
const snapshotOutput = buildOpenInitialSnapshotOutput(result.snapshot);
|
||||
if (snapshotOutput) {
|
||||
data.snapshot = snapshotOutput.jsonData ?? snapshotOutput.data;
|
||||
if (snapshotOutput.text) lines.push(snapshotOutput.text);
|
||||
}
|
||||
return {
|
||||
data,
|
||||
...(snapshotOutput?.stderr ? { stderr: snapshotOutput.stderr } : {}),
|
||||
text: lines.join('\n') || null,
|
||||
};
|
||||
}
|
||||
|
||||
function buildOpenInitialSnapshotOutput(snapshot: AppOpenResult['snapshot']): CliOutput | null {
|
||||
if (!snapshot || !Array.isArray(snapshot.nodes)) return null;
|
||||
return snapshotCliOutput({
|
||||
result: snapshot as unknown as Parameters<typeof snapshotCliOutput>[0]['result'],
|
||||
interactiveOnly: true,
|
||||
});
|
||||
}
|
||||
|
||||
function closeCliOutput(result: AppCloseResult | SessionCloseResult): CliOutput {
|
||||
|
||||
@@ -6,6 +6,7 @@ const dispatchSnapshotViaRuntime = vi.hoisted(() => vi.fn());
|
||||
vi.mock('../../ios-app-session-hint.ts', () => ({ resolveSoleForegroundIosApp }));
|
||||
vi.mock('../../snapshot-runtime.ts', () => ({ dispatchSnapshotViaRuntime }));
|
||||
|
||||
import { AppError } from '@agent-device/kernel/errors';
|
||||
import { IOS_SIMULATOR } from '../../../__tests__/test-utils/index.ts';
|
||||
import type { DaemonRequest, DaemonResponse } from '../../types.ts';
|
||||
import {
|
||||
@@ -73,6 +74,62 @@ test('rejects --foreground combined with an explicit app argument', async () =>
|
||||
expect(resolveSoleForegroundIosApp).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
test.each([
|
||||
['udid', { udid: 'explicit-udid' }],
|
||||
['device', { device: 'iPhone 16' }],
|
||||
['udid and device', { udid: 'explicit-udid', device: 'iPhone 16' }],
|
||||
])(
|
||||
'rejects --foreground with an explicit %s selector instead of silently overwriting it',
|
||||
async (_label, selectorFlags) => {
|
||||
const resolution = await resolveForegroundOpenRequest({
|
||||
req: baseRequest({ flags: { foreground: true, ...selectorFlags } }),
|
||||
hasExistingSession: false,
|
||||
});
|
||||
|
||||
expect(resolution.type).toBe('response');
|
||||
if (resolution.type === 'response') {
|
||||
expect(resolution.response.ok).toBe(false);
|
||||
if (!resolution.response.ok) {
|
||||
expect(resolution.response.error.code).toBe('INVALID_ARGS');
|
||||
expect(resolution.response.error.message).toMatch(/resolves the device itself/);
|
||||
}
|
||||
}
|
||||
expect(resolveSoleForegroundIosApp).not.toHaveBeenCalled();
|
||||
},
|
||||
);
|
||||
|
||||
test('rejects --foreground with a non-iOS platform selector', async () => {
|
||||
const resolution = await resolveForegroundOpenRequest({
|
||||
req: baseRequest({ flags: { foreground: true, platform: 'android' } }),
|
||||
hasExistingSession: false,
|
||||
});
|
||||
|
||||
expect(resolution.type).toBe('response');
|
||||
if (resolution.type === 'response') {
|
||||
expect(resolution.response.ok).toBe(false);
|
||||
if (!resolution.response.ok) {
|
||||
expect(resolution.response.error.code).toBe('INVALID_ARGS');
|
||||
expect(resolution.response.error.message).toMatch(/only supports --platform ios/);
|
||||
}
|
||||
}
|
||||
expect(resolveSoleForegroundIosApp).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
test('an explicit --platform ios passes through to resolution', async () => {
|
||||
const soleBootedDevice = { ...IOS_SIMULATOR, id: 'booted-1', name: 'iPhone 16' };
|
||||
resolveSoleForegroundIosApp.mockResolvedValue({
|
||||
device: soleBootedDevice,
|
||||
app: { bundleId: 'xyz.blueskyweb.app', name: 'Bluesky' },
|
||||
});
|
||||
|
||||
const resolution = await resolveForegroundOpenRequest({
|
||||
req: baseRequest({ flags: { foreground: true, platform: 'ios' } }),
|
||||
hasExistingSession: false,
|
||||
});
|
||||
|
||||
expect(resolution.type).toBe('resolved');
|
||||
});
|
||||
|
||||
test('an unambiguous environment rewrites positionals and pins the resolved device', async () => {
|
||||
const soleBootedDevice = { ...IOS_SIMULATOR, id: 'booted-1', name: 'iPhone 16' };
|
||||
resolveSoleForegroundIosApp.mockResolvedValue({
|
||||
@@ -174,7 +231,7 @@ test('leaves a successful open response untouched when --foreground was not requ
|
||||
expect(dispatchSnapshotViaRuntime).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
test('attaches the initial snapshot by delegating to the existing snapshot runtime dispatch', async () => {
|
||||
test('attaches the initial INTERACTIVE snapshot by delegating to the existing snapshot runtime dispatch', async () => {
|
||||
dispatchSnapshotViaRuntime.mockResolvedValue({ ok: true, data: { nodes: [], truncated: false } });
|
||||
|
||||
const req = baseRequest({ flags: { foreground: true }, positionals: ['xyz.blueskyweb.app'] });
|
||||
@@ -186,8 +243,15 @@ test('attaches the initial snapshot by delegating to the existing snapshot runti
|
||||
openResponse: okOpenResponse,
|
||||
});
|
||||
|
||||
// #1670 P1: the composed dispatch must BE the `snapshot -i` path — the CLI
|
||||
// maps `-i` to snapshotInteractiveOnly, the key the snapshot runtime reads.
|
||||
expect(dispatchSnapshotViaRuntime).toHaveBeenCalledWith({
|
||||
req: { ...req, command: 'snapshot', positionals: [] },
|
||||
req: {
|
||||
...req,
|
||||
command: 'snapshot',
|
||||
positionals: [],
|
||||
flags: { ...req.flags, snapshotInteractiveOnly: true },
|
||||
},
|
||||
sessionName: 'default',
|
||||
logPath: '/tmp/daemon.log',
|
||||
sessionStore: {},
|
||||
@@ -198,10 +262,21 @@ test('attaches the initial snapshot by delegating to the existing snapshot runti
|
||||
});
|
||||
});
|
||||
|
||||
test('surfaces a snapshot-capture failure instead of the open success', async () => {
|
||||
test('a snapshot-capture failure never masks the successful open', async () => {
|
||||
// #1670 P1: the session EXISTS once open succeeded — returning the capture
|
||||
// error made a retry of `open --foreground` fail with "close the current
|
||||
// session first". The response must stay ok, disclose the capture failure,
|
||||
// and tell the caller the session is usable.
|
||||
const snapshotFailure: DaemonResponse = {
|
||||
ok: false,
|
||||
error: { code: 'COMMAND_FAILED', message: 'capture failed' },
|
||||
error: {
|
||||
code: 'COMMAND_FAILED',
|
||||
message: 'capture failed',
|
||||
hint: 'Retry with --debug and inspect diagnostics log for details.',
|
||||
diagnosticId: 'diag-1',
|
||||
logPath: '/tmp/requests/req-1.ndjson',
|
||||
details: { reason: 'runner_unavailable' },
|
||||
},
|
||||
};
|
||||
dispatchSnapshotViaRuntime.mockResolvedValue(snapshotFailure);
|
||||
|
||||
@@ -210,8 +285,59 @@ test('surfaces a snapshot-capture failure instead of the open success', async ()
|
||||
sessionName: 'default',
|
||||
logPath: '/tmp/daemon.log',
|
||||
sessionStore: {} as never,
|
||||
openResponse: okOpenResponse,
|
||||
openResponse: { ok: true, data: { session: 'default', warnings: ['pre-existing warning'] } },
|
||||
});
|
||||
|
||||
expect(result).toBe(snapshotFailure);
|
||||
expect(result.ok).toBe(true);
|
||||
if (result.ok) {
|
||||
expect(result.data?.session).toBe('default');
|
||||
expect(result.data?.snapshot).toBeUndefined();
|
||||
// The FULL error shape survives — hint/details/diagnosticId/logPath, not
|
||||
// a code+message truncation.
|
||||
expect(result.data?.initialSnapshotError).toEqual({
|
||||
code: 'COMMAND_FAILED',
|
||||
message: 'capture failed',
|
||||
hint: 'Retry with --debug and inspect diagnostics log for details.',
|
||||
diagnosticId: 'diag-1',
|
||||
logPath: '/tmp/requests/req-1.ndjson',
|
||||
details: { reason: 'runner_unavailable' },
|
||||
});
|
||||
expect(result.data?.warnings).toEqual([
|
||||
'pre-existing warning',
|
||||
'The session is open, but the initial interactive snapshot failed (COMMAND_FAILED: capture failed). Run: agent-device snapshot -i',
|
||||
]);
|
||||
}
|
||||
});
|
||||
|
||||
test('a THROWN snapshot-capture failure never masks the successful open either', async () => {
|
||||
// The runtime dispatch rethrows ordinary capture/runner exceptions; an
|
||||
// escaped rejection would fail the whole open after the session was created
|
||||
// and wedge the retry on the existing session — same contract as a returned
|
||||
// { ok: false }: ok response, disclosed failure, usable session.
|
||||
dispatchSnapshotViaRuntime.mockRejectedValue(
|
||||
new AppError('COMMAND_FAILED', 'runner crashed mid-capture', {
|
||||
diagnosticId: 'diag-thrown-1',
|
||||
}),
|
||||
);
|
||||
|
||||
const result = await composeOpenWithInitialSnapshot({
|
||||
req: baseRequest({ flags: { foreground: true } }),
|
||||
sessionName: 'default',
|
||||
logPath: '/tmp/daemon.log',
|
||||
sessionStore: {} as never,
|
||||
openResponse: { ok: true, data: { session: 'default' } },
|
||||
});
|
||||
|
||||
expect(result.ok).toBe(true);
|
||||
if (result.ok) {
|
||||
expect(result.data?.session).toBe('default');
|
||||
expect(result.data?.snapshot).toBeUndefined();
|
||||
const error = result.data?.initialSnapshotError as Record<string, unknown>;
|
||||
expect(error?.code).toBe('COMMAND_FAILED');
|
||||
expect(error?.message).toBe('runner crashed mid-capture');
|
||||
expect(error?.diagnosticId).toBe('diag-thrown-1');
|
||||
expect(result.data?.warnings).toEqual([
|
||||
'The session is open, but the initial interactive snapshot failed (COMMAND_FAILED: runner crashed mid-capture). Run: agent-device snapshot -i',
|
||||
]);
|
||||
}
|
||||
});
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import { normalizeError, type NormalizedError } from '@agent-device/kernel/errors';
|
||||
import { resolveSoleForegroundIosApp } from '../ios-app-session-hint.ts';
|
||||
import { dispatchSnapshotViaRuntime } from '../snapshot-runtime.ts';
|
||||
import type { SessionStore } from '../session-store.ts';
|
||||
@@ -53,6 +54,27 @@ export async function resolveForegroundOpenRequest(params: {
|
||||
),
|
||||
};
|
||||
}
|
||||
// #1670 P1: the resolved-device rewrite below pins --udid/--platform, so an
|
||||
// explicit selector must fail fast instead of being silently overwritten
|
||||
// (open --foreground --udid B with sim A sole-booted must NOT open A).
|
||||
if (req.flags?.udid !== undefined || req.flags?.device !== undefined) {
|
||||
return {
|
||||
type: 'response',
|
||||
response: errorResponse(
|
||||
'INVALID_ARGS',
|
||||
'open --foreground resolves the device itself; drop --udid/--device or open explicitly: agent-device open <app> --udid <udid>.',
|
||||
),
|
||||
};
|
||||
}
|
||||
if (req.flags?.platform !== undefined && req.flags.platform !== 'ios') {
|
||||
return {
|
||||
type: 'response',
|
||||
response: errorResponse(
|
||||
'INVALID_ARGS',
|
||||
'open --foreground only supports --platform ios; drop the flag or pass --platform ios.',
|
||||
),
|
||||
};
|
||||
}
|
||||
|
||||
const resolved = await resolveSoleForegroundIosApp({
|
||||
simulatorSetPath: req.flags?.iosSimulatorDeviceSet,
|
||||
@@ -92,6 +114,12 @@ export async function resolveForegroundOpenRequest(params: {
|
||||
*
|
||||
* A no-op when `--foreground` was not requested, or when open itself failed (nothing to
|
||||
* attach a snapshot to).
|
||||
*
|
||||
* #1670 P1: a capture failure must never mask the successful open — the session exists
|
||||
* either way, and returning the capture error made a retry of `open --foreground` fail
|
||||
* with "close the current session first". Open success + snapshot failure returns
|
||||
* `ok: true` with an explicit `initialSnapshotError` detail and a rendered warning
|
||||
* telling the caller the session IS open and how to capture manually.
|
||||
*/
|
||||
export async function composeOpenWithInitialSnapshot(params: {
|
||||
req: DaemonRequest;
|
||||
@@ -103,16 +131,59 @@ export async function composeOpenWithInitialSnapshot(params: {
|
||||
const { req, sessionName, logPath, sessionStore, openResponse } = params;
|
||||
if (!openResponse.ok || req.flags?.foreground !== true) return openResponse;
|
||||
|
||||
const snapshotResponse = await dispatchSnapshotViaRuntime({
|
||||
req: { ...req, command: 'snapshot', positionals: [] },
|
||||
sessionName,
|
||||
logPath,
|
||||
sessionStore,
|
||||
});
|
||||
if (!snapshotResponse.ok) return snapshotResponse;
|
||||
try {
|
||||
const snapshotResponse = await dispatchSnapshotViaRuntime({
|
||||
req: {
|
||||
...req,
|
||||
command: 'snapshot',
|
||||
positionals: [],
|
||||
// The promised composition IS `snapshot -i`: the CLI maps `-i` to
|
||||
// `snapshotInteractiveOnly` (flag-definitions-workflow.ts), which is the
|
||||
// key the snapshot runtime reads as `interactiveOnly`.
|
||||
flags: { ...req.flags, snapshotInteractiveOnly: true },
|
||||
},
|
||||
sessionName,
|
||||
logPath,
|
||||
sessionStore,
|
||||
});
|
||||
if (!snapshotResponse.ok) {
|
||||
return openWithInitialSnapshotFailure(openResponse.data, snapshotResponse.error);
|
||||
}
|
||||
return {
|
||||
ok: true,
|
||||
data: { ...openResponse.data, snapshot: snapshotResponse.data },
|
||||
};
|
||||
} catch (error) {
|
||||
// The dispatch can also THROW (capture/runner exceptions are rethrown after
|
||||
// the runtime's own handling). An escaped rejection would fail the whole
|
||||
// open after the session was created — the exact masking this composition
|
||||
// exists to prevent — so a thrown capture failure gets the same
|
||||
// successful-open contract as a returned one.
|
||||
return openWithInitialSnapshotFailure(openResponse.data, normalizeError(error));
|
||||
}
|
||||
}
|
||||
|
||||
function openWithInitialSnapshotFailure(
|
||||
openData: Record<string, unknown> | undefined,
|
||||
error: NormalizedError,
|
||||
): DaemonResponse {
|
||||
return {
|
||||
ok: true,
|
||||
data: { ...openResponse.data, snapshot: snapshotResponse.data },
|
||||
data: {
|
||||
...openData,
|
||||
warnings: [
|
||||
...readStringWarnings(openData),
|
||||
`The session is open, but the initial interactive snapshot failed (${error.code}: ${error.message}). Run: agent-device snapshot -i`,
|
||||
],
|
||||
// The FULL error shape (hint/details/diagnosticId/logPath), not a
|
||||
// code+message truncation — recovery guidance must survive to the
|
||||
// public Node/CLI JSON surfaces.
|
||||
initialSnapshotError: error,
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
function readStringWarnings(data: Record<string, unknown> | undefined): string[] {
|
||||
if (!data || !Array.isArray(data.warnings)) return [];
|
||||
return data.warnings.filter((warning): warning is string => typeof warning === 'string');
|
||||
}
|
||||
|
||||
@@ -165,6 +165,7 @@ export function serializeOpenResult(result: AppOpenResult): Record<string, unkno
|
||||
...(result.runtime ? { runtime: result.runtime } : {}),
|
||||
...(result.device ? serializeSessionDevice(result.device) : {}),
|
||||
...(result.snapshot ? { snapshot: result.snapshot } : {}),
|
||||
...(result.initialSnapshotError ? { initialSnapshotError: result.initialSnapshotError } : {}),
|
||||
},
|
||||
target ? `Opened: ${target}` : 'Opened',
|
||||
);
|
||||
|
||||
Reference in New Issue
Block a user