From 10c382c0292ebfc5bba98ae3bd95f101fcaabd0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Thu, 24 Sep 2026 13:34:00 +0200 Subject: [PATCH 1/5] refactor(ios-runner): close the snapshot quality verdict state vocabulary The runner wrote the verdict `state` as a free String while the host accepts only healthy | recovered | sparse, and the contracts annotation reader cast any string state into the verdict type. A typo or a one-sided rename therefore dropped the verdict and its disclosure. `SnapshotQualityState` now owns the runner side, raw-value Codable keeps the wire JSON unchanged, and `reasonCode` stays open. The kernel states the vocabulary once as `SNAPSHOT_QUALITY_STATES` with `isSnapshotQualityState`; capture-kit and contracts both read through it, so neither keeps a second accepted-state set and the annotation reader rejects an unknown state instead of casting it. `contracts/fixtures/ios-snapshot-quality-states.json` is the table the Swift `allCases` order and the kernel tuple are each pinned to. --- .../RunnerTests+Snapshot.swift | 2 +- .../RunnerTests+SnapshotCapturePlan.swift | 26 +++-- ...ts+SnapshotCapturePlanOccupancyTests.swift | 4 +- ...RunnerTests+SnapshotCapturePlanTests.swift | 22 ++--- ...unnerTests+SnapshotQualityStateTests.swift | 94 +++++++++++++++++++ .../UnitTests/RunnerTests+SnapshotTests.swift | 2 +- .../fixtures/ios-snapshot-quality-states.json | 1 + .../src/snapshot-quality-verdict.test.ts | 14 +++ .../src/snapshot-quality-verdict.ts | 15 +-- .../src/snapshot-capture-annotations.test.ts | 33 +++++++ .../src/snapshot-capture-annotations.ts | 9 +- .../src/snapshot-quality-states.test.ts | 49 ++++++++++ packages/kernel/src/snapshot.ts | 25 ++++- 13 files changed, 256 insertions(+), 40 deletions(-) create mode 100644 apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SnapshotQualityStateTests.swift create mode 100644 contracts/fixtures/ios-snapshot-quality-states.json create mode 100644 packages/kernel/src/snapshot-quality-states.test.ts diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Snapshot.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Snapshot.swift index 3d469bf868..c98c926254 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Snapshot.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Snapshot.swift @@ -542,7 +542,7 @@ extension RunnerTests { return sparseTruncatedSnapshotPayload( message: recoveredSnapshotMessage(failure), snapshotQuality: SnapshotQuality( - state: "sparse", + state: .sparse, backend: SnapshotBackendKind.recursiveTree.rawValue, reason: failure.message, reasonCode: "ax-rejected", diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SnapshotCapturePlan.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SnapshotCapturePlan.swift index e7f3898652..bf56a6e2fd 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SnapshotCapturePlan.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SnapshotCapturePlan.swift @@ -8,11 +8,21 @@ import AgentDeviceSnapshotPresentation // stamp the outcome with a structured quality verdict so the daemon renders state instead of // re-deriving it from node shapes. Recovery ordering is data here, never a per-call-site branch. +/// The closed set of verdict states the host accepts. The wire strings are the shared table at +/// `contracts/fixtures/ios-snapshot-quality-states.json`, which `allCases` is pinned to; `reasonCode` +/// stays open because an unknown one costs only its wording, never the verdict. +enum SnapshotQualityState: String, Codable, CaseIterable { + /// First backend produced a usable tree. + case healthy + /// A later backend did. + case recovered + /// No backend produced a usable tree; the best attempt is returned as-is. + case sparse +} + /// Structured quality verdict shipped with every iOS snapshot payload. struct SnapshotQuality: Codable { - /// healthy: first backend produced a usable tree. recovered: a later backend did. - /// sparse: no backend produced a usable tree; the best attempt is returned as-is. - let state: String + let state: SnapshotQualityState /// Backend that produced the returned payload: tree | queries | private-ax. let backend: String /// Why recovery ran (first failure), why the payload is degraded, or why an internal backend @@ -391,7 +401,7 @@ extension RunnerTests { return stampedSnapshotPayload( capture, backend: kind, - state: recovered ? "recovered" : "healthy", + state: recovered ? .recovered : .healthy, reason: recovered || firstFailure?.code == "requested-backend" ? firstFailure : nil ) } @@ -416,11 +426,11 @@ extension RunnerTests { } let fallbackPayload = - best.map { stampedSnapshotPayload($0.capture, backend: $0.kind, state: "sparse", reason: firstFailure) } + best.map { stampedSnapshotPayload($0.capture, backend: $0.kind, state: .sparse, reason: firstFailure) } ?? stampedSnapshotPayload( SnapshotBackendCapture(payload: sparseTruncatedSnapshotPayload(), effectiveDepth: nil), backend: effectivePlan.last ?? plan.last ?? .recursiveTree, - state: "sparse", + state: .sparse, reason: firstFailure ) return fallbackPayload @@ -680,7 +690,7 @@ extension RunnerTests { func stampedSnapshotPayload( _ capture: SnapshotBackendCapture, backend: SnapshotBackendKind, - state: String, + state: SnapshotQualityState, reason: (reason: String, code: String)? ) -> DataPayload { let health: RunnerAccessibilityHealth = reason?.code == "ax-rejected" ? .unavailable : .healthy @@ -705,7 +715,7 @@ extension RunnerTests { // "recovered") stays untruncated, so strict absence reads can trust it. Only a real cap // (payload truncation, a depth-limited private AX capture) or a sparse terminal payload // is truncated. - truncated: payload.truncated == true || state == "sparse" || capture.effectiveDepth != nil, + truncated: payload.truncated == true || state == .sparse || capture.effectiveDepth != nil, qualityPayload: capture.qualityPayload.flatMap { quality in guard let nodes = quality.nodes else { return nil } return SnapshotQualityPayload(nodes: nodes, truncated: quality.truncated == true) diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SnapshotCapturePlanOccupancyTests.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SnapshotCapturePlanOccupancyTests.swift index edec6e8333..3a220bb594 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SnapshotCapturePlanOccupancyTests.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SnapshotCapturePlanOccupancyTests.swift @@ -149,7 +149,7 @@ extension RunnerTests { XCTAssertNil(box.error) let quality = try XCTUnwrap(box.payload?.snapshotQuality) XCTAssertEqual(quality.backend, SnapshotBackendKind.privateAX.rawValue) - XCTAssertEqual(quality.state, "recovered") + XCTAssertEqual(quality.state, .recovered) XCTAssertTrue( quality.reason?.contains("tree capture exceeded") == true, "the tree XPC, not the viewport read, must be the abandoned block: \(quality.reason ?? "nil")" @@ -345,7 +345,7 @@ extension RunnerTests { SnapshotBackendKind.privateAX.rawValue, "a sweep that ended on its slice deadline is a tier timeout, not an accepted capture" ) - XCTAssertEqual(quality?.state, "recovered") + XCTAssertEqual(quality?.state, .recovered) XCTAssertGreaterThan(box.payload?.nodes?.count ?? 0, 1, "private AX answers with a real tree") XCTAssertFalse(box.abandonedAtReturn, "the sweep must answer inside its own main-thread hop") XCTAssertTrue( diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SnapshotCapturePlanTests.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SnapshotCapturePlanTests.swift index a2c709eeb9..a6f552b1bc 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SnapshotCapturePlanTests.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SnapshotCapturePlanTests.swift @@ -162,7 +162,7 @@ extension RunnerTests { let payload = stampedSnapshotPayload( capture, backend: .recursiveTree, - state: "healthy", + state: .healthy, reason: nil ) @@ -187,7 +187,7 @@ extension RunnerTests { customActions: coverage ), backend: .recursiveTree, - state: "healthy", + state: .healthy, reason: nil ) XCTAssertNil(silent.message) @@ -200,7 +200,7 @@ extension RunnerTests { effectiveDepth: 4 ), backend: .privateAX, - state: "recovered", + state: .recovered, reason: (reason: "tree capture timed out", code: "budget") ) XCTAssertEqual(underlying.message, "underlying") @@ -224,24 +224,24 @@ extension RunnerTests { // The CI signature behind `is absent ... capture was truncated`: a complete private AX // tree selected while the XCTest channel is penalized is whole, and must say so. let recovered = stampedSnapshotPayload( - complete, backend: .privateAX, state: "recovered", reason: deferred) - XCTAssertEqual(recovered.snapshotQuality?.state, "recovered") + complete, backend: .privateAX, state: .recovered, reason: deferred) + XCTAssertEqual(recovered.snapshotQuality?.state, .recovered) XCTAssertEqual(recovered.truncated, false) let depthLimited = stampedSnapshotPayload( SnapshotBackendCapture(payload: complete.payload, effectiveDepth: 56), - backend: .privateAX, state: "recovered", reason: deferred) + backend: .privateAX, state: .recovered, reason: deferred) XCTAssertEqual(depthLimited.truncated, true) let cappedPayload = stampedSnapshotPayload( SnapshotBackendCapture( payload: DataPayload(nodes: complete.payload.nodes ?? [], truncated: true), effectiveDepth: nil), - backend: .recursiveTree, state: "healthy", reason: nil) + backend: .recursiveTree, state: .healthy, reason: nil) XCTAssertEqual(cappedPayload.truncated, true) let sparse = stampedSnapshotPayload( - complete, backend: .querySweep, state: "sparse", + complete, backend: .querySweep, state: .sparse, reason: ("snapshot returned no semantic controls or content", "sparse-tree")) XCTAssertEqual(sparse.truncated, true) } @@ -260,7 +260,7 @@ extension RunnerTests { let payload = stampedSnapshotPayload( capture, backend: .recursiveTree, - state: "healthy", + state: .healthy, reason: nil ) @@ -291,7 +291,7 @@ extension RunnerTests { let payload = stampedSnapshotPayload( capture, backend: .recursiveTree, - state: "healthy", + state: .healthy, reason: nil ) @@ -507,7 +507,7 @@ extension RunnerTests { let quality = try XCTUnwrap(capped.snapshotQuality) XCTAssertEqual(quality.backend, SnapshotBackendKind.privateAX.rawValue) - XCTAssertNotEqual(quality.state, "sparse") + XCTAssertNotEqual(quality.state, .sparse) let nodes = try XCTUnwrap(capped.nodes) XCTAssertGreaterThan(nodes.count, 1) XCTAssertEqual(nodes.map(\.depth).max(), 1) diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SnapshotQualityStateTests.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SnapshotQualityStateTests.swift new file mode 100644 index 0000000000..00bbefe12d --- /dev/null +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SnapshotQualityStateTests.swift @@ -0,0 +1,94 @@ +import XCTest +import AgentDeviceSnapshotPresentation + +#if AGENT_DEVICE_RUNNER_UNIT_TESTS +extension RunnerTests { + private struct StampedWireVerdict: Decodable { + struct Quality: Decodable { + let state: String + } + let snapshotQuality: Quality + } + + private func loadSnapshotQualityStatesFixture() throws -> [String] { + let fixtureURL = URL(fileURLWithPath: #filePath) + .deletingLastPathComponent() // UnitTests + .deletingLastPathComponent() // AgentDeviceRunnerUITests + .deletingLastPathComponent() // AgentDeviceRunner + .deletingLastPathComponent() // runner + .deletingLastPathComponent() // apple + .deletingLastPathComponent() // repo root + .appendingPathComponent("contracts") + .appendingPathComponent("fixtures") + .appendingPathComponent("ios-snapshot-quality-states.json") + return try JSONDecoder().decode([String].self, from: Data(contentsOf: fixtureURL)) + } + + private func wireTestCapture() -> SnapshotBackendCapture { + SnapshotBackendCapture( + payload: DataPayload( + nodes: [ + SnapshotPresentation.singleElementRead( + RawAXNode( + index: 0, + type: "Application", + label: "App", + identifier: nil, + value: nil, + rect: SnapshotRect(.zero), + enabled: true, + focused: nil, + selected: nil, + hittable: true, + depth: 0, + parentIndex: nil, + hiddenContentAbove: nil, + hiddenContentBelow: nil + ) + ) + ], + truncated: false + ), + effectiveDepth: nil + ) + } + + /// The one claim of this file: the runner's closed enum and the shared TypeScript table name the + /// same states in the same order. The kernel's `SNAPSHOT_QUALITY_STATES` is pinned to it too, so + /// the two runtimes cannot drift into a verdict the host drops along with its disclosure. + func testSnapshotQualityStatesMatchSharedWireFixture() throws { + XCTAssertEqual( + try loadSnapshotQualityStatesFixture(), + SnapshotQualityState.allCases.map(\.rawValue), + "update the fixture and the kernel tuple together with the enum" + ) + } + + /// What the daemon receives for each state, taken from the production stamping path rather than a + /// hand-built verdict: the wire string is the fixture's, so a change of representation — an + /// `Int` backing, a nested object, a renamed case — goes red here on the actual payload. + func testStampedVerdictEncodesTheFixtureStateString() throws { + let fixture = try loadSnapshotQualityStatesFixture() + XCTAssertEqual(fixture.count, SnapshotQualityState.allCases.count) + for (index, state) in SnapshotQualityState.allCases.enumerated() { + let payload = stampedSnapshotPayload( + wireTestCapture(), + backend: .recursiveTree, + state: state, + reason: nil + ) + let wire = try JSONDecoder().decode( + StampedWireVerdict.self, + from: JSONEncoder().encode(payload) + ) + XCTAssertEqual(wire.snapshotQuality.state, fixture[index], state.rawValue) + } + } + + /// Closed in both directions: a wire string nobody declared never becomes a verdict. + func testVerdictStateRejectsAnUndeclaredWireString() throws { + let json = Data(#"{"state":"degraded","backend":"tree"}"#.utf8) + XCTAssertThrowsError(try JSONDecoder().decode(SnapshotQuality.self, from: json)) + } +} +#endif diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SnapshotTests.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SnapshotTests.swift index c679bbcbde..20b3b9631e 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SnapshotTests.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SnapshotTests.swift @@ -23,7 +23,7 @@ extension RunnerTests { XCTAssertEqual(payload.runnerFatalReason, Self.axSnapshotUnavailableReason) // The planned terminal result carries the structured verdict like every other planned // snapshot — downstream sparse handling keys off it, not off node shapes. - XCTAssertEqual(payload.snapshotQuality?.state, "sparse") + XCTAssertEqual(payload.snapshotQuality?.state, .sparse) XCTAssertEqual(payload.snapshotQuality?.reasonCode, "ax-rejected") XCTAssertEqual(payload.snapshotQuality?.reason, Self.axSnapshotFailureMessage) XCTAssertNil(currentApp) diff --git a/contracts/fixtures/ios-snapshot-quality-states.json b/contracts/fixtures/ios-snapshot-quality-states.json new file mode 100644 index 0000000000..ab9c625897 --- /dev/null +++ b/contracts/fixtures/ios-snapshot-quality-states.json @@ -0,0 +1 @@ +["healthy", "recovered", "sparse"] diff --git a/packages/capture-kit/src/snapshot-quality-verdict.test.ts b/packages/capture-kit/src/snapshot-quality-verdict.test.ts index ea8afbc157..7b8f867483 100644 --- a/packages/capture-kit/src/snapshot-quality-verdict.test.ts +++ b/packages/capture-kit/src/snapshot-quality-verdict.test.ts @@ -49,6 +49,20 @@ test('readSnapshotQualityVerdict rejects unknown state or backend as verdict-abs assert.equal(readSnapshotQualityVerdict(null), undefined); }); +test('readSnapshotQualityVerdict reads every declared wire state', () => { + for (const state of ['healthy', 'recovered', 'sparse'] as const) { + assert.deepEqual(readSnapshotQualityVerdict({ state, backend: 'tree' }), { + state, + backend: 'tree', + reason: undefined, + reasonCode: undefined, + customActions: undefined, + effectiveDepth: undefined, + collapsedLeafIndexes: undefined, + }); + } +}); + test('readSnapshotQualityVerdict keeps the verdict but drops an unknown reasonCode', () => { // Forward-compat: a newer runner adding a reasonCode must still yield a usable verdict. const verdict = readSnapshotQualityVerdict({ diff --git a/packages/capture-kit/src/snapshot-quality-verdict.ts b/packages/capture-kit/src/snapshot-quality-verdict.ts index 2913abe32f..26598560b1 100644 --- a/packages/capture-kit/src/snapshot-quality-verdict.ts +++ b/packages/capture-kit/src/snapshot-quality-verdict.ts @@ -1,11 +1,6 @@ -import type { SnapshotQualityVerdict } from '@agent-device/kernel/snapshot'; +import { isSnapshotQualityState, type SnapshotQualityVerdict } from '@agent-device/kernel/snapshot'; import { SNAPSHOT_QUALITY_BACKEND_CAPABILITIES } from './snapshot-quality-backend-capabilities.ts'; -const SNAPSHOT_QUALITY_STATES = new Set([ - 'healthy', - 'recovered', - 'sparse', -]); const SNAPSHOT_QUALITY_BACKENDS = new Set( Object.keys(SNAPSHOT_QUALITY_BACKEND_CAPABILITIES) as SnapshotQualityVerdict['backend'][], ); @@ -26,10 +21,8 @@ export function readSnapshotQualityVerdict(value: unknown): SnapshotQualityVerdi // Validate the load-bearing union fields: an object with an unknown state/backend is not a // verdict this version understands, so it falls through as verdict-absent and the legacy // node-shape detectors run instead of being silently suppressed by a malformed payload. - if ( - typeof raw.state !== 'string' || - !SNAPSHOT_QUALITY_STATES.has(raw.state as SnapshotQualityVerdict['state']) - ) { + const state = raw.state; + if (!isSnapshotQualityState(state)) { return undefined; } if ( @@ -40,7 +33,7 @@ export function readSnapshotQualityVerdict(value: unknown): SnapshotQualityVerdi } const timing = readSnapshotQualityTiming(raw.timing); return { - state: raw.state as SnapshotQualityVerdict['state'], + state, backend: raw.backend as SnapshotQualityVerdict['backend'], reason: typeof raw.reason === 'string' ? raw.reason : undefined, // An unknown reasonCode is dropped, not rejected: a forward-version runner that adds one diff --git a/packages/contracts/src/snapshot-capture-annotations.test.ts b/packages/contracts/src/snapshot-capture-annotations.test.ts index 2deeb0f761..b34231bb8e 100644 --- a/packages/contracts/src/snapshot-capture-annotations.test.ts +++ b/packages/contracts/src/snapshot-capture-annotations.test.ts @@ -29,3 +29,36 @@ test('absent or non-array warnings stay absent on the serialized annotations', ( undefined, ); }); + +test('every wire verdict state survives the serialized annotations', () => { + for (const state of ['healthy', 'recovered', 'sparse']) { + const verdict = { + state, + backend: 'private-ax', + reason: 'tree capture timed out', + reasonCode: 'budget', + effectiveDepth: 56, + collapsedLeafIndexes: [3], + customActions: { read: 12, candidates: 19, truncated: 1, blocked: false }, + timing: { acquisitionMs: 12.5, presentationMs: 34.75 }, + }; + assert.deepEqual( + readSerializedSnapshotCaptureAnnotations({ snapshotQuality: verdict }).snapshotQuality, + verdict, + ); + } +}); + +/** + * This reader runs on the daemon's serialized response, and it used to project any string into the + * verdict type. A state the declared vocabulary does not name now reads as verdict-absent, which is + * what lets the shape-based fallback stay in charge instead of a disclosure for nothing. + */ +test('a state outside the declared vocabulary drops the serialized verdict', () => { + for (const state of ['heathy', 'healthy ', 'Sparse', 'degraded', '', 42, null, undefined]) { + const annotations = readSerializedSnapshotCaptureAnnotations({ + snapshotQuality: { state, backend: 'tree' }, + }); + assert.equal(annotations.snapshotQuality, undefined, JSON.stringify(state)); + } +}); diff --git a/packages/contracts/src/snapshot-capture-annotations.ts b/packages/contracts/src/snapshot-capture-annotations.ts index da90cae94f..5569892ea4 100644 --- a/packages/contracts/src/snapshot-capture-annotations.ts +++ b/packages/contracts/src/snapshot-capture-annotations.ts @@ -1,3 +1,4 @@ +import { isSnapshotQualityState } from '@agent-device/kernel/snapshot'; import type { IosTargetActivation, SnapshotQualityVerdict } from '@agent-device/kernel/snapshot'; import type { AndroidSnapshotBackendMetadata } from './snapshot-types.ts'; @@ -91,9 +92,11 @@ function readTargetActivation(value: unknown): IosTargetActivation | undefined { function readSnapshotQualityVerdict(value: unknown): SnapshotQualityVerdict | undefined { if (!value || typeof value !== 'object') return undefined; const raw = value as Record; - return typeof raw.state === 'string' && typeof raw.backend === 'string' - ? (raw as SnapshotQualityVerdict) - : undefined; + // `state` decides whether a capture reads as degraded, so it goes through the declared + // vocabulary instead of a cast: this reader sees whatever a runner or an older daemon put on the + // wire, and a state it cannot name must read as verdict-absent. + if (!isSnapshotQualityState(raw.state) || typeof raw.backend !== 'string') return undefined; + return raw as SnapshotQualityVerdict; } function readObject(value: unknown): Record | undefined { diff --git a/packages/kernel/src/snapshot-quality-states.test.ts b/packages/kernel/src/snapshot-quality-states.test.ts new file mode 100644 index 0000000000..37b061dcfc --- /dev/null +++ b/packages/kernel/src/snapshot-quality-states.test.ts @@ -0,0 +1,49 @@ +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import path from 'node:path'; +import { test } from 'vitest'; +import { + SNAPSHOT_QUALITY_STATES, + isSnapshotQualityState, + type SnapshotQualityState, +} from './snapshot.ts'; + +const SNAPSHOT_QUALITY_STATES_FIXTURE_PATH = path.resolve( + import.meta.dirname, + '..', + '..', + '..', + 'contracts', + 'fixtures', + 'ios-snapshot-quality-states.json', +); + +function readSnapshotQualityStatesFixture(): string[] { + return JSON.parse(fs.readFileSync(SNAPSHOT_QUALITY_STATES_FIXTURE_PATH, 'utf8')) as string[]; +} + +/** + * The tuple's own claim, stated on `SNAPSHOT_QUALITY_STATES`: the fixture is its wire vocabulary, + * and the runner's `SnapshotQualityState.allCases` is pinned to the same file by a unit test. + */ +test('the declared verdict states are the shared wire vocabulary', () => { + assert.deepEqual( + readSnapshotQualityStatesFixture(), + [...SNAPSHOT_QUALITY_STATES], + 'update the fixture and the Swift enum together with the tuple', + ); +}); + +test('state predicate accepts exactly the declared states', () => { + for (const state of SNAPSHOT_QUALITY_STATES) { + assert.equal(isSnapshotQualityState(state), true, state); + } + assert.equal(isSnapshotQualityState('degraded'), false); + assert.equal(isSnapshotQualityState('Healthy'), false); + assert.equal(isSnapshotQualityState(''), false); + assert.equal(isSnapshotQualityState(undefined), false); + assert.equal(isSnapshotQualityState(42), false); + // @ts-expect-error a state nobody declared cannot enter the verdict type + const undeclared: SnapshotQualityState = 'degraded'; + void undeclared; +}); diff --git a/packages/kernel/src/snapshot.ts b/packages/kernel/src/snapshot.ts index eace66eba8..7ce096608a 100644 --- a/packages/kernel/src/snapshot.ts +++ b/packages/kernel/src/snapshot.ts @@ -3,8 +3,9 @@ * The daemon renders it; it never re-derives degradation from node shapes. * * Defined here (the foundational snapshot type module) rather than in - * snapshot-quality/verdict.ts so SnapshotNode can reference it without a cyclic import; - * snapshot-quality/verdict.ts owns the validation logic. + * capture-kit's snapshot-quality-verdict.ts so SnapshotNode can reference it without a cyclic + * import; that module owns reading a verdict off the wire, and the vocabularies below are the + * states and strategies it admits. */ /** * Which capture STRATEGY produced a snapshot, within one platform's plan — @@ -23,8 +24,26 @@ export type SnapshotQualityTiming = { presentationMs: number; }; +/** + * The verdict states a capture plan may stamp. This tuple is the ONE accepted set; its order is the + * order of `contracts/fixtures/ios-snapshot-quality-states.json`, which both this module and the + * Apple runner's `SnapshotQualityState.allCases` are pinned to positionally. A state one side + * renames or adds without the other goes red there, instead of arriving as a verdict the host + * cannot name — which reads as verdict-absent and drops the disclosure with it. + */ +export const SNAPSHOT_QUALITY_STATES = ['healthy', 'recovered', 'sparse'] as const; + +export type SnapshotQualityState = (typeof SNAPSHOT_QUALITY_STATES)[number]; + +/** Whether `value` is a declared verdict state; the only gate readers apply to the wire field. */ +export function isSnapshotQualityState(value: unknown): value is SnapshotQualityState { + return ( + typeof value === 'string' && (SNAPSHOT_QUALITY_STATES as readonly string[]).includes(value) + ); +} + export type SnapshotQualityVerdict = { - state: 'healthy' | 'recovered' | 'sparse'; + state: SnapshotQualityState; backend: SnapshotCaptureBackend; reason?: string; // 'deferred' = the penalty circuit breaker pre-selected a non-XCTest backend; nothing new From 1a3d455f443839872fa5158a4a5e7c02f4ed418b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Thu, 24 Sep 2026 13:54:01 +0200 Subject: [PATCH 2/5] refactor(substrate): hold the verdict state map in each eager-frozen reader MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The readers reached the vocabulary through `isSnapshotQualityState` in `kernel/snapshot.ts`, which made that module eager in six entry closures the eager-closure gate holds at its merge-base size — the contracts capture façade at 9 modules, capture-kit's verdict reader at 2. The only module those two closures already evaluate is the one whose edge the gate rejects, so no single runtime home for the set exists. Each reader now keys a `Record` over the kernel union, the home the gate prescribes for code that has to live where it is already evaluated. Membership goes through `Object.hasOwn`, so an inherited key is never a state, and a state added to the tuple without a key in a reader is a compile error there: the guarantee the shared import was bought for, without the eager edge. Both reader tests walk the tuple and the kernel test still pins it to the fixture. --- .../src/snapshot-quality-verdict.test.ts | 5 +++- .../src/snapshot-quality-verdict.ts | 17 ++++++++++++- .../src/snapshot-capture-annotations.test.ts | 15 ++++++++++-- .../src/snapshot-capture-annotations.ts | 24 +++++++++++++++++-- .../src/snapshot-quality-states.test.ts | 24 +++++++++---------- packages/kernel/src/snapshot.ts | 20 +++++++--------- 6 files changed, 75 insertions(+), 30 deletions(-) diff --git a/packages/capture-kit/src/snapshot-quality-verdict.test.ts b/packages/capture-kit/src/snapshot-quality-verdict.test.ts index 7b8f867483..c15aec73d6 100644 --- a/packages/capture-kit/src/snapshot-quality-verdict.test.ts +++ b/packages/capture-kit/src/snapshot-quality-verdict.test.ts @@ -1,6 +1,7 @@ import { test } from 'vitest'; import assert from 'node:assert/strict'; +import { SNAPSHOT_QUALITY_STATES } from '@agent-device/kernel/snapshot'; import { isSparseSnapshotQualityVerdict, preferredSnapshotBackendForVerdict, @@ -47,10 +48,12 @@ test('readSnapshotQualityVerdict rejects unknown state or backend as verdict-abs assert.equal(readSnapshotQualityVerdict({ state: 'sparse', backend: 'mystery' }), undefined); assert.equal(readSnapshotQualityVerdict({ backend: 'tree' }), undefined); assert.equal(readSnapshotQualityVerdict(null), undefined); + // An inherited key is not a declared state: membership stays on the map's own keys. + assert.equal(readSnapshotQualityVerdict({ state: 'constructor', backend: 'tree' }), undefined); }); test('readSnapshotQualityVerdict reads every declared wire state', () => { - for (const state of ['healthy', 'recovered', 'sparse'] as const) { + for (const state of SNAPSHOT_QUALITY_STATES) { assert.deepEqual(readSnapshotQualityVerdict({ state, backend: 'tree' }), { state, backend: 'tree', diff --git a/packages/capture-kit/src/snapshot-quality-verdict.ts b/packages/capture-kit/src/snapshot-quality-verdict.ts index 26598560b1..6fdf2e61f5 100644 --- a/packages/capture-kit/src/snapshot-quality-verdict.ts +++ b/packages/capture-kit/src/snapshot-quality-verdict.ts @@ -1,6 +1,21 @@ -import { isSnapshotQualityState, type SnapshotQualityVerdict } from '@agent-device/kernel/snapshot'; +import type { SnapshotQualityState, SnapshotQualityVerdict } from '@agent-device/kernel/snapshot'; import { SNAPSHOT_QUALITY_BACKEND_CAPABILITIES } from './snapshot-quality-backend-capabilities.ts'; +/** + * Every declared state, keyed against the kernel union so this map cannot fall behind it: a state + * added there without a key here is a compile error, where a set literal merely typed as the union + * stays green and this reader drops the verdict as verdict-absent. + */ +const snapshotQualityStatesAreTheVocabulary: Record = { + healthy: true, + recovered: true, + sparse: true, +}; + +function isSnapshotQualityState(value: unknown): value is SnapshotQualityState { + return typeof value === 'string' && Object.hasOwn(snapshotQualityStatesAreTheVocabulary, value); +} + const SNAPSHOT_QUALITY_BACKENDS = new Set( Object.keys(SNAPSHOT_QUALITY_BACKEND_CAPABILITIES) as SnapshotQualityVerdict['backend'][], ); diff --git a/packages/contracts/src/snapshot-capture-annotations.test.ts b/packages/contracts/src/snapshot-capture-annotations.test.ts index b34231bb8e..d2cf3956e8 100644 --- a/packages/contracts/src/snapshot-capture-annotations.test.ts +++ b/packages/contracts/src/snapshot-capture-annotations.test.ts @@ -1,5 +1,6 @@ import assert from 'node:assert/strict'; import { test } from 'vitest'; +import { SNAPSHOT_QUALITY_STATES } from '@agent-device/kernel/snapshot'; import { readResponseWarnings } from '@agent-device/kernel/success-text'; import { readSerializedSnapshotCaptureAnnotations } from './snapshot-capture-annotations.ts'; @@ -31,7 +32,7 @@ test('absent or non-array warnings stay absent on the serialized annotations', ( }); test('every wire verdict state survives the serialized annotations', () => { - for (const state of ['healthy', 'recovered', 'sparse']) { + for (const state of SNAPSHOT_QUALITY_STATES) { const verdict = { state, backend: 'private-ax', @@ -55,7 +56,17 @@ test('every wire verdict state survives the serialized annotations', () => { * what lets the shape-based fallback stay in charge instead of a disclosure for nothing. */ test('a state outside the declared vocabulary drops the serialized verdict', () => { - for (const state of ['heathy', 'healthy ', 'Sparse', 'degraded', '', 42, null, undefined]) { + for (const state of [ + 'heathy', + 'healthy ', + 'Sparse', + 'degraded', + 'constructor', + '', + 42, + null, + undefined, + ]) { const annotations = readSerializedSnapshotCaptureAnnotations({ snapshotQuality: { state, backend: 'tree' }, }); diff --git a/packages/contracts/src/snapshot-capture-annotations.ts b/packages/contracts/src/snapshot-capture-annotations.ts index 5569892ea4..21050ad75b 100644 --- a/packages/contracts/src/snapshot-capture-annotations.ts +++ b/packages/contracts/src/snapshot-capture-annotations.ts @@ -1,7 +1,23 @@ -import { isSnapshotQualityState } from '@agent-device/kernel/snapshot'; -import type { IosTargetActivation, SnapshotQualityVerdict } from '@agent-device/kernel/snapshot'; +import type { + IosTargetActivation, + SnapshotQualityState, + SnapshotQualityVerdict, +} from '@agent-device/kernel/snapshot'; import type { AndroidSnapshotBackendMetadata } from './snapshot-types.ts'; +/** + * Every declared state, keyed against the kernel union so this map cannot fall behind it: a state + * added there without a key here is a compile error, where a cast or a set literal merely typed as + * the union stays green and a runner's verdict is dropped as verdict-absent. This reader holds the + * map rather than importing the kernel's, because `facades/capture.ts` pins its eager module + * closure and `kernel/snapshot.ts` is not in it. + */ +const snapshotQualityStatesAreTheVocabulary: Record = { + healthy: true, + recovered: true, + sparse: true, +}; + export type SnapshotCaptureAnalysis = { rawNodeCount: number; maxDepth: number; @@ -99,6 +115,10 @@ function readSnapshotQualityVerdict(value: unknown): SnapshotQualityVerdict | un return raw as SnapshotQualityVerdict; } +function isSnapshotQualityState(value: unknown): value is SnapshotQualityState { + return typeof value === 'string' && Object.hasOwn(snapshotQualityStatesAreTheVocabulary, value); +} + function readObject(value: unknown): Record | undefined { return typeof value === 'object' && value !== null ? (value as Record) diff --git a/packages/kernel/src/snapshot-quality-states.test.ts b/packages/kernel/src/snapshot-quality-states.test.ts index 37b061dcfc..13896f5398 100644 --- a/packages/kernel/src/snapshot-quality-states.test.ts +++ b/packages/kernel/src/snapshot-quality-states.test.ts @@ -2,11 +2,7 @@ import assert from 'node:assert/strict'; import fs from 'node:fs'; import path from 'node:path'; import { test } from 'vitest'; -import { - SNAPSHOT_QUALITY_STATES, - isSnapshotQualityState, - type SnapshotQualityState, -} from './snapshot.ts'; +import { SNAPSHOT_QUALITY_STATES, type SnapshotQualityState } from './snapshot.ts'; const SNAPSHOT_QUALITY_STATES_FIXTURE_PATH = path.resolve( import.meta.dirname, @@ -34,15 +30,19 @@ test('the declared verdict states are the shared wire vocabulary', () => { ); }); -test('state predicate accepts exactly the declared states', () => { +/** + * The union the readers key their exhaustive maps against: it admits exactly the declared states, + * so an undeclared one cannot reach a verdict and a state added to the tuple reaches every map. + */ +test('the verdict state type admits exactly the declared states', () => { + const declared: Record = { + healthy: true, + recovered: true, + sparse: true, + }; for (const state of SNAPSHOT_QUALITY_STATES) { - assert.equal(isSnapshotQualityState(state), true, state); + assert.equal(declared[state], true, state); } - assert.equal(isSnapshotQualityState('degraded'), false); - assert.equal(isSnapshotQualityState('Healthy'), false); - assert.equal(isSnapshotQualityState(''), false); - assert.equal(isSnapshotQualityState(undefined), false); - assert.equal(isSnapshotQualityState(42), false); // @ts-expect-error a state nobody declared cannot enter the verdict type const undeclared: SnapshotQualityState = 'degraded'; void undeclared; diff --git a/packages/kernel/src/snapshot.ts b/packages/kernel/src/snapshot.ts index 7ce096608a..ba3ab36ec4 100644 --- a/packages/kernel/src/snapshot.ts +++ b/packages/kernel/src/snapshot.ts @@ -25,23 +25,19 @@ export type SnapshotQualityTiming = { }; /** - * The verdict states a capture plan may stamp. This tuple is the ONE accepted set; its order is the - * order of `contracts/fixtures/ios-snapshot-quality-states.json`, which both this module and the - * Apple runner's `SnapshotQualityState.allCases` are pinned to positionally. A state one side - * renames or adds without the other goes red there, instead of arriving as a verdict the host - * cannot name — which reads as verdict-absent and drops the disclosure with it. + * The verdict states a capture plan may stamp. This tuple is the ONE declaration of that + * vocabulary, and `SnapshotQualityVerdict['state']` is its projection; readers hold exhaustive maps + * over the union rather than importing this module, because the eager-closure gate freezes their + * loading shape. Its order is the order of + * `contracts/fixtures/ios-snapshot-quality-states.json`, which this module and the Apple runner's + * `SnapshotQualityState.allCases` are pinned to positionally, so a state one side renames or adds + * without the other goes red there instead of arriving as a verdict the host cannot name — which + * reads as verdict-absent and drops the disclosure with it. */ export const SNAPSHOT_QUALITY_STATES = ['healthy', 'recovered', 'sparse'] as const; export type SnapshotQualityState = (typeof SNAPSHOT_QUALITY_STATES)[number]; -/** Whether `value` is a declared verdict state; the only gate readers apply to the wire field. */ -export function isSnapshotQualityState(value: unknown): value is SnapshotQualityState { - return ( - typeof value === 'string' && (SNAPSHOT_QUALITY_STATES as readonly string[]).includes(value) - ); -} - export type SnapshotQualityVerdict = { state: SnapshotQualityState; backend: SnapshotCaptureBackend; From 159b11c45bbb88ef6887a5b4af3cbb135b1a5b23 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Thu, 24 Sep 2026 14:27:46 +0200 Subject: [PATCH 3/5] refactor(snapshot): gate the backend name, and stop pinning vocabulary order MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The annotation reader was one predicate plus a blanket cast. `backend` names the recovery strategy in the warning line, so it goes through a declared map keyed against the kernel union now, and capture-kit lost both of its `as` casts on the way: the strategy is gated on the capability table it already imports, the reason code on an exhaustive map over its union. A full projection in contracts was tried and the repo's own gates refuse it — fallow reports a 4-group, 104-line clone family against capture-kit's normalizer, on top of the eager-closure gate that already forbids a shared reader. The re-read stays in the shape `readTargetActivation` in the same file uses: check the two names that decide presentation, forward what this module published, and pin the pair payload-by-payload from capture-kit's test. Order pinning drops out. The fixture is compared as a set on both sides, the stamping test asserts each case's own raw value, and the unread 28-line capture builder and its import are gone. --- ...unnerTests+SnapshotQualityStateTests.swift | 58 +++++------------ .../src/snapshot-quality-verdict.test.ts | 40 ++++++++++++ .../src/snapshot-quality-verdict.ts | 63 +++++++++---------- .../src/snapshot-capture-annotations.test.ts | 14 +++++ .../src/snapshot-capture-annotations.ts | 44 +++++++++---- .../src/snapshot-quality-states.test.ts | 7 ++- packages/kernel/src/snapshot.ts | 13 ++-- 7 files changed, 140 insertions(+), 99 deletions(-) diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SnapshotQualityStateTests.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SnapshotQualityStateTests.swift index 00bbefe12d..8ef84b8fc8 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SnapshotQualityStateTests.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SnapshotQualityStateTests.swift @@ -1,5 +1,4 @@ import XCTest -import AgentDeviceSnapshotPresentation #if AGENT_DEVICE_RUNNER_UNIT_TESTS extension RunnerTests { @@ -24,55 +23,30 @@ extension RunnerTests { return try JSONDecoder().decode([String].self, from: Data(contentsOf: fixtureURL)) } - private func wireTestCapture() -> SnapshotBackendCapture { - SnapshotBackendCapture( - payload: DataPayload( - nodes: [ - SnapshotPresentation.singleElementRead( - RawAXNode( - index: 0, - type: "Application", - label: "App", - identifier: nil, - value: nil, - rect: SnapshotRect(.zero), - enabled: true, - focused: nil, - selected: nil, - hittable: true, - depth: 0, - parentIndex: nil, - hiddenContentAbove: nil, - hiddenContentBelow: nil - ) - ) - ], - truncated: false - ), - effectiveDepth: nil - ) - } - /// The one claim of this file: the runner's closed enum and the shared TypeScript table name the - /// same states in the same order. The kernel's `SNAPSHOT_QUALITY_STATES` is pinned to it too, so - /// the two runtimes cannot drift into a verdict the host drops along with its disclosure. + /// same states. The kernel's `SNAPSHOT_QUALITY_STATES` is pinned to it too, so the two runtimes + /// cannot drift into a verdict the host drops along with its disclosure. Compared as a set: the + /// names are the contract, and a reordering of `allCases` cannot produce a wrong verdict. func testSnapshotQualityStatesMatchSharedWireFixture() throws { XCTAssertEqual( - try loadSnapshotQualityStatesFixture(), - SnapshotQualityState.allCases.map(\.rawValue), + Set(try loadSnapshotQualityStatesFixture()), + Set(SnapshotQualityState.allCases.map(\.rawValue)), "update the fixture and the kernel tuple together with the enum" ) } /// What the daemon receives for each state, taken from the production stamping path rather than a - /// hand-built verdict: the wire string is the fixture's, so a change of representation — an - /// `Int` backing, a nested object, a renamed case — goes red here on the actual payload. - func testStampedVerdictEncodesTheFixtureStateString() throws { - let fixture = try loadSnapshotQualityStatesFixture() - XCTAssertEqual(fixture.count, SnapshotQualityState.allCases.count) - for (index, state) in SnapshotQualityState.allCases.enumerated() { + /// hand-built verdict: the wire string is the case's own raw value, so a change of + /// representation — an `Int` backing, a nested object — goes red here on the actual payload, and + /// a renamed raw value goes red in the fixture test above. + func testStampedVerdictEncodesTheCaseRawValue() throws { + let capture = SnapshotBackendCapture( + payload: DataPayload(nodes: [], truncated: false), + effectiveDepth: nil + ) + for state in SnapshotQualityState.allCases { let payload = stampedSnapshotPayload( - wireTestCapture(), + capture, backend: .recursiveTree, state: state, reason: nil @@ -81,7 +55,7 @@ extension RunnerTests { StampedWireVerdict.self, from: JSONEncoder().encode(payload) ) - XCTAssertEqual(wire.snapshotQuality.state, fixture[index], state.rawValue) + XCTAssertEqual(wire.snapshotQuality.state, state.rawValue) } } diff --git a/packages/capture-kit/src/snapshot-quality-verdict.test.ts b/packages/capture-kit/src/snapshot-quality-verdict.test.ts index c15aec73d6..ee344e97a2 100644 --- a/packages/capture-kit/src/snapshot-quality-verdict.test.ts +++ b/packages/capture-kit/src/snapshot-quality-verdict.test.ts @@ -1,6 +1,7 @@ import { test } from 'vitest'; import assert from 'node:assert/strict'; +import { readSerializedSnapshotCaptureAnnotations } from '@agent-device/contracts/capture'; import { SNAPSHOT_QUALITY_STATES } from '@agent-device/kernel/snapshot'; import { isSparseSnapshotQualityVerdict, @@ -115,3 +116,42 @@ test('preferredSnapshotBackendForVerdict pins only private-ax captures', () => { ); assert.equal(preferredSnapshotBackendForVerdict(undefined), undefined); }); + +/** + * Two readings of one verdict exist on purpose: this module normalizes an untrusted runner payload, + * while contracts re-publishes what this repo published and normalizes nothing (the eager-closure + * gate forbids either reaching a shared module, and the duplication gate refuses a second + * normalization). They must still agree on which payloads are a verdict at all: a name one version + * cannot speak is verdict-absent on both sides of the daemon boundary. + */ +const VERDICT_PAYLOADS: unknown[] = [ + { state: 'sparse', backend: 'private-ax' }, + { state: 'healthy', backend: 'tree', reason: 'ok', reasonCode: 'requested-backend' }, + { state: 'recovered', backend: 'queries', reason: 42, effectiveDepth: '56' }, + { state: 'sparse', backend: 'tree', collapsedLeafIndexes: [3, 'four'] }, + { state: 'sparse', backend: 'tree', customActions: { read: 12 } }, + { state: 'sparse', backend: 'tree', customActions: { read: 12, candidates: 19 } }, + { state: 'sparse', backend: 'tree', timing: { acquisitionMs: 12.5 } }, + { state: 'sparse', backend: 'tree', timing: { acquisitionMs: 12.5, presentationMs: 34.75 } }, + { state: 'sparse', backend: 'tree', reasonCode: 'future-code' }, + { state: 'degraded', backend: 'tree' }, + { state: 'sparse', backend: 'uiautomator' }, + { state: 'constructor', backend: 'constructor' }, + { backend: 'tree' }, + { state: 'sparse' }, + null, + 'verdict', +]; + +test('the contracts re-read calls a verdict a verdict on every payload', () => { + for (const payload of VERDICT_PAYLOADS) { + const reRead = readSerializedSnapshotCaptureAnnotations({ + snapshotQuality: payload, + }).snapshotQuality; + assert.equal( + reRead === undefined, + readSnapshotQualityVerdict(payload) === undefined, + JSON.stringify(payload), + ); + } +}); diff --git a/packages/capture-kit/src/snapshot-quality-verdict.ts b/packages/capture-kit/src/snapshot-quality-verdict.ts index 6fdf2e61f5..675fabe316 100644 --- a/packages/capture-kit/src/snapshot-quality-verdict.ts +++ b/packages/capture-kit/src/snapshot-quality-verdict.ts @@ -2,33 +2,36 @@ import type { SnapshotQualityState, SnapshotQualityVerdict } from '@agent-device import { SNAPSHOT_QUALITY_BACKEND_CAPABILITIES } from './snapshot-quality-backend-capabilities.ts'; /** - * Every declared state, keyed against the kernel union so this map cannot fall behind it: a state - * added there without a key here is a compile error, where a set literal merely typed as the union - * stays green and this reader drops the verdict as verdict-absent. + * The verdict names this version can speak, each keyed against its kernel union so a map cannot + * fall behind it: a name added there without a key here is a compile error, where a set literal + * merely typed as the union stays green and this reader drops the verdict as verdict-absent. These + * readers hold the maps rather than importing the kernel's, because this module's eager closure is + * frozen at its merge-base size. The strategies need no map: `SNAPSHOT_QUALITY_BACKEND_CAPABILITIES` + * is the accepted set, keyed by the same names. */ -const snapshotQualityStatesAreTheVocabulary: Record = { +const DECLARED_STATES: Record = { healthy: true, recovered: true, sparse: true, }; -function isSnapshotQualityState(value: unknown): value is SnapshotQualityState { - return typeof value === 'string' && Object.hasOwn(snapshotQualityStatesAreTheVocabulary, value); -} +const DECLARED_REASON_CODES: Record, true> = { + 'ax-rejected': true, + 'sparse-tree': true, + budget: true, + 'no-nodes': true, + 'capture-failed': true, + 'presentation-failed': true, + deferred: true, + 'requested-backend': true, +}; -const SNAPSHOT_QUALITY_BACKENDS = new Set( - Object.keys(SNAPSHOT_QUALITY_BACKEND_CAPABILITIES) as SnapshotQualityVerdict['backend'][], -); -const SNAPSHOT_QUALITY_REASON_CODES = new Set>([ - 'ax-rejected', - 'sparse-tree', - 'budget', - 'no-nodes', - 'capture-failed', - 'presentation-failed', - 'deferred', - 'requested-backend', -]); +function isDeclared( + vocabulary: Record, + value: unknown, +): value is Key { + return typeof value === 'string' && Object.hasOwn(vocabulary, value); +} export function readSnapshotQualityVerdict(value: unknown): SnapshotQualityVerdict | undefined { if (!value || typeof value !== 'object') return undefined; @@ -36,30 +39,20 @@ export function readSnapshotQualityVerdict(value: unknown): SnapshotQualityVerdi // Validate the load-bearing union fields: an object with an unknown state/backend is not a // verdict this version understands, so it falls through as verdict-absent and the legacy // node-shape detectors run instead of being silently suppressed by a malformed payload. - const state = raw.state; - if (!isSnapshotQualityState(state)) { - return undefined; - } if ( - typeof raw.backend !== 'string' || - !SNAPSHOT_QUALITY_BACKENDS.has(raw.backend as SnapshotQualityVerdict['backend']) + !isDeclared(DECLARED_STATES, raw.state) || + !isDeclared(SNAPSHOT_QUALITY_BACKEND_CAPABILITIES, raw.backend) ) { return undefined; } const timing = readSnapshotQualityTiming(raw.timing); return { - state, - backend: raw.backend as SnapshotQualityVerdict['backend'], + state: raw.state, + backend: raw.backend, reason: typeof raw.reason === 'string' ? raw.reason : undefined, // An unknown reasonCode is dropped, not rejected: a forward-version runner that adds one // still yields a usable verdict (only the budget-specific wording is keyed off it). - reasonCode: - typeof raw.reasonCode === 'string' && - SNAPSHOT_QUALITY_REASON_CODES.has( - raw.reasonCode as NonNullable, - ) - ? (raw.reasonCode as SnapshotQualityVerdict['reasonCode']) - : undefined, + reasonCode: isDeclared(DECLARED_REASON_CODES, raw.reasonCode) ? raw.reasonCode : undefined, customActions: readCustomActionCoverage(raw.customActions), effectiveDepth: typeof raw.effectiveDepth === 'number' ? raw.effectiveDepth : undefined, collapsedLeafIndexes: Array.isArray(raw.collapsedLeafIndexes) diff --git a/packages/contracts/src/snapshot-capture-annotations.test.ts b/packages/contracts/src/snapshot-capture-annotations.test.ts index d2cf3956e8..0a9dff237e 100644 --- a/packages/contracts/src/snapshot-capture-annotations.test.ts +++ b/packages/contracts/src/snapshot-capture-annotations.test.ts @@ -73,3 +73,17 @@ test('a state outside the declared vocabulary drops the serialized verdict', () assert.equal(annotations.snapshotQuality, undefined, JSON.stringify(state)); } }); + +/** + * `backend` names the recovery strategy in the user-facing warning line, so it goes through the + * declared strategies too: a strategy this version cannot name is not a verdict it can present. The + * optional fields are forwarded as published (see the reader); normalizing them is capture-kit. + */ +test('an undeclared backend drops the serialized verdict', () => { + for (const backend of ['uiautomator', 'tree ', 'Tree', 'constructor', '', 42, null, undefined]) { + const annotations = readSerializedSnapshotCaptureAnnotations({ + snapshotQuality: { state: 'sparse', backend }, + }); + assert.equal(annotations.snapshotQuality, undefined, JSON.stringify(backend)); + } +}); diff --git a/packages/contracts/src/snapshot-capture-annotations.ts b/packages/contracts/src/snapshot-capture-annotations.ts index 21050ad75b..ed5a1effad 100644 --- a/packages/contracts/src/snapshot-capture-annotations.ts +++ b/packages/contracts/src/snapshot-capture-annotations.ts @@ -1,22 +1,30 @@ import type { IosTargetActivation, + SnapshotCaptureBackend, SnapshotQualityState, SnapshotQualityVerdict, } from '@agent-device/kernel/snapshot'; import type { AndroidSnapshotBackendMetadata } from './snapshot-types.ts'; /** - * Every declared state, keyed against the kernel union so this map cannot fall behind it: a state - * added there without a key here is a compile error, where a cast or a set literal merely typed as - * the union stays green and a runner's verdict is dropped as verdict-absent. This reader holds the - * map rather than importing the kernel's, because `facades/capture.ts` pins its eager module - * closure and `kernel/snapshot.ts` is not in it. + * The two verdict names this host has to be able to speak: `state` decides whether a capture reads + * as degraded, and `backend` names the recovery strategy in the warning line. Each map is keyed + * against its kernel union, so a name added there without a key here is a compile error, where a set + * literal merely typed as the union stays green and a runner's verdict is dropped as + * verdict-absent. The maps live here rather than behind a kernel import because + * `facades/capture.ts` pins its eager module closure and `kernel/snapshot.ts` is not in it. */ -const snapshotQualityStatesAreTheVocabulary: Record = { +const DECLARED_STATES: Record = { healthy: true, recovered: true, sparse: true, }; +const DECLARED_BACKENDS: Record = { + tree: true, + queries: true, + 'private-ax': true, + 'android-helper': true, +}; export type SnapshotCaptureAnalysis = { rawNodeCount: number; @@ -105,18 +113,30 @@ function readTargetActivation(value: unknown): IosTargetActivation | undefined { : undefined; } +/** + * Re-read of a fact this module published, in the shape `readTargetActivation` above also uses: the + * two names that decide presentation are checked, and the verdict is forwarded as published. Reading + * an untrusted runner payload is capture-kit's `readSnapshotQualityVerdict`, which normalizes every + * field; this one cannot share that code (the eager-closure gate freezes both readers' module + * closures, and the duplication gate refuses a second normalization), so the pair is pinned together + * by `snapshot-quality-verdict.test.ts`. What stays guaranteed here is the part only this boundary + * can check: a name this version cannot speak reads as verdict-absent, so a version-skewed runner + * cannot hand the host a degradation it would present under a state or strategy nobody declared. + */ function readSnapshotQualityVerdict(value: unknown): SnapshotQualityVerdict | undefined { if (!value || typeof value !== 'object') return undefined; const raw = value as Record; - // `state` decides whether a capture reads as degraded, so it goes through the declared - // vocabulary instead of a cast: this reader sees whatever a runner or an older daemon put on the - // wire, and a state it cannot name must read as verdict-absent. - if (!isSnapshotQualityState(raw.state) || typeof raw.backend !== 'string') return undefined; + if (!isDeclared(DECLARED_STATES, raw.state) || !isDeclared(DECLARED_BACKENDS, raw.backend)) { + return undefined; + } return raw as SnapshotQualityVerdict; } -function isSnapshotQualityState(value: unknown): value is SnapshotQualityState { - return typeof value === 'string' && Object.hasOwn(snapshotQualityStatesAreTheVocabulary, value); +function isDeclared( + vocabulary: Record, + value: unknown, +): value is Key { + return typeof value === 'string' && Object.hasOwn(vocabulary, value); } function readObject(value: unknown): Record | undefined { diff --git a/packages/kernel/src/snapshot-quality-states.test.ts b/packages/kernel/src/snapshot-quality-states.test.ts index 13896f5398..c7839152d9 100644 --- a/packages/kernel/src/snapshot-quality-states.test.ts +++ b/packages/kernel/src/snapshot-quality-states.test.ts @@ -20,12 +20,13 @@ function readSnapshotQualityStatesFixture(): string[] { /** * The tuple's own claim, stated on `SNAPSHOT_QUALITY_STATES`: the fixture is its wire vocabulary, - * and the runner's `SnapshotQualityState.allCases` is pinned to the same file by a unit test. + * and the runner's `SnapshotQualityState.allCases` is pinned to the same file by a unit test. As a + * set: the names are the contract, and a reordering breaks no verdict anywhere. */ test('the declared verdict states are the shared wire vocabulary', () => { assert.deepEqual( - readSnapshotQualityStatesFixture(), - [...SNAPSHOT_QUALITY_STATES], + new Set(readSnapshotQualityStatesFixture()), + new Set(SNAPSHOT_QUALITY_STATES), 'update the fixture and the Swift enum together with the tuple', ); }); diff --git a/packages/kernel/src/snapshot.ts b/packages/kernel/src/snapshot.ts index ba3ab36ec4..a21b3e2024 100644 --- a/packages/kernel/src/snapshot.ts +++ b/packages/kernel/src/snapshot.ts @@ -4,8 +4,8 @@ * * Defined here (the foundational snapshot type module) rather than in * capture-kit's snapshot-quality-verdict.ts so SnapshotNode can reference it without a cyclic - * import; that module owns reading a verdict off the wire, and the vocabularies below are the - * states and strategies it admits. + * import. Ownership splits three ways: this module owns the vocabularies below, capture-kit parses + * an untrusted runner payload into them, and contracts re-hydrates a verdict this repo published. */ /** * Which capture STRATEGY produced a snapshot, within one platform's plan — @@ -28,11 +28,10 @@ export type SnapshotQualityTiming = { * The verdict states a capture plan may stamp. This tuple is the ONE declaration of that * vocabulary, and `SnapshotQualityVerdict['state']` is its projection; readers hold exhaustive maps * over the union rather than importing this module, because the eager-closure gate freezes their - * loading shape. Its order is the order of - * `contracts/fixtures/ios-snapshot-quality-states.json`, which this module and the Apple runner's - * `SnapshotQualityState.allCases` are pinned to positionally, so a state one side renames or adds - * without the other goes red there instead of arriving as a verdict the host cannot name — which - * reads as verdict-absent and drops the disclosure with it. + * loading shape. This tuple and the Apple runner's `SnapshotQualityState.allCases` are each pinned + * to `contracts/fixtures/ios-snapshot-quality-states.json` as a set, so a state one side renames, + * adds, or deletes without the other goes red there instead of arriving as a verdict the host + * cannot name — which reads as verdict-absent and drops the disclosure with it. */ export const SNAPSHOT_QUALITY_STATES = ['healthy', 'recovered', 'sparse'] as const; From 0394c17b623002a32476e765a99856b916d19274 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Thu, 24 Sep 2026 15:58:31 +0200 Subject: [PATCH 4/5] refactor(contracts): name the verdict re-read apart from the strict reader Two same-named readers with different trust levels is how the half-migration read as validating more than it does. This module's version checks the two load-bearing names and forwards what this repo published, in the shape `readTargetActivation` uses, so it is now `readPublishedSnapshotQualityVerdict` and the strict per-field reading keeps the plain name for capture-kit's untrusted-payload reader. --- .../src/snapshot-capture-annotations.ts | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/packages/contracts/src/snapshot-capture-annotations.ts b/packages/contracts/src/snapshot-capture-annotations.ts index ed5a1effad..cb51d54c7d 100644 --- a/packages/contracts/src/snapshot-capture-annotations.ts +++ b/packages/contracts/src/snapshot-capture-annotations.ts @@ -58,7 +58,7 @@ export type PublicSnapshotCaptureAnnotations = Pick< export function snapshotCaptureAnnotationsFrom( source: Partial> & { quality?: unknown }, ): SnapshotCaptureAnnotations { - const quality = readSnapshotQualityVerdict(source.quality); + const quality = readPublishedSnapshotQualityVerdict(source.quality); return { ...(source.analysis ? { analysis: source.analysis } : {}), ...(source.androidSnapshot ? { androidSnapshot: source.androidSnapshot } : {}), @@ -92,7 +92,7 @@ export function readSerializedSnapshotCaptureAnnotations( const warnings = Array.isArray(data.warnings) ? data.warnings.filter((entry): entry is string => typeof entry === 'string') : undefined; - const quality = readSnapshotQualityVerdict(data.snapshotQuality); + const quality = readPublishedSnapshotQualityVerdict(data.snapshotQuality); const targetActivation = readTargetActivation(data.targetActivation); return publicSnapshotCaptureAnnotations({ ...(androidSnapshot @@ -115,15 +115,16 @@ function readTargetActivation(value: unknown): IosTargetActivation | undefined { /** * Re-read of a fact this module published, in the shape `readTargetActivation` above also uses: the - * two names that decide presentation are checked, and the verdict is forwarded as published. Reading - * an untrusted runner payload is capture-kit's `readSnapshotQualityVerdict`, which normalizes every - * field; this one cannot share that code (the eager-closure gate freezes both readers' module - * closures, and the duplication gate refuses a second normalization), so the pair is pinned together - * by `snapshot-quality-verdict.test.ts`. What stays guaranteed here is the part only this boundary - * can check: a name this version cannot speak reads as verdict-absent, so a version-skewed runner - * cannot hand the host a degradation it would present under a state or strategy nobody declared. + * two names that decide presentation are checked, and the verdict is forwarded as published. Named + * apart from capture-kit's stricter `readSnapshotQualityVerdict`, which normalizes an untrusted + * runner payload and reads every field. Reading + * This one cannot share that code — the eager-closure gate freezes both readers' module closures and + * the duplication gate refuses a second normalization — so the pair is pinned together by + * `snapshot-quality-verdict.test.ts`. What stays guaranteed here is the part only this boundary can + * check: a name this version cannot speak reads as verdict-absent, so a version-skewed runner cannot + * hand the host a degradation it would present under a state or strategy nobody declared. */ -function readSnapshotQualityVerdict(value: unknown): SnapshotQualityVerdict | undefined { +function readPublishedSnapshotQualityVerdict(value: unknown): SnapshotQualityVerdict | undefined { if (!value || typeof value !== 'object') return undefined; const raw = value as Record; if (!isDeclared(DECLARED_STATES, raw.state) || !isDeclared(DECLARED_BACKENDS, raw.backend)) { From 6ccf3aef3bc2a17ebc3bdd0e752326de6d84bfb0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Thu, 24 Sep 2026 17:26:53 +0200 Subject: [PATCH 5/5] refactor(snapshot): answer the review round on sharing, parity, and evidence The eager-closure refusal the review asked to be pointed at or filed is not a table of budgets: the gate ratchets every entry's closure against the committed merge-base tree, so the numbers to read are 9 for `facades/capture.ts` and 2 for `snapshot-quality-verdict.ts`. Re-installing one shared kernel predicate for both readers on this head fails six entries and names both of those, which is why the vocabulary stays a map per reader; recorded on #2872 as the accepted deviation. The `android-helper` asymmetry is not real: `SNAPSHOT_QUALITY_BACKEND_CAPABILITIES` merges the Android declaration, so both readers accept that strategy and the parity table now carries the row instead of omitting it. The client-side drop of an unknown state or backend gets its CHANGELOG entry, the justification comments shrink to one constraint sentence each, the kernel test keeps only its `@ts-expect-error`, and the word left dangling by the last rename goes. --- .../src/snapshot-quality-verdict.test.ts | 1 + .../src/snapshot-quality-verdict.ts | 10 ++++----- .../src/snapshot-capture-annotations.ts | 22 ++++++------------- .../src/snapshot-quality-states.test.ts | 12 ---------- packages/kernel/src/snapshot.ts | 10 ++++----- 5 files changed, 17 insertions(+), 38 deletions(-) diff --git a/packages/capture-kit/src/snapshot-quality-verdict.test.ts b/packages/capture-kit/src/snapshot-quality-verdict.test.ts index ee344e97a2..0c27cf5588 100644 --- a/packages/capture-kit/src/snapshot-quality-verdict.test.ts +++ b/packages/capture-kit/src/snapshot-quality-verdict.test.ts @@ -134,6 +134,7 @@ const VERDICT_PAYLOADS: unknown[] = [ { state: 'sparse', backend: 'tree', timing: { acquisitionMs: 12.5 } }, { state: 'sparse', backend: 'tree', timing: { acquisitionMs: 12.5, presentationMs: 34.75 } }, { state: 'sparse', backend: 'tree', reasonCode: 'future-code' }, + { state: 'recovered', backend: 'android-helper', reasonCode: 'requested-backend' }, { state: 'degraded', backend: 'tree' }, { state: 'sparse', backend: 'uiautomator' }, { state: 'constructor', backend: 'constructor' }, diff --git a/packages/capture-kit/src/snapshot-quality-verdict.ts b/packages/capture-kit/src/snapshot-quality-verdict.ts index 675fabe316..475d09009f 100644 --- a/packages/capture-kit/src/snapshot-quality-verdict.ts +++ b/packages/capture-kit/src/snapshot-quality-verdict.ts @@ -2,12 +2,10 @@ import type { SnapshotQualityState, SnapshotQualityVerdict } from '@agent-device import { SNAPSHOT_QUALITY_BACKEND_CAPABILITIES } from './snapshot-quality-backend-capabilities.ts'; /** - * The verdict names this version can speak, each keyed against its kernel union so a map cannot - * fall behind it: a name added there without a key here is a compile error, where a set literal - * merely typed as the union stays green and this reader drops the verdict as verdict-absent. These - * readers hold the maps rather than importing the kernel's, because this module's eager closure is - * frozen at its merge-base size. The strategies need no map: `SNAPSHOT_QUALITY_BACKEND_CAPABILITIES` - * is the accepted set, keyed by the same names. + * The verdict names this version can speak, keyed against the kernel unions so a map cannot fall + * behind one. They cannot be one shared kernel predicate: this module's eager closure is frozen at + * its merge-base size (#2872). The strategies need no map — `SNAPSHOT_QUALITY_BACKEND_CAPABILITIES` + * is already keyed by exactly those names. */ const DECLARED_STATES: Record = { healthy: true, diff --git a/packages/contracts/src/snapshot-capture-annotations.ts b/packages/contracts/src/snapshot-capture-annotations.ts index cb51d54c7d..7ad86290cd 100644 --- a/packages/contracts/src/snapshot-capture-annotations.ts +++ b/packages/contracts/src/snapshot-capture-annotations.ts @@ -7,12 +7,9 @@ import type { import type { AndroidSnapshotBackendMetadata } from './snapshot-types.ts'; /** - * The two verdict names this host has to be able to speak: `state` decides whether a capture reads - * as degraded, and `backend` names the recovery strategy in the warning line. Each map is keyed - * against its kernel union, so a name added there without a key here is a compile error, where a set - * literal merely typed as the union stays green and a runner's verdict is dropped as - * verdict-absent. The maps live here rather than behind a kernel import because - * `facades/capture.ts` pins its eager module closure and `kernel/snapshot.ts` is not in it. + * The verdict names this host has to speak, each keyed against its kernel union so a name added + * there without a key here is a compile error. They cannot be one shared kernel predicate: the + * eager-closure gate keeps `kernel/snapshot.ts` out of `facades/capture.ts` (#2872). */ const DECLARED_STATES: Record = { healthy: true, @@ -114,15 +111,10 @@ function readTargetActivation(value: unknown): IosTargetActivation | undefined { } /** - * Re-read of a fact this module published, in the shape `readTargetActivation` above also uses: the - * two names that decide presentation are checked, and the verdict is forwarded as published. Named - * apart from capture-kit's stricter `readSnapshotQualityVerdict`, which normalizes an untrusted - * runner payload and reads every field. Reading - * This one cannot share that code — the eager-closure gate freezes both readers' module closures and - * the duplication gate refuses a second normalization — so the pair is pinned together by - * `snapshot-quality-verdict.test.ts`. What stays guaranteed here is the part only this boundary can - * check: a name this version cannot speak reads as verdict-absent, so a version-skewed runner cannot - * hand the host a degradation it would present under a state or strategy nobody declared. + * Re-read of a fact this module published, in the shape `readTargetActivation` above uses: the two + * names that decide presentation are checked, the rest is forwarded as published. capture-kit's + * `readSnapshotQualityVerdict` normalizes an untrusted runner payload field by field; the two + * readings are pinned to each other in `snapshot-quality-verdict.test.ts`. */ function readPublishedSnapshotQualityVerdict(value: unknown): SnapshotQualityVerdict | undefined { if (!value || typeof value !== 'object') return undefined; diff --git a/packages/kernel/src/snapshot-quality-states.test.ts b/packages/kernel/src/snapshot-quality-states.test.ts index c7839152d9..45fdbb4e2a 100644 --- a/packages/kernel/src/snapshot-quality-states.test.ts +++ b/packages/kernel/src/snapshot-quality-states.test.ts @@ -31,19 +31,7 @@ test('the declared verdict states are the shared wire vocabulary', () => { ); }); -/** - * The union the readers key their exhaustive maps against: it admits exactly the declared states, - * so an undeclared one cannot reach a verdict and a state added to the tuple reaches every map. - */ test('the verdict state type admits exactly the declared states', () => { - const declared: Record = { - healthy: true, - recovered: true, - sparse: true, - }; - for (const state of SNAPSHOT_QUALITY_STATES) { - assert.equal(declared[state], true, state); - } // @ts-expect-error a state nobody declared cannot enter the verdict type const undeclared: SnapshotQualityState = 'degraded'; void undeclared; diff --git a/packages/kernel/src/snapshot.ts b/packages/kernel/src/snapshot.ts index a21b3e2024..7b24d551b5 100644 --- a/packages/kernel/src/snapshot.ts +++ b/packages/kernel/src/snapshot.ts @@ -27,11 +27,11 @@ export type SnapshotQualityTiming = { /** * The verdict states a capture plan may stamp. This tuple is the ONE declaration of that * vocabulary, and `SnapshotQualityVerdict['state']` is its projection; readers hold exhaustive maps - * over the union rather than importing this module, because the eager-closure gate freezes their - * loading shape. This tuple and the Apple runner's `SnapshotQualityState.allCases` are each pinned - * to `contracts/fixtures/ios-snapshot-quality-states.json` as a set, so a state one side renames, - * adds, or deletes without the other goes red there instead of arriving as a verdict the host - * cannot name — which reads as verdict-absent and drops the disclosure with it. + * over the union instead of importing this module, because the eager-closure gate freezes their + * loading shape (#2872). This tuple and the Apple runner's `SnapshotQualityState.allCases` are each + * pinned as a set to `contracts/fixtures/ios-snapshot-quality-states.json`, so a state one side + * renames, adds, or deletes without the other goes red there instead of arriving as a verdict the + * host cannot name — which reads as verdict-absent and drops the disclosure with it. */ export const SNAPSHOT_QUALITY_STATES = ['healthy', 'recovered', 'sparse'] as const;