mirror of
https://github.com/morluto/rea.git
synced 2026-11-15 11:13:21 +00:00
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
This commit is contained in:
1 parent
89640a446a
commit
cd5ea9a9fb
5 files changed
+180
-4
No files matched your search
@@ -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",
|
||||
|
||||
@@ -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<string | number, unknown>)[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<Record<string, JsonValue>> => {
|
||||
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 = <T>(
|
||||
operation: string,
|
||||
@@ -11,8 +90,6 @@ export const invalidNativeOutput = <T>(
|
||||
new AnalysisOutputError(
|
||||
operation,
|
||||
`Provider returned an invalid ${expected}`,
|
||||
{
|
||||
cause,
|
||||
},
|
||||
{ cause, diagnostics: outputContractDiagnostics(cause) },
|
||||
),
|
||||
);
|
||||
@@ -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;
|
||||
|
||||
@@ -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 = <M extends HopperReplyMethod>(
|
||||
new AnalysisOutputError(
|
||||
operation,
|
||||
`Hopper bridge output did not match the ${method} contract`,
|
||||
{ cause: parsed.error },
|
||||
{
|
||||
cause: parsed.error,
|
||||
diagnostics: outputContractDiagnostics(parsed.error, value),
|
||||
},
|
||||
),
|
||||
);
|
||||
};
|
||||
@@ -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 }],
|
||||
|
||||
Reference in new issue
Block a user