From cd5ea9a9fb34107fb8f86d9bea59d59ba17c7fea Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Sun, 15 Nov 2026 10:11:08 +0800 Subject: [PATCH] fix(hopper): name the failing fields when a bridge reply misses its contract (#10713) * fix(hopper): name the failing fields when a bridge reply misses its contract * fix(ci): run the package baseline when the fake Hopper bridge changes * fix(hopper): name nested union failures and cover every native schema in the package scope --- scripts/ci/scopes.mjs | 11 +++ src/domain/native/nativeProviderOutput.ts | 83 ++++++++++++++++++- src/hopper/HopperReplyContracts.test.ts | 56 +++++++++++++ src/hopper/HopperReplyContracts.ts | 6 +- .../boundary/filesystem/ciChangeScope.test.ts | 28 +++++++ 5 files changed, 180 insertions(+), 4 deletions(-) diff --git a/scripts/ci/scopes.mjs b/scripts/ci/scopes.mjs index 20da3e3149..5dff69eb60 100644 --- a/scripts/ci/scopes.mjs +++ b/scripts/ci/scopes.mjs @@ -253,6 +253,17 @@ const PATH_SCOPES = { "scripts/lib/runtime-admission.d.mts", "install.sh", "src/config/**", + // The darwin package baseline runs packaged Hopper commands against the + // fake bridge, through the reply contracts and the domain schemas they + // are built from. Check a change to either side before merge, not after. + "tests/fixtures/fakeLauncher.mjs", + "tests/fixtures/fakeHopper*.mjs", + "bridge/hopper_bridge.py", + "src/hopper/HopperReplyContracts.ts", + "src/hopper/HopperProvider.ts", + // The contracts import native domain schemas transitively; cover them + // all rather than a hand list that drifts. + "src/domain/native/!(*.test).ts", "src/application/Setup*.ts", "src/application/Uninstall*.ts", "src/application/ClientConfiguration*.ts", diff --git a/src/domain/native/nativeProviderOutput.ts b/src/domain/native/nativeProviderOutput.ts index 9dc0ba3060..7f298d7569 100644 --- a/src/domain/native/nativeProviderOutput.ts +++ b/src/domain/native/nativeProviderOutput.ts @@ -1,6 +1,85 @@ import { z } from "zod"; import { err, type Result } from "../result.js"; import { AnalysisOutputError } from "../analysisErrorCore.js"; +import type { JsonValue } from "../jsonValue.js"; + +/** Issues kept per refusal; the count still reports every issue. */ +const MAX_CONTRACT_ISSUES = 16; + +const jsonKind = (value: unknown): string => + value === null + ? "null" + : Array.isArray(value) + ? "array" + : value === undefined + ? "missing" + : typeof value; + +const valueAt = (value: unknown, path: readonly PropertyKey[]): unknown => { + let current = value; + for (const key of path) { + if (typeof current !== "object" || current === null) return undefined; + if (typeof key === "symbol") return undefined; + current = (current as Record)[key]; + } + return current; +}; + +interface LocatedIssue { + readonly issue: z.core.$ZodIssue; + readonly path: readonly PropertyKey[]; + /** The union option index chosen at each enclosing failed union. */ + readonly branches: readonly number[]; +} + +/** + * Replace each failed union with the issues of every option it tried, located + * at their full path, so a nested field failure stays named. + */ +const locatedIssues = ( + issues: readonly z.core.$ZodIssue[], + prefix: readonly PropertyKey[] = [], + branches: readonly number[] = [], +): LocatedIssue[] => + issues.flatMap((issue) => { + const path = [...prefix, ...issue.path]; + return issue.code === "invalid_union" && issue.errors.length > 0 + ? issue.errors.flatMap((option, index) => + locatedIssues(option, path, [...branches, index]), + ) + : [{ issue, path, branches }]; + }); + +/** + * Name the fields a provider reply failed its contract on. Each issue carries + * only its field path, Zod issue code, the expected kind and the observed JSON + * kind, never reported text, so the diagnostic is safe to return to callers. + * Issues inside a failed union carry the option index at each union level. + */ +export const outputContractDiagnostics = ( + cause: z.ZodError, + value?: unknown, +): Readonly> => { + const located = locatedIssues(cause.issues); + return { + contract_issue_count: located.length, + contract_issues: located + .slice(0, MAX_CONTRACT_ISSUES) + .map(({ issue, path, branches }) => ({ + path: path.map((key) => (typeof key === "number" ? key : String(key))), + code: issue.code, + ...(branches.length === 0 ? {} : { union_branches: [...branches] }), + ...(issue.code === "invalid_type" + ? { + expected: issue.expected, + ...(value === undefined + ? {} + : { received: jsonKind(valueAt(value, path)) }), + } + : {}), + })), + }; +}; export const invalidNativeOutput = ( operation: string, @@ -11,8 +90,6 @@ export const invalidNativeOutput = ( new AnalysisOutputError( operation, `Provider returned an invalid ${expected}`, - { - cause, - }, + { cause, diagnostics: outputContractDiagnostics(cause) }, ), ); diff --git a/src/hopper/HopperReplyContracts.test.ts b/src/hopper/HopperReplyContracts.test.ts index 0dccba573f..014846d1d1 100644 --- a/src/hopper/HopperReplyContracts.test.ts +++ b/src/hopper/HopperReplyContracts.test.ts @@ -51,6 +51,62 @@ it("names the requesting operation and the bridge method in a contract failure", }); }); +it("names each failing field without echoing reported text", () => { + const region = { + name: "secret-name", + start: "0x0", + end: "0x10", + readable: null, + writable: null, + executable: null, + permissions: { available: false, reason: "unavailable" }, + provenance: "hopper-public-python-api", + }; + + const parsed = parseHopperReply("list_segments", [ + { ...region, sections: [{ ...region, name: null }] }, + ]); + + const error = parsed.ok ? undefined : parsed.error; + expect(error?.diagnostics).toEqual({ + contract_issue_count: 1, + contract_issues: [ + { + path: [0, "sections", 0, "name"], + code: "invalid_type", + expected: "string", + received: "null", + }, + ], + }); + expect(JSON.stringify(error?.diagnostics)).not.toContain("secret-name"); +}); + +it("names the nested field of each option a failed union tried", () => { + const parsed = parseHopperReply("list_segments", [ + { + name: "segment", + start: "0x0", + end: "0x10", + readable: null, + writable: null, + executable: null, + permissions: { available: false, reason: null }, + provenance: "hopper-public-python-api", + sections: [], + }, + ]); + + const diagnostics = parsed.ok ? undefined : parsed.error.diagnostics; + expect(diagnostics?.["contract_issues"]).toContainEqual({ + path: [0, "permissions", "reason"], + code: "invalid_type", + union_branches: [0], + expected: "string", + received: "null", + }); +}); + const providerClient = () => { const config = parseConfig({}); if (!config.ok) throw config.error; diff --git a/src/hopper/HopperReplyContracts.ts b/src/hopper/HopperReplyContracts.ts index 7909c8bcfb..c8f0e51686 100644 --- a/src/hopper/HopperReplyContracts.ts +++ b/src/hopper/HopperReplyContracts.ts @@ -17,6 +17,7 @@ import { referenceEdgeSchema, unresolvedCallSchema, } from "../domain/native/nativeReference.js"; +import { outputContractDiagnostics } from "../domain/native/nativeProviderOutput.js"; import { nativeSegmentListSchema } from "../domain/native/nativeSegments.js"; import { procedureIdentitySchema, @@ -163,7 +164,10 @@ export const parseHopperReply = ( new AnalysisOutputError( operation, `Hopper bridge output did not match the ${method} contract`, - { cause: parsed.error }, + { + cause: parsed.error, + diagnostics: outputContractDiagnostics(parsed.error, value), + }, ), ); }; diff --git a/tests/boundary/filesystem/ciChangeScope.test.ts b/tests/boundary/filesystem/ciChangeScope.test.ts index f741deb77c..7815126e42 100644 --- a/tests/boundary/filesystem/ciChangeScope.test.ts +++ b/tests/boundary/filesystem/ciChangeScope.test.ts @@ -692,6 +692,34 @@ it.each([ "tests/module/example.test.ts", { code: true, runtime: false, browser: false, package: false }, ], + // The darwin package baseline is the CI run of packaged Hopper commands + // against the fake bridge and the reply contracts. + [ + "tests/fixtures/fakeHopper.mjs", + { code: true, runtime: true, package: true }, + ], + [ + "tests/fixtures/fakeHopperReplies.mjs", + { code: true, runtime: true, package: true }, + ], + [ + "src/hopper/HopperReplyContracts.ts", + { code: true, runtime: true, package: true }, + ], + [ + "src/domain/native/nativeSegments.ts", + { code: true, runtime: true, package: true }, + ], + // Schemas the contracts import transitively. + [ + "src/domain/native/functionBody.ts", + { code: true, runtime: true, package: true }, + ], + [ + "src/domain/native/nativeSymbol.ts", + { code: true, runtime: true, package: true }, + ], + ["src/domain/native/functionBody.test.ts", { code: true, package: false }], ["scripts/lib/release-proposal.mjs", { code: true, package: true }], ["scripts/lib/release-proposal.d.mts", { code: true, package: true }], ["scripts/lib/release-pull-request.mjs", { code: true, package: true }],