From 82520804b6bab58933816449beba44a84600dee5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Mon, 29 Jun 2026 18:56:43 +0200 Subject: [PATCH] =?UTF-8?q?feat:=20typed=20errors=20=E2=80=94=20supportedO?= =?UTF-8?q?n=20+=20retriable=20signals=20=E2=80=94=20Phase=202?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Graft the Phase 2 TypedError signals onto DaemonError: machine-readable hints an agent can act on without a wasted round-trip. - supportedOn: for UNSUPPORTED_OPERATION / UNSUPPORTED_PLATFORM errors, the platform families that DO support the command, DERIVED from the capability matrix (new supportedPlatformsForCommand in core/capabilities.ts). So an agent that hit a platform mismatch learns where the command works. - retriable: flags transient failures worth retrying. Conservative policy (retriableForErrorCode in utils/errors.ts) — currently only DEVICE_IN_USE (device healthy but busy); ambiguous/deterministic codes stay undefined. Both are additive and applied at a single chokepoint (handleRequest, where the command is in scope) only when a signal applies, so the default error wire shape is unchanged for the common codes — verified across 1632 daemon/core/contracts/ utils/commands tests (no error-shape ripple). New unit + router e2e test covers all three cases (supportedOn present, retriable true, deterministic unchanged). Verified: tsc --noEmit, oxfmt + oxlint --deny-warnings, fallow audit clean, Layering Guard empty. --- src/contracts.ts | 12 ++ src/core/capabilities.ts | 19 +++ .../request-router-typed-error.test.ts | 123 ++++++++++++++++++ src/daemon/request-router.ts | 43 ++++-- src/utils/errors.ts | 18 +++ 5 files changed, 207 insertions(+), 8 deletions(-) create mode 100644 src/daemon/__tests__/request-router-typed-error.test.ts diff --git a/src/contracts.ts b/src/contracts.ts index e67504778..c7b45e693 100644 --- a/src/contracts.ts +++ b/src/contracts.ts @@ -123,6 +123,18 @@ export type DaemonError = { diagnosticId?: string; logPath?: string; details?: Record; + /** + * Machine-readable typed-error signals (Phase 2). Additive: present only when + * derivable, so the default error wire shape is unchanged. + * + * `retriable` flags a transient failure an agent should retry (vs. a + * deterministic one where a retry is wasted). `supportedOn` lists the platform + * families that DO support the command (derived from the capability matrix), + * surfaced on platform-mismatch errors so an agent self-corrects without a + * wasted round-trip. + */ + retriable?: boolean; + supportedOn?: string; }; export type DaemonResponse = diff --git a/src/core/capabilities.ts b/src/core/capabilities.ts index 79b27a2d5..8cffde35c 100644 --- a/src/core/capabilities.ts +++ b/src/core/capabilities.ts @@ -101,3 +101,22 @@ export function unsupportedHintForDevice(command: string, device: DeviceInfo): s export function listCapabilityCommands(): string[] { return Object.keys(COMMAND_CAPABILITY_MATRIX).sort(); } + +/** + * The platform families that DO support `command`, derived from the capability + * matrix (a family counts when it has at least one supported device kind). Used + * by the typed-error graft to populate `DaemonError.supportedOn` on platform + * mismatches. Returns `[]` for commands with no capability row (supported + * everywhere) so callers can omit the signal. + */ +export function supportedPlatformsForCommand(command: string): string[] { + const capability = COMMAND_CAPABILITY_MATRIX[command]; + if (!capability) return []; + const families: Array = ['apple', 'android', 'linux', 'web']; + const supported: string[] = []; + for (const family of families) { + const kinds = capability[family] as KindMatrix | undefined; + if (kinds && Object.values(kinds).some((value) => value === true)) supported.push(family); + } + return supported; +} diff --git a/src/daemon/__tests__/request-router-typed-error.test.ts b/src/daemon/__tests__/request-router-typed-error.test.ts new file mode 100644 index 000000000..eb54821cb --- /dev/null +++ b/src/daemon/__tests__/request-router-typed-error.test.ts @@ -0,0 +1,123 @@ +import { test, expect, vi, beforeEach } from 'vitest'; +import os from 'node:os'; +import path from 'node:path'; + +vi.mock('../../core/dispatch.ts', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, dispatchCommand: vi.fn(async () => ({})) }; +}); + +vi.mock('../../platforms/ios/runner-client.ts', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, stopIosRunnerSession: vi.fn(async () => {}) }; +}); + +vi.mock('../device-ready.ts', () => ({ ensureDeviceReady: vi.fn(async () => {}) })); + +import { dispatchCommand } from '../../core/dispatch.ts'; +import { createRequestHandler } from '../request-router.ts'; +import type { DaemonRequest, SessionState } from '../types.ts'; +import { LeaseRegistry } from '../lease-registry.ts'; +import { makeSessionStore } from '../../__tests__/test-utils/store-factory.ts'; +import { AppError, retriableForErrorCode } from '../../utils/errors.ts'; +import { supportedPlatformsForCommand } from '../../core/capabilities.ts'; + +const mockDispatch = vi.mocked(dispatchCommand); + +function makeIosSession(name: string): SessionState { + return { + name, + createdAt: 1_700_000_000_000, + actions: [], + device: { + platform: 'ios', + target: 'mobile', + id: 'SIM-001', + name: 'iPhone 16', + kind: 'simulator', + booted: true, + simulatorSetPath: '/tmp/tenant-a/set', + }, + }; +} + +function makeHandler(sessionStore = makeSessionStore('agent-device-router-typed-error-')) { + return { + sessionStore, + handler: createRequestHandler({ + logPath: path.join(os.tmpdir(), 'daemon.log'), + token: 'test-token', + sessionStore, + leaseRegistry: new LeaseRegistry(), + trackDownloadableArtifact: () => 'artifact-id', + }), + }; +} + +function request(command: string, overrides: Partial = {}): DaemonRequest { + return { + token: 'test-token', + session: 'typed-error', + command, + positionals: [], + flags: {}, + ...overrides, + }; +} + +beforeEach(() => { + mockDispatch.mockReset(); +}); + +test('retriableForErrorCode is a conservative policy: transient => true, others => undefined', () => { + expect(retriableForErrorCode('DEVICE_IN_USE')).toBe(true); + expect(retriableForErrorCode('INVALID_ARGS')).toBeUndefined(); + expect(retriableForErrorCode('UNSUPPORTED_OPERATION')).toBeUndefined(); + expect(retriableForErrorCode('COMMAND_FAILED')).toBeUndefined(); +}); + +test('UNSUPPORTED_OPERATION errors carry supportedOn derived from the capability matrix', async () => { + const { sessionStore, handler } = makeHandler(); + sessionStore.set('typed-error', makeIosSession('typed-error')); + mockDispatch.mockRejectedValue(new AppError('UNSUPPORTED_OPERATION', 'nope on this platform')); + + // `home` routes through the (mocked) generic dispatch and is platform-restricted. + const response = await handler(request('home')); + + expect(response.ok).toBe(false); + if (response.ok) return; + const expected = supportedPlatformsForCommand('home'); + expect(expected.length).toBeGreaterThan(0); // home is a platform-restricted command + expect(response.error.supportedOn).toBe(expected.join(', ')); +}); + +test('DEVICE_IN_USE errors are flagged retriable; supportedOn stays absent', async () => { + const { sessionStore, handler } = makeHandler(); + sessionStore.set('typed-error', makeIosSession('typed-error')); + mockDispatch.mockRejectedValue(new AppError('DEVICE_IN_USE', 'device busy')); + + const response = await handler(request('home')); + + expect(response.ok).toBe(false); + if (response.ok) return; + expect(response.error.retriable).toBe(true); + expect('supportedOn' in response.error).toBe(false); +}); + +test('deterministic errors (INVALID_ARGS) are returned with the default shape — no typed-error fields', async () => { + const { sessionStore, handler } = makeHandler(); + sessionStore.set('typed-error', makeIosSession('typed-error')); + + // Conflicting explicit selector under a reject lock policy fails with INVALID_ARGS + // before dispatch — a deterministic error. + const response = await handler( + request('home', { flags: { udid: 'SIM-999' }, meta: { lockPolicy: 'reject' } }), + ); + + expect(response.ok).toBe(false); + if (response.ok) return; + expect(response.error.code).toBe('INVALID_ARGS'); + expect('retriable' in response.error).toBe(false); + expect('supportedOn' in response.error).toBe(false); + expect(mockDispatch).not.toHaveBeenCalled(); +}); diff --git a/src/daemon/request-router.ts b/src/daemon/request-router.ts index 1b0d18c23..1995e3e81 100644 --- a/src/daemon/request-router.ts +++ b/src/daemon/request-router.ts @@ -2,9 +2,10 @@ import { type DeviceInventoryProvider, withTargetDeviceResolutionScope, } from '../core/dispatch-resolve.ts'; -import { AppError, normalizeError } from '../utils/errors.ts'; +import { AppError, normalizeError, retriableForErrorCode } from '../utils/errors.ts'; +import { supportedPlatformsForCommand } from '../core/capabilities.ts'; import { timingSafeStringEqual } from '../utils/timing-safe-equal.ts'; -import type { ResponseCost } from '../contracts.ts'; +import type { DaemonError, ResponseCost } from '../contracts.ts'; import type { DaemonInvokeFn, DaemonRequest, DaemonResponse } from './types.ts'; import { SessionStore } from './session-store.ts'; import { noActiveSessionError } from './handlers/response.ts'; @@ -102,13 +103,21 @@ export function createRequestHandler(deps: RequestRouterDeps): DaemonInvokeFn { }, async () => { const response = await runRequestWithinScope(req); + // Phase 2 (typed errors) graft: enrich error responses with additive, + // machine-readable signals — `supportedOn` for platform mismatches and + // `retriable` for transient failures — so an agent self-corrects without a + // wasted round-trip. Returned unchanged when neither applies, so the + // default error wire shape is preserved. + if (!response.ok) { + return { ok: false, error: enrichDaemonError(req.command, response.error) }; + } // Phase 4 (agent-cost) graft: cost is purely additive and opt-in. With - // the flag off — or on an error response — the serialized DaemonResponse - // is byte-identical to today (Maestro `.ad` recompare diffs it). Mirrors - // the conditional `registerDownloadableArtifacts` spread in - // request-finalization. Runs inside the diagnostics scope so it can read - // this request's accumulated runner-round-trip tally. - if (!req.meta?.includeCost || !response.ok) return response; + // the flag off the serialized DaemonResponse is byte-identical to today + // (Maestro `.ad` recompare diffs it). Mirrors the conditional + // `registerDownloadableArtifacts` spread in request-finalization. Runs + // inside the diagnostics scope so it can read this request's accumulated + // runner-round-trip tally. + if (!req.meta?.includeCost) return response; const cost: ResponseCost = { wallClockMs: Date.now() - start, runnerRoundTrips: countDiagnosticEventsByPhase(RUNNER_ROUND_TRIP_PHASES), @@ -301,3 +310,21 @@ function finalizeThrownRequestError(error: unknown): DaemonResponse { }); return { ok: false, error: normalizedError }; } + +// Phase 2 typed-error graft: add machine-readable signals to an error response. +// Returns the error unchanged unless a signal applies, so the default wire shape +// is preserved for the common codes. +function enrichDaemonError(command: string, error: DaemonError): DaemonError { + const supportedPlatforms = + error.code === 'UNSUPPORTED_OPERATION' || error.code === 'UNSUPPORTED_PLATFORM' + ? supportedPlatformsForCommand(command) + : []; + const supportedOn = supportedPlatforms.length > 0 ? supportedPlatforms.join(', ') : undefined; + const retriable = retriableForErrorCode(error.code); + if (supportedOn === undefined && retriable === undefined) return error; + return { + ...error, + ...(retriable !== undefined ? { retriable } : {}), + ...(supportedOn !== undefined ? { supportedOn } : {}), + }; +} diff --git a/src/utils/errors.ts b/src/utils/errors.ts index c63cd557e..e6e6f0a15 100644 --- a/src/utils/errors.ts +++ b/src/utils/errors.ts @@ -144,6 +144,24 @@ function stripDiagnosticMeta( return Object.keys(output).length > 0 ? output : undefined; } +/** + * Conservative retriability policy for the Phase 2 typed-error graft. Returns + * `true` only for codes that are clearly transient (a retry can succeed without + * the caller changing anything) and `undefined` for ambiguous/deterministic + * codes — so the error wire shape is unchanged unless we have a confident answer. + * Intentionally small; extend as codes gain a clear retriability verdict. + */ +export function retriableForErrorCode(code: string): boolean | undefined { + switch (code) { + // The device is healthy but currently leased/busy — the same request can + // succeed once it frees up. + case 'DEVICE_IN_USE': + return true; + default: + return undefined; + } +} + export function defaultHintForCode(code: string): string | undefined { switch (code) { case 'INVALID_ARGS':