fix: carry response warnings to every surface, and stamp the sheet behind a refusal

CHANGELOG.md

@@ -139,6 +139,19 @@
   exactly as before, and a payload the runner did not declare sparse keeps the plain invariant byte for
   byte.
 
+- Fixed: skipped `optional: true` Maestro steps are no longer invisible on the human surface
+  (#2560). The warning a skip leaves now travels with the run whether it later passes or fails:
+  `replay` prints a `Warning:` line after its summary and repeats the run's warnings after a
+  failed run's error, `test` prints a `Warnings:` section after the suite summary naming each
+  test, and a failed test result gained the `warnings` array the passing result already had, so
+  `--json` and JUnit carry the skipped steps of a failing test too.
+- Fixed: an Apple runner presentation refusal now carries the registry identity of the system
+  surface the tree was acquired from as `error.details.systemSurface` (#2560). Previously a
+  selector-backed command failing at the capture boundary under `optional: true` gave nothing
+  naming that boundary — the web sign-in sheet out of `SafariViewService` was invisible in the
+  error, and the miss looked like a selector problem inside the app. The sparse-declared case
+  already names the surface host in its hint via #2572; this covers the provenance everywhere
+  the runner reports one.
 - Changed: a capture that a backend cut at one of its limits now says so in the snapshot's
   warnings, on every platform, instead of only setting `truncated: true` in JSON. The text path
   had no disclosure at all, so an agent read a screen missing its footer, tab bar, or the items

packages/contracts/src/replay.ts

@@ -108,6 +108,8 @@ export type ReplaySuiteTestFailed = {
   attempts: number;
   artifactsDir?: string;
   error: DaemonError;
+  /** Warnings accumulated before the failing step (skipped `optional` steps, capture degradations). */
+  warnings?: string[];
   /** Present when the owning runtime classified the failure as device/runner infrastructure. */
   infrastructure?: true;
   shardIndex?: number;

packages/contracts/src/snapshot-capture-annotations.test.ts

@@ -0,0 +1,31 @@
+import assert from 'node:assert/strict';
+import { test } from 'vitest';
+import { readResponseWarnings } from '@agent-device/kernel/success-text';
+import { readSerializedSnapshotCaptureAnnotations } from './snapshot-capture-annotations.ts';
+
+test('the annotations filter and the shared warnings parser agree on adversarial arrays', () => {
+  for (const warnings of [
+    ['a note'],
+    ['a note', 42, { nested: true }, null],
+    ['', 'kept'],
+    ['a note', ''],
+  ]) {
+    assert.deepEqual(
+      readSerializedSnapshotCaptureAnnotations({ warnings }).warnings,
+      readResponseWarnings({ warnings }),
+      `drift for ${JSON.stringify(warnings)}`,
+    );
+  }
+});
+
+test('an empty warnings array serializes back to absent', () => {
+  assert.equal(readSerializedSnapshotCaptureAnnotations({ warnings: [] }).warnings, undefined);
+});
+
+test('absent or non-array warnings stay absent on the serialized annotations', () => {
+  assert.equal(readSerializedSnapshotCaptureAnnotations({}).warnings, undefined);
+  assert.equal(
+    readSerializedSnapshotCaptureAnnotations({ warnings: 'a note' }).warnings,
+    undefined,
+  );
+});

packages/contracts/src/snapshot-capture-annotations.ts

@@ -57,6 +57,9 @@ export function readSerializedSnapshotCaptureAnnotations(
   data: Record<string, unknown>,
 ): PublicSnapshotCaptureAnnotations {
   const androidSnapshot = readObject(data.androidSnapshot);
+  // Declared exception to kernel's shared `readResponseWarnings` (see its doc): this facade
+  // pins its eager module closure, and absent-or-non-array keeps the serialized tri-state.
+  // `snapshot-capture-annotations.test.ts` cross-checks this filter against the shared parser.
   const warnings = Array.isArray(data.warnings)
     ? data.warnings.filter((entry): entry is string => typeof entry === 'string')
     : undefined;

packages/kernel/src/success-text.test.ts

@@ -0,0 +1,12 @@
+import assert from 'node:assert/strict';
+import { describe, test } from 'vitest';
+import { readCommandMessage } from './success-text.ts';
+
+describe('readCommandMessage', () => {
+  test('an empty message is absent, not an empty success line', () => {
+    assert.equal(readCommandMessage({ message: '' }), null);
+    assert.equal(readCommandMessage({ message: 42 }), null);
+    assert.equal(readCommandMessage(undefined), null);
+    assert.equal(readCommandMessage({ message: 'Replayed 7 steps' }), 'Replayed 7 steps');
+  });
+});

packages/kernel/src/success-text.ts

@@ -12,3 +12,25 @@ export function withSuccessText<T extends Record<string, unknown>>(
 export function readCommandMessage(data: Record<string, unknown> | undefined): string | null {
   return typeof data?.message === 'string' && data.message.length > 0 ? data.message : null;
 }
+
+/**
+ * The composable response-warnings channel (skipped `optional` steps, capture
+ * degradations): readers that project a response or error record onto note
+ * strings go through here — daemon append, attempt outcome, and
+ * recovered-quality latch, CLI success line, CLI/MCP error text, SDK client
+ * (open and screenshot result), and the
+ * snapshot text renderer — so one field contract has one parser. Consumers may
+ * add rendering rules on top (snapshot text and screenshot result drop empty
+ * notes; screenshot result keeps absent-means-undefined). The one declared
+ * exception is contracts' `readSerializedSnapshotCaptureAnnotations`, which
+ * keeps a local copy of the filter: contracts facades pin their eager module
+ * closure (`scripts/__tests__/eager-closure-budgets.test.ts`) and this module
+ * is outside it; its test cross-checks both parses so the contract cannot
+ * drift. Non-string entries are other producers' bugs.
+ */
+export function readResponseWarnings(data: Record<string, unknown> | undefined): string[] {
+  const warnings = data?.warnings;
+  return Array.isArray(warnings)
+    ? warnings.filter((warning): warning is string => typeof warning === 'string')
+    : [];
+}

packages/maestro/src/internal/__tests__/engine.test.ts

@@ -334,6 +334,38 @@ describe('executeMaestroProgram', () => {
     expect(observer.commandCompleted).toHaveBeenCalledOnce();
   });
 
+  test('failure events carry warnings accumulated before the failing step (#2560)', async () => {
+    const execute = vi.fn(async (request: MaestroRuntimeRequest) => {
+      request.invalidateObservation();
+      throw new AppError('COMMAND_FAILED', 'leaf command failed');
+    });
+    const port = makePort({
+      observe: vi.fn(async ({ generation }) => ({ generation, matched: false })),
+      execute,
+    });
+    const observer = { commandFailed: vi.fn() };
+    const program = parseMaestroProgram(
+      [
+        '---',
+        '- assertVisible:',
+        '    text: Missing assertion',
+        '    optional: true',
+        '- tapOn: Missing target',
+      ].join('\n'),
+    );
+
+    await expect(executeMaestroProgram(program, port, { observer })).rejects.toThrow(
+      'leaf command failed',
+    );
+
+    expect(observer.commandFailed).toHaveBeenCalledWith(
+      expect.objectContaining({
+        command: expect.objectContaining({ kind: 'tapOn' }),
+        warnings: [expect.stringMatching(/Optional Maestro assertVisible skipped at line 2/)],
+      }),
+    );
+  });
+
   test('observer failure cannot mask nested leaf failure provenance', async () => {
     const execute = vi.fn(async (request: MaestroRuntimeRequest) => {
       request.invalidateObservation();

packages/maestro/src/internal/engine-types.ts

@@ -138,6 +138,8 @@ export type MaestroEngineObserver = {
       runtimeMetrics?: MaestroRuntimeMetrics;
       error: unknown;
       artifactPaths: readonly string[];
+      /** Warnings accumulated before this failure, including skipped `optional` steps. */
+      warnings: readonly string[];
     },
   ): void;
 };

packages/maestro/src/internal/facade-execution.ts

@@ -59,6 +59,8 @@ export type MaestroFailedAction = MaestroActionEvent & {
   readonly runtimeMetrics?: MaestroCompletedActionEvent['runtimeMetrics'];
   readonly error: unknown;
   readonly artifactPaths: readonly string[];
+  /** Warnings accumulated before this failure, including skipped `optional` steps. */
+  readonly warnings: readonly string[];
   readonly isControl: boolean;
   readonly redactions: readonly { name: string; value: string }[];
   readonly resume:
@@ -215,6 +217,7 @@ function createObserver(
         durationMs: event.durationMs,
         error: event.error,
         artifactPaths: event.artifactPaths,
+        warnings: [...event.warnings],
         isControl: isMaestroControlCommandDescriptor(event.command),
         redactions:
           event.command.kind === 'inputText' && event.command.text.length > 0

packages/maestro/src/internal/replay-plan-execution.ts

@@ -96,6 +96,7 @@ async function executeObservedStep(
         ...runtimeMetricsDelta(metricsBefore, state.port.readMetrics?.()),
         error: failure.error,
         artifactPaths: [...state.artifacts],
+        warnings: [...state.warnings],
       }),
     );
     throw failure;

packages/platform-apple/src/runner/__tests__/snapshot-presentation.test.ts

@@ -1,8 +1,10 @@
 import { AppError } from '@agent-device/kernel/errors';
 import assert from 'node:assert/strict';
 import { test } from 'vitest';
+import type { RawSnapshotNode } from '@agent-device/kernel/snapshot';
+import type { IosSystemSurfaceProvenance } from '@agent-device/contracts/ios-system-surface';
 import type { AppleRunnerSnapshotResult } from '../snapshot-presentation.ts';
-import { presentAppleRunnerSnapshot } from '../snapshot-presentation.ts';
+import { presentAppleRunnerSnapshot, readAppleSnapshotResult } from '../snapshot-presentation.ts';
 
 const NO_VIEWPORT_ROOT = { index: 0, type: 'Application', label: 'App' };
 
@@ -73,6 +75,10 @@ test('a sparse capture of a presented system surface names the surface host', ()
     String(error.details?.hint),
     /com\.apple\.SafariViewService hosts the surface presented over the app/,
   );
+  assert.deepEqual(error.details?.systemSurface, {
+    bundleId: 'com.apple.SafariViewService',
+    kind: 'web-auth',
+  });
 });
 
 test('a sparse payload failing another invariant still carries the verdict', () => {
@@ -125,3 +131,61 @@ test('a sparse verdict still presents the nodes it did read', () => {
     ['App', 'Not Now'],
   );
 });
+
+const SYSTEM_SHEET: IosSystemSurfaceProvenance = {
+  bundleId: 'com.apple.SafariViewService',
+  kind: 'web-auth',
+};
+
+test('a presented system surface travels with an undeclared-payload refusal as typed provenance', () => {
+  try {
+    presentAppleRunnerSnapshot('device-1', undefined, {
+      nodes: [NO_VIEWPORT_ROOT],
+      quality: { state: 'healthy', backend: 'tree' },
+      systemSurface: SYSTEM_SHEET,
+    });
+  } catch (error) {
+    assert.ok(error instanceof AppError);
+    assert.deepEqual((error.details as Record<string, unknown>).systemSurface, SYSTEM_SHEET);
+    assert.equal((error.details as Record<string, unknown>).snapshotQuality, undefined);
+    return;
+  }
+  assert.fail('expected the presentation to refuse the payload');
+});
+
+test('readAppleSnapshotResult keeps registry-known system surface provenance only', () => {
+  const known = readAppleSnapshotResult({
+    nodes: [],
+    systemSurface: { bundleId: 'com.apple.SafariViewService', kind: 'web-auth' },
+  });
+  assert.deepEqual(known.systemSurface, SYSTEM_SHEET);
+
+  const unknown = readAppleSnapshotResult({
+    nodes: [],
+    systemSurface: { bundleId: 'com.example.PhishingService', kind: 'web-auth' },
+  });
+  assert.equal(unknown.systemSurface, undefined);
+});
+
+test('a healthy payload with valid viewport roots still presents', () => {
+  const screen: RawSnapshotNode = {
+    index: 0,
+    type: 'Application',
+    rect: { x: 0, y: 0, width: 390, height: 844 },
+  };
+  const button: RawSnapshotNode = {
+    index: 1,
+    parentIndex: 0,
+    type: 'Button',
+    label: 'Not Now',
+    rect: { x: 16, y: 400, width: 80, height: 32 },
+    hittable: true,
+  };
+  const nodes = presentAppleRunnerSnapshot('device-1', undefined, {
+    nodes: [screen, button],
+  });
+  assert.deepEqual(
+    nodes.map((node) => node.index),
+    [0, 1],
+  );
+});

packages/platform-apple/src/runner/snapshot-presentation.ts

@@ -45,7 +45,7 @@ export function readAppleSnapshotResult(
     nodes: Array.isArray(result.nodes) ? (result.nodes as RawSnapshotNode[]) : undefined,
     truncated: typeof result.truncated === 'boolean' ? result.truncated : undefined,
     quality: readSnapshotQualityVerdict(result.snapshotQuality),
-    qualityPayload: readQualityPayload(result.qualityPayload),
+    qualityPayload: readQualityPayload(result.qualityPayload, systemSurface),
     runnerFatal: result.runnerFatal === true,
     ...(systemSurface ? { systemSurface } : {}),
     message:
@@ -131,6 +131,7 @@ function throwSnapshotPresentationError(error: unknown, result: AppleRunnerSnaps
       error.message,
       {
         ...toIosSnapshotEngineErrorDetails(error),
+        ...(result.systemSurface ? { systemSurface: result.systemSurface } : {}),
         snapshotQuality: {
           state: verdict.state,
           backend: verdict.backend,
@@ -142,7 +143,7 @@ function throwSnapshotPresentationError(error: unknown, result: AppleRunnerSnaps
       error,
     );
   }
-  throwSnapshotEngineError(error);
+  throwSnapshotEngineError(error, result.systemSurface);
 }
 
 function sparseCaptureHint(
@@ -159,14 +160,18 @@ function sparseCaptureHint(
     .join(' ');
 }
 
-function readQualityPayload(value: unknown): IosRunnerQualityPayloadFacts | undefined {
+function readQualityPayload(
+  value: unknown,
+  systemSurface?: IosSystemSurfaceProvenance,
+): IosRunnerQualityPayloadFacts | undefined {
   if (value === undefined) return undefined;
   if (!isRecord(value) || !Array.isArray(value.nodes) || typeof value.truncated !== 'boolean') {
     throwSnapshotEngineError(
       new IosSnapshotEngineError(
         'invalid-quality-payload',
         'iOS runner returned an invalid quality payload',
       ),
+      systemSurface,
     );
   }
   if (value.scope !== undefined && value.scope !== null) {
@@ -176,6 +181,7 @@ function readQualityPayload(value: unknown): IosRunnerQualityPayloadFacts | unde
         'iOS runner quality payload must be unscoped',
         { field: 'scope' },
       ),
+      systemSurface,
     );
   }
   return { nodes: value.nodes as RawSnapshotNode[], truncated: value.truncated, scope: null };
@@ -204,12 +210,18 @@ function isRecord(value: unknown): value is Record<string, unknown> {
   return typeof value === 'object' && value !== null && !Array.isArray(value);
 }
 
-function throwSnapshotEngineError(error: unknown): never {
+function throwSnapshotEngineError(
+  error: unknown,
+  systemSurface?: IosSystemSurfaceProvenance,
+): never {
   if (!(error instanceof IosSnapshotEngineError)) throw error;
   throw new AppError(
     'COMMAND_FAILED',
     error.message,
-    toIosSnapshotEngineErrorDetails(error),
+    {
+      ...toIosSnapshotEngineErrorDetails(error),
+      ...(systemSurface ? { systemSurface } : {}),
+    },
     error,
   );
 }

packages/replay-test/src/internal/__tests__/session-test-artifacts.test.ts

@@ -106,6 +106,7 @@ test('materializeReplayTestAttemptArtifacts writes failure manifest and copies l
         details: { reason: 'timeout', artifactPaths: [screenshotPath] },
       },
       artifactPaths: [screenshotPath],
+      warnings: [],
       infrastructure: false,
     },
     filePath: replayPath,
@@ -189,6 +190,7 @@ test('materialization preserves replay sources and diagnostics named after attem
       status: 'failed',
       error: { code: 'COMMAND_FAILED', message: 'original failure' },
       artifactPaths,
+      warnings: [],
       infrastructure: false,
     },
     filePath: replayPath,
@@ -267,6 +269,7 @@ test('materialization copies a log listed in both the outcome and error only onc
       status: 'failed',
       error: { code: 'COMMAND_FAILED', message: 'failed', logPath },
       artifactPaths: [logPath, logPath],
+      warnings: [],
       infrastructure: false,
     },
     filePath: replayPath,
@@ -300,6 +303,7 @@ test.each(['result.txt', 'failure.txt', 'RESULT.TXT'])(
         status: 'failed',
         error: { code: 'COMMAND_FAILED', message: 'original failure' },
         artifactPaths: [diagnosticPath],
+        warnings: [],
         infrastructure: false,
       },
       filePath: replayPath,