Skip to content

Commit 13a45b9

Browse files
authored
test(remote): assert the proxied snapshot's whole wire conversation (#2387)
#2198 required proof that "the bridge adds no network round trip and transfers only the published response". That was read off the RTT benchmark — identical response bytes, unchanged wall-clock slope — which is inference, not proof, and the raw-result schema carries no round-trip count. Assert it instead. Record every request the proxy forwards upstream while one `snapshot -i` runs, and pin the whole conversation rather than just the RPC, so a new call of any kind fails. The bridge contributes nothing to it: no helper, admin or acquisition route appears. Then read what crossed rather than sizing it, at every level of the payload — envelope, result, published data — each against its declared key set. Comparing the wire's result against the client's response proves nothing on its own, since the client publishes whatever the result holds and both sides move together. Refs #2198.
1 parent bfec821 commit 13a45b9

1 file changed

Lines changed: 129 additions & 2 deletions

File tree

‎test/integration/provider-scenarios/remote-proxy-parity.test.ts‎

Lines changed: 129 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import assert from 'node:assert/strict';
22
import fs from 'node:fs';
3+
import http from 'node:http';
34
import os from 'node:os';
45
import path from 'node:path';
56
import { test } from 'vitest';
@@ -228,7 +229,7 @@ async function runLeg(leg: ScenarioLeg, legName: string): Promise<DaemonResponse
228229
}
229230

230231
async function withProxiedWorld<T>(
231-
run: (context: { world: ParityWorld; proxied: ScenarioLeg }) => Promise<T>,
232+
run: (context: { world: ParityWorld; proxied: ScenarioLeg; upstream: http.Server }) => Promise<T>,
232233
options: ParityWorldOptions = {},
233234
): Promise<T> {
234235
const world = await createParityWorld(options);
@@ -259,7 +260,7 @@ async function withProxiedWorld<T>(
259260
return responseFromClientError(error);
260261
}
261262
};
262-
return await run({ world, proxied });
263+
return await run({ world, proxied, upstream });
263264
} finally {
264265
await closeLoopbackServer(proxy);
265266
await closeLoopbackServer(upstream);
@@ -268,6 +269,59 @@ async function withProxiedWorld<T>(
268269
}
269270
}
270271

272+
/**
273+
* Every request the proxy forwards upstream, in order. Registering a second `request`
274+
* listener records without displacing the server's own handler, so the daemon behaves
275+
* exactly as it does untraced.
276+
*/
277+
function recordUpstreamRequests(upstream: http.Server): { forwarded: ForwardedRequest[] } {
278+
const forwarded: ForwardedRequest[] = [];
279+
upstream.on('request', (req, res) => {
280+
const route = (req.url ?? '').split('?', 1)[0] ?? '';
281+
const entry: ForwardedRequest = { method: req.method ?? '', route, body: '' };
282+
const end = res.end.bind(res);
283+
const write = res.write.bind(res);
284+
res.write = ((chunk: unknown, ...rest: unknown[]) => {
285+
entry.body += textOf(chunk);
286+
return (write as (...args: unknown[]) => boolean)(chunk, ...rest);
287+
}) as typeof res.write;
288+
res.end = ((chunk?: unknown, ...rest: unknown[]) => {
289+
entry.body += textOf(chunk);
290+
return (end as (...args: unknown[]) => http.ServerResponse)(chunk, ...rest);
291+
}) as typeof res.end;
292+
forwarded.push(entry);
293+
});
294+
return { forwarded };
295+
}
296+
297+
/** One request the proxy forwarded, with the exact bytes the daemon answered it with. */
298+
type ForwardedRequest = { method: string; route: string; body: string };
299+
300+
function textOf(chunk: unknown): string {
301+
if (typeof chunk === 'string') return chunk;
302+
if (chunk instanceof Uint8Array) return Buffer.from(chunk).toString('utf8');
303+
return '';
304+
}
305+
306+
/**
307+
* What a published `snapshot --interactive-only` response is made of. #2198 forbids the
308+
* internal acquired tree, lineage, target-generation facts and runner quality payload from
309+
* crossing as wire payloads, so the guard is the KEY SET, not a size: an extra field is
310+
* small, and one added inside the payload grows the wire and the client's copy together.
311+
*/
312+
const PUBLISHED_RPC_ENVELOPE_KEYS = ['id', 'jsonrpc', 'result'] as const;
313+
const PUBLISHED_SNAPSHOT_RESULT_KEYS = ['data', 'ok'] as const;
314+
const PUBLISHED_SNAPSHOT_DATA_KEYS = [
315+
'appBundleId',
316+
'appName',
317+
'nodes',
318+
'refsGeneration',
319+
'snapshotDiagnostics',
320+
'truncated',
321+
'visibility',
322+
'warnings',
323+
] as const;
324+
271325
type LegRun = { responses: DaemonResponse[]; sessionRoot: string };
272326

