From 5ef1e70e7cf3b4b1f4876dc30e89bb11d1744046 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Tue, 30 Jun 2026 22:14:05 +0200 Subject: [PATCH] fix: restore runnerRoundTrips agent-cost field dropped in #968 PR #968 (apple-platform-consolidation) accidentally dropped the runnerRoundTrips field from the agent-cost block during an over-broad conflict resolution. This restores the shipped Phase-4 feature: - src/kernel/contracts.ts: re-add ResponseCost.runnerRoundTrips: number - src/utils/diagnostics.ts: restore the countDiagnosticEventsByPhase() accessor over the flush-surviving phaseCounts tally (the tally itself survived #968; only the accessor was removed) - src/daemon/request-router.ts: repopulate runnerRoundTrips in the cost graft by counting the two real round-trip diagnostic phases (ios_runner_command_send + ios_runner_readiness_preflight). The RUNNER_ROUND_TRIP_PHASES constant is now defined locally (its former home, the dev-only runner-request-count.ts, was removed in #968 and is out of scope to restore) - request-router-cost.test.ts: restore the round-trip counting test and the runnerRoundTrips:0 assertion Byte-identical-default invariant preserved: with --cost OFF or on an error response the serialized payload is unchanged. --- .../__tests__/request-router-cost.test.ts | 29 ++++++++++++++++++- src/daemon/request-router.ts | 15 ++++++++++ src/kernel/contracts.ts | 4 +++ src/utils/diagnostics.ts | 16 ++++++++++ 4 files changed, 63 insertions(+), 1 deletion(-) diff --git a/src/daemon/__tests__/request-router-cost.test.ts b/src/daemon/__tests__/request-router-cost.test.ts index d32bf15fe..1e79a9b4b 100644 --- a/src/daemon/__tests__/request-router-cost.test.ts +++ b/src/daemon/__tests__/request-router-cost.test.ts @@ -17,6 +17,7 @@ vi.mock('../device-ready.ts', () => ({ ensureDeviceReady: vi.fn(async () => {}) import { dispatchCommand } from '../../core/dispatch.ts'; import { createRequestHandler } from '../request-router.ts'; +import { emitDiagnostic } from '../../utils/diagnostics.ts'; import type { DaemonRequest, SessionState } from '../types.ts'; import { LeaseRegistry } from '../lease-registry.ts'; import { makeSessionStore } from '../../__tests__/test-utils/store-factory.ts'; @@ -110,9 +111,12 @@ test('(b) flag-on additive-only: cost block is the ONLY delta vs flag-off', asyn expect(respFlagOn.ok).toBe(true); if (!respFlagOff.ok || !respFlagOn.ok) return; + // The cost block carries BOTH wallClockMs and runnerRoundTrips, both numbers + // ≥ 0. A request that never touches the iOS runner reports 0 — honest. const cost = respFlagOn.data?.cost; expect(cost).toMatchObject({ wallClockMs: expect.any(Number), + runnerRoundTrips: 0, }); expect(cost?.wallClockMs).toBeGreaterThanOrEqual(0); // This payload has no node tree, so nodeCount is omitted entirely. @@ -123,7 +127,30 @@ test('(b) flag-on additive-only: cost block is the ONLY delta vs flag-off', asyn expect(respFlagOn.data).toEqual(respFlagOff.data); }); -test('(c) nodeCount reports the node-tree size whenever data carries a nodes array, additive-only', async () => { +test('(c) runnerRoundTrips counts real iOS-runner round-trip diagnostics in scope', async () => { + const { sessionStore, handler } = makeHandler(); + sessionStore.set('cost-session', makeIosSession('cost-session')); + + // The mocked dispatch runs inside the request's diagnostics scope, so emitting + // here is equivalent to the runner-session emitting these phases per round-trip. + mockDispatch.mockImplementation(async () => { + emitDiagnostic({ phase: 'ios_runner_readiness_preflight' }); // real round-trip + emitDiagnostic({ phase: 'ios_runner_command_send' }); // real round-trip + emitDiagnostic({ phase: 'ios_runner_command_send' }); // real round-trip + emitDiagnostic({ level: 'debug', phase: 'ios_runner_readiness_preflight_skipped' }); // NOT + emitDiagnostic({ phase: 'some_other_phase' }); // NOT + return { ...REPRESENTATIVE_PAYLOAD }; + }); + + const resp = await handler(baseRequest({ meta: { includeCost: true } })); + expect(resp.ok).toBe(true); + if (!resp.ok) return; + // 1 preflight + 2 command_send = 3; the _skipped marker and unrelated phases + // are excluded. + expect(resp.data?.cost?.runnerRoundTrips).toBe(3); +}); + +test('(c2) nodeCount reports the node-tree size whenever data carries a nodes array, additive-only', async () => { const { sessionStore, handler } = makeHandler(); sessionStore.set('cost-session', makeIosSession('cost-session')); diff --git a/src/daemon/request-router.ts b/src/daemon/request-router.ts index 53e5210d4..dd6f0bd3c 100644 --- a/src/daemon/request-router.ts +++ b/src/daemon/request-router.ts @@ -22,6 +22,7 @@ import { withRequestPlatformProviderScope, } from './request-platform-providers.ts'; import { + countDiagnosticEventsByPhase, emitDiagnostic, flushDiagnosticsToSessionFile, getDiagnosticsMeta, @@ -344,12 +345,26 @@ function applyResponseLevelView( return view ? { ok: true, data: view(response.data ?? {}, level) } : response; } +// Diagnostic phases emitted once per real iOS-runner round-trip. `..._command_send` +// is the command itself; `..._readiness_preflight` is the pre-command uptime probe +// (a real network round-trip). The `..._skipped` / `..._recovered` markers do NOT +// hit the runner and are intentionally excluded. +const RUNNER_ROUND_TRIP_PHASES = [ + 'ios_runner_command_send', + 'ios_runner_readiness_preflight', +] as const; + function buildResponseCost( originalData: DaemonResponseData | undefined, startedAt: number, ): ResponseCost { const cost: ResponseCost = { wallClockMs: Date.now() - startedAt, + // Counts this request's real runner round-trips from the flush-surviving + // diagnostics phase tally. Reads 0 when no runner was hit (e.g. a no-op or a + // command served entirely from the daemon). Must run inside the request's + // diagnostics scope (see `applyAgentCostGrafts` call site). + runnerRoundTrips: countDiagnosticEventsByPhase(RUNNER_ROUND_TRIP_PHASES), }; // nodeCount reads the ORIGINAL node tree (the digest view may have already // collapsed `data.nodes`), so the count stays accurate. diff --git a/src/kernel/contracts.ts b/src/kernel/contracts.ts index 56a19a75d..c1509423d 100644 --- a/src/kernel/contracts.ts +++ b/src/kernel/contracts.ts @@ -120,6 +120,10 @@ export type DaemonArtifact = { export type ResponseCost = { wallClockMs: number; + // Number of real iOS-runner round-trips made while serving the request (the + // `ios_runner_command_send` + `ios_runner_readiness_preflight` diagnostic + // phases). Always present when cost is included; 0 when no runner was hit. + runnerRoundTrips: number; // Number of UI/accessibility nodes in the response, when the command returns a // node tree (e.g. snapshot). Absent for commands that produce no nodes, so an // agent can size a snapshot before re-fetching at a different depth/scope. diff --git a/src/utils/diagnostics.ts b/src/utils/diagnostics.ts index 5e176e690..f3e2daf45 100644 --- a/src/utils/diagnostics.ts +++ b/src/utils/diagnostics.ts @@ -86,6 +86,22 @@ export function getDiagnosticsMeta(): { }; } +/** + * Sum the number of diagnostic events emitted in the current scope whose phase + * is one of `phases`. Backed by the flush-surviving `phaseCounts` tally, so it + * stays accurate for the whole request even under `--debug` (where `events` is + * streamed out and reset). Returns 0 when called outside a diagnostics scope. + */ +export function countDiagnosticEventsByPhase(phases: readonly string[]): number { + const scope = diagnosticsStorage.getStore(); + if (!scope) return 0; + let total = 0; + for (const phase of phases) { + total += scope.phaseCounts.get(phase) ?? 0; + } + return total; +} + export function emitDiagnostic(event: { level?: DiagnosticLevel; phase: string;