mirror of
https://github.com/callstack/agent-device.git
synced 2026-09-14 20:06:34 +08:00
05a1d76f2e
* test: gate daemon RPC wire compatibility against the last released tag (#1432) ADR 0006 fixes exactly when DAEMON_RPC_PROTOCOL_VERSION must be bumped, and nothing checked that it was. The runtime guard (readRemoteDaemonHealth) refuses a mismatched peer, but only fires when someone remembered the bump — a wire change that skipped it left both sides advertising protocol 2 while parsing different payloads, which is the failure ADR 0006 exists to prevent. Local daemons cannot skew (isReusableDaemonInfo takes over on any package version mismatch). Cross-machine is skewed by design — proxy, cloud/limrun, a remote macOS host — and ADR 0006 explicitly rules package version out as the compatibility gate there, so the one boundary where skew is intended was the one boundary with no gate. test/wire-compat/surface.ts declares the wire surface grouped by the ADR bullet each group serves, quoting it, with an `uncovered` note where a bullet is only partly digestible (the /health and /rpc literals inside http-server.ts stay reviewer-owned: a moved route 404s at connect time rather than misparsing). ledger.json records what each declaration hashes to, at which protocol version. Two gates, split for the same reason the replay-compat corpus splits: - unit-core holds the ledger to its source and prints the digest to paste; - Released-Surface Compatibility reads the ledger at the last RELEASED tag and requires the drift since then to carry a bump or a compatibleChanges ack. From one commit a bumped ledger and an unbumped one are both just an edited file, so only a released baseline can tell them apart. Acks are keyed by the digest they cover, so one "added an optional field" cannot launder later changes. Digests ignore comments and formatting; the manifest's closure is derived from the AST, so a field typed by an unlisted sibling fails rather than sitting outside the gate. CI cost: one added job (checkout + toolchain + two node scripts, ~1 min), mirroring the existing full-history replay-compat job. * test: close wire-surface overclaim and make the closure fail closed (#1432) Addresses both review P1s on #1717. P1 — the manifest materially overclaimed ADR 0006 coverage. It quoted all four bullets while digesting only the payload TYPES, so the producer and consumer seams could break a skewed peer without moving a listed digest. Now listed on both sides of every boundary: JSON-RPC method sets and the projections that turn each method's params into a DaemonRequest, createRpcError/sendJson/ writeRpcResponseEnvelope, resolveToken and the auth-hook types, upload preflight/finalize/308 handlers and the resumable ticket shape, artifact route and download/inventory framing, REST error mapping, and the client's own payload builder, lease-method mapping, response parser and error projection. 57 -> 117 declarations. What stays out is now named rather than implied: createDaemonHttpServer's dispatch wiring and the /health and /rpc literals inside it. Everything it dispatches WITH is digested individually, and a moved route 404s at connect time rather than misparsing — the loud failure, not the silent one. P1 — imported and re-exported payload shapes escaped the closure. declarationHomes() scanned only the manifest's own files and the walk continued silently when a name could not be placed, so a listed type could gain foo?: ImportedShape from a new module and stay green. Resolution is now explicit and fails closed: relative imports, workspace specifiers (through the owning package's own exports map, so a re-pointed export cannot drop a type), and facade re-export chains. Every referenced name must land on a listed declaration, a waiver with a written reason, a declared external module, or the TS/Node global set. Fixed two extractor blind spots the walk exposed: a declaration's own generic parameters and `as const` were being reported as references. Planted-red proofs (wire-mutations.test.ts): 13 cases independently mutate method naming, response serialization, response parsing, auth projection, upload ticket shape, 308 framing, artifact framing, REST error mapping, and progress framing, each asserting the digest moves; 3 probes prove the closure really reaches across a package boundary, a facade re-export, and a plain relative import. Mutations apply inside the declaration's own span — a whole-file replace silently hit a sibling sharing the substring, which is how the first draft of one case passed vacuously. The largest waiver pair (InternalRequestOptions, CommandFlags) rests on ADR 0006's own additive rule: they reach the peer inside DaemonRequest's untyped flags/input bags, and the decision says a new flag needs no bump. Digesting them would fire the gate on every new CLI flag and train reviewers to rubber-stamp acks. * test: list the consumer half of the auxiliary HTTP boundaries (#1432) Addresses the remaining review P1 on #1717. The manifest claimed both sides of response/upload/artifact framing while listing nothing from upload-client.ts, daemon-artifacts.ts, or the health consumer in daemon-client-transport.ts, so those parsers could narrow without moving a listed digest or protocol 2. Now listed (117 -> 141 declarations): - /health consumer: RemoteDaemonHealth, readHealthPayload, readDaemonHttpHealth, readRemoteDaemonHealth. This is the sharpest of the three — narrowing the reader or the comparison disables the very refusal ADR 0006 exists to guarantee, and nothing else in the repo would notice. - /upload consumer: UploadResponse, UploadPreflightResponse, UploadPreflightResult, parseUploadPreflightResult, requestUploadPreflight, uploadDirectArtifact, tryDirectUploadWithResume, shouldRetryDirectUpload, finalizeDirectUpload, uploadLegacyArtifact, ARTIFACT_HASH_ALGORITHM, isStringRecord, and PreparedUploadArtifact — whose sha256/sizeBytes/fileName/artifactType/ contentType fields ARE the preflight body the daemon parses. - /artifacts/* consumer: DaemonArtifactEndpoint, buildDaemonArtifactUrl, isRemoteDaemon, DownloadRemoteArtifactParams, downloadRemoteArtifact, materializeRemoteArtifacts, resolveMaterializedArtifactPath. Running the closure fail-closed over the new files surfaced three more stops, each decided rather than skipped: PreparedUploadArtifact listed (it is payload), UploadProgressSink waived (client-local rendering, never leaves the process), and src/daemon/types.ts#DaemonArtifact waived as a re-export alias of the listed kernel type, matching its DaemonRequest/DaemonResponse siblings. 10 more planted-red mutations cover the new seams: health version-read and mismatch-refusal defeated, RemoteDaemonHealth field dropped, preflight parser narrowed, preflight/legacy response shapes narrowed, finalize body key renamed, ticket field renamed, artifact tenant header dropped, artifact URL moved. A fourth closure probe proves the upload-consumer files are genuinely reached by the walk rather than merely listed. 22 -> 33 tests. The README now states the coverage as a producer/consumer table per boundary, so the claim is checkable at a glance instead of asserted in prose. * test: list the client half of the resumable 308 contract (#1432) Addresses the third review P1 on #1717. Listing the daemon's handleResumableUpload proved it still PRODUCES 308; nothing proved the client still CONSUMES the released one. src/remote/upload-stream.ts owns that half and was entirely outside the manifest, so a newer client could stop accepting `upload-offset`, change how it reads `Range: bytes=0-N`, or emit a different resumed `Content-Range` without moving one of the 141 listed digests. Now listed (141 -> 151): UploadStreamResponse, streamFileToHttpRequest, streamFileToHttpRequestAttempt, buildUploadRequestHeaders, isUploadResumeStatus, isUploadRedirectStatus, parseUploadResumeOffset, parseNonNegativeIntegerHeader, firstHeaderValue, MAX_UPLOAD_REDIRECTS. streamFileToHttpRequestAttempt is listed despite its size, unlike createDaemonHttpServer which stays in `uncovered`. The distinction is stated at the declaration: the HTTP server only dispatches to handlers that are each digested, while the attempt loop IS the resume state machine — it decides whether a 308 continues the upload and what the next request carries, so its sequencing alone can break a released daemon while every helper keeps its digest. 6 new planted-red mutations prove the client half moves the ledger: a dropped `upload-offset` fallback, narrowed Range parsing, a changed resumed Content-Range, 308 no longer treated as continue, a narrowed UploadStreamResponse, and dropped header-value coercion. 33 -> 39 tests. Closure fail-closed surfaced two more stops: UploadStreamProgressOptions waived (local byte-progress rendering) and URL/URLSearchParams added to the global set. README now carries a `/upload` resume row in the producer/consumer table, and names the pattern behind three rounds of review: the coverage sentence kept getting written ahead of the coverage, so the table and the `uncovered` notes are the claims to trust — they are checkable against surface.ts, prose is not. --------- Co-authored-by: Claude <noreply@anthropic.com>
219 lines
9.0 KiB
TypeScript
219 lines
9.0 KiB
TypeScript
/**
|
|
* What a single named top-level declaration says: its content digest, and the
|
|
* type names it references (#1432).
|
|
*
|
|
* The daemon RPC gate needs to answer "did this wire declaration's SHAPE
|
|
* change?" without answering "did this file change?". Hashing whole files
|
|
* would fail on every unrelated edit to `packages/kernel/src/contracts.ts`,
|
|
* and a gate that cries wolf gets its baseline regenerated reflexively —
|
|
* which is exactly how a real break walks through.
|
|
*
|
|
* AST-based (`oxc-parser`, the standing precedent in this repo — see
|
|
* `scripts/layering/facade-exports.ts` for the same reasoning) rather than a
|
|
* regex over `export type X = …`: a regex has to enumerate every declaration
|
|
* FORM by hand, and the one it forgets is the one that slips through. Here it
|
|
* would also have to find the matching brace.
|
|
*/
|
|
|
|
import { createHash } from 'node:crypto';
|
|
import { parseSync } from 'oxc-parser';
|
|
|
|
type Span = { start: number; end: number };
|
|
type Node = Record<string, unknown>;
|
|
|
|
/**
|
|
* Comments and formatting are stripped before hashing, so reflowing a type or
|
|
* rewriting the prose above a field does not move the digest — only the
|
|
* declaration's tokens do. Whitespace collapses to a single space rather than
|
|
* to nothing, so removing a comment cannot fuse two adjacent tokens into one.
|
|
*
|
|
* The normalization is deliberately one-directional: it can only make the
|
|
* digest LESS sensitive (a string literal carrying a run of spaces would
|
|
* normalize), never more. No wire declaration in the manifest contains one,
|
|
* and a miss still leaves a hand-edited ledger line in the diff rather than a
|
|
* silent pass.
|
|
*/
|
|
function normalize(source: string, span: Span, comments: readonly Span[]): string {
|
|
let text = source.slice(span.start, span.end);
|
|
// Right-to-left so each splice leaves earlier offsets valid.
|
|
const inner = comments
|
|
.filter((comment) => comment.start >= span.start && comment.end <= span.end)
|
|
.sort((a, b) => b.start - a.start);
|
|
for (const comment of inner) {
|
|
text = `${text.slice(0, comment.start - span.start)} ${text.slice(comment.end - span.start)}`;
|
|
}
|
|
return text.replace(/\s+/g, ' ').trim();
|
|
}
|
|
|
|
/**
|
|
* The statement that declares `name`, or null when this one does not.
|
|
*
|
|
* An `export` wrapper stays inside the returned span on purpose: a wire type
|
|
* that stops being exported is a compatibility change for every consumer that
|
|
* imports it, so it must move the digest rather than pass unnoticed.
|
|
*/
|
|
function matchDeclaration(statement: Node, name: string, file: string): Node | null {
|
|
const inner = (statement.declaration ?? statement) as Node;
|
|
|
|
if (inner.type === 'VariableDeclaration') {
|
|
const declarators = inner.declarations as Array<{ id?: { name?: string } }>;
|
|
if (!declarators.some((declarator) => declarator.id?.name === name)) return null;
|
|
if (declarators.length > 1) {
|
|
throw new Error(
|
|
`${file}#${name} shares one statement with ${declarators.length - 1} sibling declarator(s), ` +
|
|
`so its digest could not name only "${name}". Give each wire constant its own statement.`,
|
|
);
|
|
}
|
|
return statement;
|
|
}
|
|
|
|
const id = inner.id as { name?: string } | undefined;
|
|
return id?.name === name ? statement : null;
|
|
}
|
|
|
|
function topLevelStatements(file: string, source: string): { body: Node[]; comments: Span[] } {
|
|
const parsed = parseSync(file, source);
|
|
return {
|
|
body: parsed.program.body as unknown as Node[],
|
|
comments: parsed.comments as unknown as Span[],
|
|
};
|
|
}
|
|
|
|
/**
|
|
* Every top-level name a module declares. The closure check needs this to ask
|
|
* "is the type this wire declaration references declared in a file the manifest
|
|
* already draws from?", which is what turns an omitted sibling into a failure.
|
|
*/
|
|
export function readTopLevelDeclarationNames(file: string, source: string): string[] {
|
|
const names: string[] = [];
|
|
for (const statement of topLevelStatements(file, source).body) {
|
|
const inner = (statement.declaration ?? statement) as Node;
|
|
if (inner.type === 'VariableDeclaration') {
|
|
for (const declarator of inner.declarations as Array<{ id?: { name?: string } }>) {
|
|
if (declarator.id?.name) names.push(declarator.id.name);
|
|
}
|
|
continue;
|
|
}
|
|
const name = (inner.id as { name?: string } | undefined)?.name;
|
|
if (name) names.push(name);
|
|
}
|
|
return names;
|
|
}
|
|
|
|
function findDeclaration(
|
|
file: string,
|
|
source: string,
|
|
name: string,
|
|
): { node: Node; span: Span; comments: readonly Span[] } {
|
|
const parsed = topLevelStatements(file, source);
|
|
for (const statement of parsed.body) {
|
|
const node = matchDeclaration(statement, name, file);
|
|
if (node) return { node, span: node as unknown as Span, comments: parsed.comments };
|
|
}
|
|
throw new Error(
|
|
`${file} no longer declares "${name}", which test/wire-compat/surface.ts lists as daemon RPC ` +
|
|
`wire surface. Renaming or removing a wire declaration is a protocol break: point the ` +
|
|
`manifest at the new declaration and follow test/wire-compat/README.md.`,
|
|
);
|
|
}
|
|
|
|
/**
|
|
* Digest of `name` as declared in `source`. Throws when the declaration is
|
|
* absent — the manifest naming a symbol its file no longer declares has to
|
|
* fail loudly rather than silently digest nothing.
|
|
*/
|
|
export function digestDeclaration(file: string, source: string, name: string): string {
|
|
const { span, comments } = findDeclaration(file, source, name);
|
|
return `sha256:${createHash('sha256')
|
|
.update(normalize(source, span, comments))
|
|
.digest('hex')}`;
|
|
}
|
|
|
|
/**
|
|
* Replaces `from` with `to` **inside `name`'s declaration only**, returning the
|
|
* whole mutated source.
|
|
*
|
|
* The planted-red proofs need this rather than `source.replace`: sibling
|
|
* declarations in the same file share substrings (two functions in
|
|
* `request-progress-protocol.ts` both end their template with the same framing),
|
|
* so a whole-file replace silently mutates the FIRST match and leaves the
|
|
* declaration under test untouched — a vacuous proof that looks like a real one.
|
|
* Throws when `from` is absent from the span, so that case fails loudly.
|
|
*/
|
|
export function replaceInDeclaration(
|
|
file: string,
|
|
source: string,
|
|
name: string,
|
|
from: string,
|
|
to: string,
|
|
): string {
|
|
const { span } = findDeclaration(file, source, name);
|
|
const declaration = source.slice(span.start, span.end);
|
|
if (!declaration.includes(from)) {
|
|
throw new Error(
|
|
`${file}#${name} does not contain ${JSON.stringify(from)}, so a mutation built on it would ` +
|
|
`be a no-op. Re-point the case at the declaration's current text.`,
|
|
);
|
|
}
|
|
return source.slice(0, span.start) + declaration.replace(from, to) + source.slice(span.end);
|
|
}
|
|
|
|
function walk(node: unknown, visit: (node: Node) => void): void {
|
|
if (!node || typeof node !== 'object') return;
|
|
if (Array.isArray(node)) {
|
|
for (const child of node) walk(child, visit);
|
|
return;
|
|
}
|
|
visit(node as Node);
|
|
for (const value of Object.values(node as Node)) walk(value, visit);
|
|
}
|
|
|
|
/** Left-most identifier of a type name (`A` in `A.B.C`), or null. */
|
|
function rootTypeName(typeName: unknown): string | null {
|
|
let current = typeName as Node | undefined;
|
|
while (current?.type === 'TSQualifiedName') current = current.left as Node;
|
|
const name = (current as { name?: unknown } | undefined)?.name;
|
|
return typeof name === 'string' ? name : null;
|
|
}
|
|
|
|
/**
|
|
* Every type name `name`'s declaration references, including the operand of a
|
|
* `typeof` query so alias-of-const unions (`(typeof LEASE_BACKENDS)[number]`)
|
|
* report the const they are derived from.
|
|
*
|
|
* This is what lets the manifest's closure be CHECKED rather than asserted: a
|
|
* wire type that grows a field typed by a sibling declaration in the same file
|
|
* names that sibling here, and the gate can refuse a manifest that omits it.
|
|
* Without it, adding `foo?: NewShape` to `DaemonRequestMeta` would move only
|
|
* `DaemonRequestMeta`'s digest and leave `NewShape` — the thing that actually
|
|
* decides what the peer parses — outside the gate entirely.
|
|
*/
|
|
export function readTypeReferences(file: string, source: string, name: string): string[] {
|
|
const { node } = findDeclaration(file, source, name);
|
|
const names = new Set<string>();
|
|
// A declaration's own generic parameters (`JsonRpcRequestEnvelope<TParams>`)
|
|
// read as type references but have no declaration site to gate — they are
|
|
// bound right here. Collected across the whole subtree so a nested generic
|
|
// helper's parameters drop out too.
|
|
const typeParameters = new Set<string>();
|
|
walk(node, (child) => {
|
|
if (child.type === 'TSTypeParameter') {
|
|
const bound = (child.name as { name?: unknown } | undefined)?.name;
|
|
if (typeof bound === 'string') typeParameters.add(bound);
|
|
}
|
|
if (child.type === 'TSTypeReference') {
|
|
const referenced = rootTypeName(child.typeName);
|
|
if (referenced) names.add(referenced);
|
|
}
|
|
if (child.type === 'TSTypeQuery') {
|
|
const referenced = rootTypeName(child.exprName);
|
|
if (referenced) names.add(referenced);
|
|
}
|
|
});
|
|
names.delete(name);
|
|
// `x as const` parses as a type reference to the contextual keyword.
|
|
names.delete('const');
|
|
for (const bound of typeParameters) names.delete(bound);
|
|
return [...names].sort();
|
|
}
|