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>
223 lines
7.1 KiB
TypeScript
223 lines
7.1 KiB
TypeScript
// Catalog for the check-affected selector: how each derived CheckId maps to a
|
|
// runnable command and the authoritative GitHub CI job(s) it mirrors.
|
|
//
|
|
// Commands are resolved from real package.json scripts or Vitest's native
|
|
// affected-test command, so this stays a thin projection over existing
|
|
// aggregate checks rather than a second source of truth for how to run them.
|
|
|
|
import { ALL_CHECKS, type CheckId } from './model.ts';
|
|
|
|
export type CheckKind =
|
|
| { readonly type: 'script'; readonly script: string }
|
|
| { readonly type: 'vitest-related' };
|
|
|
|
export type CheckSpec = {
|
|
readonly id: CheckId;
|
|
readonly label: string;
|
|
readonly kind: CheckKind;
|
|
readonly ciJobs: readonly string[];
|
|
// Whether `--run` should attempt the check locally. Device/emulator lanes and
|
|
// network/toolchain-gated lanes stay authoritative on GitHub CI.
|
|
readonly localRunnable: boolean;
|
|
};
|
|
|
|
export const CHECK_CATALOG: readonly CheckSpec[] = [
|
|
{
|
|
id: 'format',
|
|
label: 'Formatting (oxfmt)',
|
|
kind: { type: 'script', script: 'format:check' },
|
|
ciJobs: ['Lint & Format'],
|
|
localRunnable: true,
|
|
},
|
|
{
|
|
id: 'lint',
|
|
label: 'Lint (oxlint)',
|
|
kind: { type: 'script', script: 'lint' },
|
|
ciJobs: ['Lint & Format'],
|
|
localRunnable: true,
|
|
},
|
|
{
|
|
id: 'typecheck',
|
|
label: 'Typecheck (tsc)',
|
|
kind: { type: 'script', script: 'typecheck' },
|
|
ciJobs: ['Typecheck'],
|
|
localRunnable: true,
|
|
},
|
|
{
|
|
id: 'test-app-typecheck',
|
|
label: 'Expo test app typecheck',
|
|
kind: { type: 'script', script: 'test-app:typecheck' },
|
|
ciJobs: ['Resolve native fingerprint'],
|
|
// The test app intentionally owns a separate Expo dependency graph. Do
|
|
// not make every root-checkout validation install it implicitly.
|
|
localRunnable: false,
|
|
},
|
|
{
|
|
id: 'layering',
|
|
label: 'Import-direction layering guard',
|
|
kind: { type: 'script', script: 'check:layering' },
|
|
ciJobs: ['Layering Guard'],
|
|
localRunnable: true,
|
|
},
|
|
{
|
|
id: 'fallow',
|
|
label: 'Fallow code-quality audit',
|
|
kind: { type: 'script', script: 'check:fallow' },
|
|
ciJobs: ['Fallow Code Quality'],
|
|
localRunnable: true,
|
|
},
|
|
{
|
|
id: 'mcp-metadata',
|
|
label: 'MCP registry metadata sync',
|
|
kind: { type: 'script', script: 'check:mcp-metadata' },
|
|
ciJobs: ['Typecheck'],
|
|
localRunnable: true,
|
|
},
|
|
{
|
|
id: 'build',
|
|
label: 'Build (tsdown + declarations)',
|
|
kind: { type: 'script', script: 'build' },
|
|
ciJobs: ['Packaged CLI Node 22.12'],
|
|
localRunnable: true,
|
|
},
|
|
{
|
|
id: 'package',
|
|
label: 'Published package (publint, attw, clean-install resolution)',
|
|
kind: { type: 'script', script: 'check:package' },
|
|
ciJobs: ['Packaged CLI Node 22.12'],
|
|
// Needs a `pnpm build` output and the npm registry, both of which local runs already have.
|
|
localRunnable: true,
|
|
},
|
|
{
|
|
id: 'integration-node',
|
|
label: 'Node integration smoke',
|
|
kind: { type: 'script', script: 'test:integration:node' },
|
|
ciJobs: ['Integration Tests'],
|
|
localRunnable: true,
|
|
},
|
|
{
|
|
id: 'vitest-related',
|
|
label: 'Tests related by Vitest module graph',
|
|
kind: { type: 'vitest-related' },
|
|
ciJobs: ['Coverage'],
|
|
localRunnable: true,
|
|
},
|
|
{
|
|
id: 'unit',
|
|
label: 'Unit + smoke suite',
|
|
kind: { type: 'script', script: 'check:unit' },
|
|
ciJobs: ['Coverage', 'Integration Tests'],
|
|
localRunnable: true,
|
|
},
|
|
{
|
|
id: 'coverage',
|
|
label: 'Affected LCOV + changed-line coverage',
|
|
kind: { type: 'script', script: 'check:coverage-changed' },
|
|
ciJobs: ['Coverage'],
|
|
localRunnable: true,
|
|
},
|
|
{
|
|
id: 'provider-integration',
|
|
label: 'Provider-backed integration suite',
|
|
kind: { type: 'script', script: 'test:integration:provider' },
|
|
ciJobs: ['Integration Tests', 'Coverage'],
|
|
localRunnable: true,
|
|
},
|
|
{
|
|
id: 'integration-progress',
|
|
label: 'Integration architecture-progress gate',
|
|
kind: { type: 'script', script: 'test:integration:progress:check' },
|
|
ciJobs: ['Integration Tests'],
|
|
localRunnable: true,
|
|
},
|
|
{
|
|
id: 'swift-runner',
|
|
label: 'Swift runner build',
|
|
kind: { type: 'script', script: 'build:xcuitest' },
|
|
ciJobs: ['Swift Runner Unit Compile', 'iOS / Smoke Tests', 'macOS / Smoke Tests'],
|
|
localRunnable: false,
|
|
},
|
|
{
|
|
id: 'android-helpers',
|
|
label: 'Android helper builds',
|
|
kind: { type: 'script', script: 'build:android-snapshot-helper' },
|
|
ciJobs: ['Android / Smoke Tests'],
|
|
localRunnable: false,
|
|
},
|
|
{
|
|
id: 'macos-helper',
|
|
label: 'macOS helper build',
|
|
kind: { type: 'script', script: 'build:macos-helper' },
|
|
ciJobs: ['macOS / Smoke Tests'],
|
|
localRunnable: false,
|
|
},
|
|
{
|
|
id: 'web-smoke',
|
|
label: 'Live web platform smoke',
|
|
kind: { type: 'script', script: 'test:smoke:web' },
|
|
ciJobs: ['Web Platform Smoke'],
|
|
localRunnable: false,
|
|
},
|
|
{
|
|
id: 'replay-compat',
|
|
label: 'Replay-compat corpus provenance (released blobs)',
|
|
kind: { type: 'script', script: 'check:replay-compat' },
|
|
// Needs full history and tags, so it runs in its own fetch-depth: 0 job
|
|
// rather than inside the shallow-clone-safe unit lane.
|
|
ciJobs: ['Replay-Compat Provenance'],
|
|
localRunnable: true,
|
|
},
|
|
{
|
|
id: 'daemon-wire-compat',
|
|
label: 'Daemon RPC wire surface vs. last released tag',
|
|
kind: { type: 'script', script: 'check:daemon-wire-compat' },
|
|
// Same shape as replay-compat: the released ledger is only readable from a
|
|
// full-history checkout, so this cannot live in the shallow unit lane.
|
|
ciJobs: ['Released-Surface Compatibility'],
|
|
localRunnable: true,
|
|
},
|
|
];
|
|
|
|
export function getCheckSpec(id: CheckId): CheckSpec {
|
|
const spec = CHECK_CATALOG.find((entry) => entry.id === id);
|
|
if (!spec) throw new Error(`No catalog entry for check "${id}".`);
|
|
return spec;
|
|
}
|
|
|
|
// Resolve the runnable command for a check. Script-backed checks are validated
|
|
// against package.json so a renamed/removed script fails loudly instead of
|
|
// silently skipping a gate. `fallow` threads the same --base the audit uses.
|
|
export function resolveCommand(
|
|
spec: CheckSpec,
|
|
scripts: Readonly<Record<string, string>>,
|
|
base: string,
|
|
changedFiles: readonly string[] = [],
|
|
): string[] {
|
|
if (spec.kind.type === 'vitest-related') {
|
|
return ['pnpm', 'exec', 'vitest', 'related', '--run', '--passWithNoTests', ...changedFiles];
|
|
}
|
|
const { script } = spec.kind;
|
|
if (!(script in scripts)) {
|
|
throw new Error(
|
|
`Check "${spec.id}" references package.json script "${script}", which does not exist.`,
|
|
);
|
|
}
|
|
const command = ['pnpm', 'run', script];
|
|
if (spec.id === 'fallow') command.push('--base', base);
|
|
return command;
|
|
}
|
|
|
|
// Guard: the catalog must cover exactly the CheckId universe. The self-test
|
|
// asserts this so a new check cannot ship half-wired.
|
|
export function assertCatalogComplete(): void {
|
|
const catalogIds = new Set(CHECK_CATALOG.map((entry) => entry.id));
|
|
const missing = ALL_CHECKS.filter((id) => !catalogIds.has(id));
|
|
const extra = CHECK_CATALOG.filter((entry) => !ALL_CHECKS.includes(entry.id)).map((e) => e.id);
|
|
if (missing.length > 0 || extra.length > 0) {
|
|
throw new Error(
|
|
`Check catalog out of sync with ALL_CHECKS. Missing: [${missing.join(', ')}]; ` +
|
|
`extra: [${extra.join(', ')}].`,
|
|
);
|
|
}
|
|
}
|