273327
function assertPublishedParity(direct: LegRun, proxied: LegRun): void {
@@ -403,6 +457,79 @@ test(
403457
PARALLEL_PROVIDER_SCENARIO_TIMEOUT_MS,
404458
);
405459

460+
/**
461+
* #2198: "the regular-snapshot trace must prove the bridge adds no network round trip and
462+
* transfers only the published response." That was previously read off the RTT benchmark —
463+
* identical response bytes and an unchanged wall-clock slope — which is inference, not proof.
464+
* This asserts it: one `snapshot -i` behind the proxy forwards exactly one upstream request,
465+
* it is the RPC, and the bytes crossing are the published response and nothing else.
466+
*
467+
* The regression it guards is a second forwarded call per snapshot — a bridge, helper or
468+
* admin round trip appearing on the wire, which #2198 forbids outright.
469+
*/
470+
test(
471+
'a proxied snapshot forwards one upstream request and only the published response',
472+
{
473+
timeout: PARALLEL_PROVIDER_SCENARIO_TIMEOUT_MS,
474+
},
475+
async (t) => {
476+
if (await skipWhenLoopbackUnavailable(t)) return;
477+
await withProxiedWorld(async ({ world, proxied, upstream }) => {
478+
const flags = { platform: 'ios', udid: SIM.id } as const;
479+
await proxied({ session: 'default', command: 'open', positionals: [APP], flags });
480+
481+
// Record only the snapshot: the open above is setup and makes its own calls.
482+
const { forwarded } = recordUpstreamRequests(upstream);
483+
const response = await proxied({
484+
session: 'default',
485+
command: 'snapshot',
486+
positionals: [],
487+
flags: { snapshotInteractiveOnly: true },
488+
});
489+
assert.equal(response.ok, true);
490+
491+
// The WHOLE wire conversation, not just the RPC, so any new call of any kind breaks
492+
// this. `GET /health` is the client's own ADR 0006 protocol-compatibility probe on the
493+
// remote path (`ensureRemoteDaemon`); it is not the bridge, and it predates the bridge.
494+
// The bridge itself contributes nothing: no helper, admin or acquisition route appears.
495+
assert.deepEqual(
496+
forwarded.map((entry) => `${entry.method} ${entry.route}`),
497+
['GET /health', 'POST /rpc'],
498+
'a proxied snapshot must cross the wire only as the compat probe and the RPC',
499+
);
500+
501+
// What crossed, read rather than sized, at every level of the payload. Comparing the
502+
// wire's `result` against the client's response would prove nothing — the client
503+
// publishes whatever `result` holds, so both sides move together — so each level is
504+
// pinned against its declared key set instead.
505+
const rpcBody = JSON.parse(forwarded.at(-1)?.body ?? '{}') as {
506+
result?: { data?: Record<string, unknown> };
507+
};
508+
assert.deepEqual(
509+
Object.keys(rpcBody).sort(),
510+
[...PUBLISHED_RPC_ENVELOPE_KEYS],
511+
'the upstream RPC body carried something beyond the JSON-RPC envelope',
512+
);
513+
assert.deepEqual(
514+
Object.keys(rpcBody.result ?? {}).sort(),
515+
[...PUBLISHED_SNAPSHOT_RESULT_KEYS],
516+
'the RPC result carried something beyond the published response',
517+
);
518+
assert.deepEqual(
519+
Object.keys(rpcBody.result?.data ?? {}).sort(),
520+
[...PUBLISHED_SNAPSHOT_DATA_KEYS],
521+
'the published snapshot payload carried a field it does not publish',
522+
);
523+
const sessionRoot = sessionRootOf(world.daemon);
524+
assert.deepEqual(
525+
stripVolatile(rpcBody.result, sessionRoot),
526+
stripVolatile(response, sessionRoot),
527+
'the result on the wire is not the response the client published',
528+
);
529+
});
530+
},
531+
);
532+
406533
const LEASE_SCOPE = {
407534
tenantId: 'team-a',
408535
runId: 'run-a',

0 commit comments

Comments
 (0)