fix: surface specific sub-issue for config validation union errors (#40841)

Merged via squash.

Prepared head SHA: 6d7da51629
Co-authored-by: Hollychou924 <128659251+Hollychou924@users.noreply.github.com>
Co-authored-by: altaywtf <9790196+altaywtf@users.noreply.github.com>
Reviewed-by: @altaywtf
This commit is contained in:
HollyChou
2026-04-09 16:40:22 +08:00
committed by GitHub
parent 89acb92011
commit ab49afcd27
3 changed files with 185 additions and 0 deletions

View File

@@ -70,4 +70,79 @@ describe("config validation allowed-values metadata", () => {
expect(issue?.message).not.toContain("(allowed:");
}
});
it("surfaces specific sub-issue for invalid_union bindings errors instead of generic 'Invalid input'", () => {
const result = validateConfigObjectRaw({
bindings: [
{
type: "acp",
agentId: "test",
match: { channel: "discord", peer: { kind: "direct", id: "123" } },
acp: { agent: "claude" },
},
],
});
expect(result.ok).toBe(false);
if (!result.ok) {
expect(result.issues).not.toContainEqual({
path: "bindings.0",
message: "Invalid input",
});
expect(result.issues).toContainEqual({
path: "bindings.0.acp",
message: 'Unrecognized key: "agent"',
});
}
});
it("prefers the matching union branch for top-level unexpected keys", () => {
const result = validateConfigObjectRaw({
bindings: [
{
type: "acp",
agentId: "test",
match: { channel: "discord", peer: { kind: "direct", id: "123" } },
acp: { mode: "persistent" },
extraTopLevel: true,
},
],
});
expect(result.ok).toBe(false);
if (!result.ok) {
expect(result.issues).not.toContainEqual({
path: "bindings.0.type",
message: 'Invalid input: expected "route"',
});
expect(result.issues).toContainEqual({
path: "bindings.0",
message: 'Unrecognized key: "extraTopLevel"',
});
}
});
it("keeps generic union messaging for mixed scalar-or-object unions", () => {
const result = validateConfigObjectRaw({
agents: {
list: [{ id: "a", model: true }],
},
});
expect(result.ok).toBe(false);
if (!result.ok) {
expect(result.issues).not.toContainEqual({
path: "agents.list.0.model",
message: "Invalid input: expected string, received boolean",
});
expect(result.issues).not.toContainEqual({
path: "agents.list.0.model",
message: "Invalid input: expected object, received boolean",
});
expect(result.issues).toContainEqual({
path: "agents.list.0.model",
message: "Invalid input",
});
}
});
});

View File

@@ -262,6 +262,99 @@ function collectAllowedValuesFromUnknownIssue(issue: unknown): unknown[] {
return collection.values;
}
function isBindingsIssuePath(pathSegments: readonly ConfigPathSegment[]): boolean {
return pathSegments[0] === "bindings" && typeof pathSegments[1] === "number";
}
function isRouteTypeMismatchIssue(issue: UnknownIssueRecord): boolean {
const issuePath = toConfigPathSegments(issue.path);
if (issuePath.length !== 1 || issuePath[0] !== "type") {
return false;
}
if (issue.code !== "invalid_value" || !Array.isArray(issue.values)) {
return false;
}
return issue.values.includes("route");
}
function extractBindingsSpecificUnionIssue(
record: UnknownIssueRecord,
parentPath: string,
): ConfigValidationIssue | null {
if (!isBindingsIssuePath(toConfigPathSegments(record.path)) || !Array.isArray(record.errors)) {
return null;
}
let matchingBranchIssue: UnknownIssueRecord | null = null;
let matchingBranchIsUnrecognized = false;
let matchingBranchPathLen = -1;
let sawRouteTypeMismatch = false;
for (const errGroup of record.errors) {
if (!Array.isArray(errGroup)) {
continue;
}
const branch = errGroup
.map((issue) => toIssueRecord(issue))
.filter(Boolean) as UnknownIssueRecord[];
if (branch.length === 0) {
continue;
}
if (branch.some((issue) => isRouteTypeMismatchIssue(issue))) {
sawRouteTypeMismatch = true;
continue;
}
let branchBestIssue: UnknownIssueRecord | null = null;
let branchBestIsUnrecognized = false;
let branchBestPathLen = -1;
for (const issue of branch) {
const issueCode = typeof issue.code === "string" ? issue.code : "";
const issuePathLen = toConfigPathSegments(issue.path).length;
const issueIsUnrecognized = issueCode === "unrecognized_keys";
const issueIsBetter =
issuePathLen > branchBestPathLen
? true
: issuePathLen === branchBestPathLen && issueIsUnrecognized && !branchBestIsUnrecognized;
if (issueIsBetter) {
branchBestIssue = issue;
branchBestIsUnrecognized = issueIsUnrecognized;
branchBestPathLen = issuePathLen;
}
}
if (!branchBestIssue) {
continue;
}
if (matchingBranchIssue) {
return null;
}
matchingBranchIssue = branchBestIssue;
matchingBranchIsUnrecognized = branchBestIsUnrecognized;
matchingBranchPathLen = branchBestPathLen;
}
if (!sawRouteTypeMismatch || !matchingBranchIssue) {
return null;
}
if (matchingBranchPathLen === 0 && !matchingBranchIsUnrecognized) {
return null;
}
const subPath = formatConfigPath(toConfigPathSegments(matchingBranchIssue.path));
const fullPath = parentPath && subPath ? `${parentPath}.${subPath}` : parentPath || subPath;
const subMessage =
typeof matchingBranchIssue.message === "string" ? matchingBranchIssue.message : "Invalid input";
return { path: fullPath, message: subMessage };
}
function isObjectSecretRefCandidate(value: unknown): boolean {
if (!value || typeof value !== "object" || Array.isArray(value)) {
return false;
@@ -354,8 +447,24 @@ function mapZodIssueToConfigIssue(issue: unknown): ConfigValidationIssue {
const record = toIssueRecord(issue);
const path = formatConfigPath(toConfigPathSegments(record?.path));
const message = typeof record?.message === "string" ? record.message : "Invalid input";
const allowedValuesSummary = summarizeAllowedValues(collectAllowedValuesFromUnknownIssue(issue));
// Bindings use a plain union because legacy route bindings may omit `type`.
// When an explicit ACP binding fails strict-object checks, Zod collapses the
// useful ACP branch issue behind a generic union-level "Invalid input".
if (
record &&
typeof record.code === "string" &&
record.code === "invalid_union" &&
!allowedValuesSummary
) {
const betterIssue = extractBindingsSpecificUnionIssue(record, path);
if (betterIssue) {
return betterIssue;
}
}
if (!allowedValuesSummary) {
return { path, message };
}