diff --git a/CHANGELOG.md b/CHANGELOG.md index f1e2304..5c9663c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ - Fixed Fastify route-object mapping to emit static method arrays while ignoring dynamic entries, thanks @rohitjavvadi. - Fixed Fastify plugin callback route mapping for typed parameters and plugin aliases, thanks @rohitjavvadi. - Fixed Bun package-manager detection to recognize the text `bun.lock` lockfile, thanks @austinm911. +- Fixed review-output schema to tolerate optional `reproduction` and `minimumFixScope` fields and zero-valued evidence line numbers (normalized to `null`), recovering 4 of 28 zod issue patterns observed in run `20260517T190759-3c9e9e` (78 errors over 1000 features) that previously dropped whole-feature output instead of the affected finding. ## 0.3.0 - 2026-05-18 diff --git a/src/provider-schema.ts b/src/provider-schema.ts index 78d25ba..a43bba7 100644 --- a/src/provider-schema.ts +++ b/src/provider-schema.ts @@ -21,7 +21,9 @@ export const revalidateJsonSchema = providerJsonSchema(revalidateOutputSchema); export const fixPlanJsonSchema = providerJsonSchema(fixPlanOutputSchema); export function providerJsonSchema(schema: z.ZodType): object { - return stripProviderUnsupportedSchemaKeywords(z.toJSONSchema(schema)) as object; + return stripProviderUnsupportedSchemaKeywords( + z.toJSONSchema(schema, { io: "input", unrepresentable: "any" }), + ) as object; } function stripProviderUnsupportedSchemaKeywords(value: unknown): unknown { @@ -38,5 +40,13 @@ function stripProviderUnsupportedSchemaKeywords(value: unknown): unknown { } output[key] = stripProviderUnsupportedSchemaKeywords(item); } + if (output["type"] === "object" && isRecord(output["properties"])) { + output["additionalProperties"] = false; + output["required"] = Object.keys(output["properties"]); + } return output; } + +function isRecord(value: unknown): value is Record { + return typeof value === "object" && value !== null && !Array.isArray(value); +} diff --git a/src/provider.test.ts b/src/provider.test.ts index 9b41a10..5be14fc 100644 --- a/src/provider.test.ts +++ b/src/provider.test.ts @@ -2,7 +2,7 @@ import { afterEach, describe, expect, it } from "vitest"; import { ClawpatchError } from "./errors.js"; import { __testing, extractJson, providerByName } from "./provider.js"; import { safeProviderPreview } from "./provider-json.js"; -import { revalidateOutputSchema, reviewOutputSchema } from "./types.js"; +import { evidenceRefSchema, revalidateOutputSchema, reviewOutputSchema } from "./types.js"; // eslint-disable-next-line no-underscore-dangle const { @@ -260,6 +260,20 @@ describe("providerJsonSchema", () => { expect(enumNodes.every((node) => node["type"] === "string")).toBe(true); } }); + + it("keeps object schemas strict even when parser input fields are optional", () => { + const schema = providerJsonSchema(reviewOutputSchema) as Record; + const findings = propertySchema(schema, "findings"); + const finding = itemSchema(findings); + const inspected = propertySchema(schema, "inspected"); + + for (const objectSchema of [schema, finding, inspected]) { + expect(objectSchema["additionalProperties"]).toBe(false); + expect(objectSchema["required"]).toEqual(Object.keys(propertiesOf(objectSchema))); + } + expect(finding["required"]).toContain("reproduction"); + expect(finding["required"]).toContain("minimumFixScope"); + }); }); describe("piThinkingLevel", () => { @@ -294,6 +308,30 @@ function enumSchemaNodes(value: unknown): Array> { return Array.isArray(node["enum"]) ? [node, ...nested] : nested; } +function propertySchema(schema: Record, name: string): Record { + const value = propertiesOf(schema)[name]; + if (typeof value !== "object" || value === null || Array.isArray(value)) { + throw new Error(`missing schema property: ${name}`); + } + return value as Record; +} + +function itemSchema(schema: Record): Record { + const value = schema["items"]; + if (typeof value !== "object" || value === null || Array.isArray(value)) { + throw new Error("missing item schema"); + } + return value as Record; +} + +function propertiesOf(schema: Record): Record { + const value = schema["properties"]; + if (typeof value !== "object" || value === null || Array.isArray(value)) { + throw new Error("missing schema properties"); + } + return value as Record; +} + describe("codexFailureMessage", () => { it("adds scope guidance for missing Responses API write permission", () => { const message = codexFailureMessage( @@ -748,6 +786,79 @@ describe("providerByName", () => { }); }); +function buildToleranceFinding(overrides: Record = {}): Record { + return { + title: "x", + category: "bug", + severity: "low", + confidence: "low", + evidence: [], + reasoning: "r", + reproduction: null, + recommendation: "rec", + whyTestsDoNotAlreadyCoverThis: "", + suggestedRegressionTest: null, + minimumFixScope: "", + ...overrides, + }; +} + +function buildToleranceOutput(finding: Record): Record { + return { + findings: [finding], + inspected: { files: [], symbols: [], notes: [] }, + }; +} + +describe("reviewOutputSchema tolerance", () => { + it("accepts findings with null reproduction", () => { + const parsed = reviewOutputSchema.parse( + buildToleranceOutput(buildToleranceFinding({ reproduction: null })), + ); + expect(parsed.findings[0]!.reproduction).toBeNull(); + }); + + it("accepts findings with omitted reproduction (becomes null)", () => { + const finding = buildToleranceFinding(); + delete finding["reproduction"]; + const parsed = reviewOutputSchema.parse(buildToleranceOutput(finding)); + expect(parsed.findings[0]!.reproduction).toBeNull(); + }); + + it("accepts findings with omitted minimumFixScope (becomes empty string)", () => { + const finding = buildToleranceFinding(); + delete finding["minimumFixScope"]; + const parsed = reviewOutputSchema.parse(buildToleranceOutput(finding)); + expect(parsed.findings[0]!.minimumFixScope).toBe(""); + }); +}); + +describe("evidenceRefSchema tolerance", () => { + it("accepts startLine 0 and normalizes to null", () => { + const parsed = evidenceRefSchema.parse({ + path: "src/index.ts", + startLine: 0, + endLine: 5, + symbol: null, + quote: null, + }); + expect(parsed.startLine).toBeNull(); + expect(parsed.endLine).toBeNull(); + }); + + it("accepts endLine 0 and normalizes to null", () => { + const parsed = evidenceRefSchema.parse({ + path: "src/index.ts", + startLine: 5, + endLine: 0, + symbol: null, + quote: null, + }); + expect(parsed.startLine).toBeNull(); + expect(parsed.endLine).toBeNull(); + }); +}); + describe("acpxPromptRetries", () => { afterEach(() => { delete process.env["CLAWPATCH_ACPX_PROMPT_RETRIES"]; diff --git a/src/types.ts b/src/types.ts index 4c958c0..70f7a78 100644 --- a/src/types.ts +++ b/src/types.ts @@ -206,13 +206,21 @@ export type FeatureRecord = z.infer; export type FeatureKind = FeatureRecord["kind"]; export type TrustBoundary = FeatureRecord["trustBoundaries"][number]; -export const evidenceRefSchema = z.object({ - path: z.string(), - startLine: z.number().int().positive().nullable(), - endLine: z.number().int().positive().nullable(), - symbol: z.string().nullable(), - quote: z.string().nullable(), -}); +const evidenceLineSchema = z.number().int().min(0).nullable(); + +export const evidenceRefSchema = z + .object({ + path: z.string(), + startLine: evidenceLineSchema, + endLine: evidenceLineSchema, + symbol: z.string().nullable(), + quote: z.string().nullable(), + }) + .transform((evidence) => + evidence.startLine === 0 || evidence.endLine === 0 + ? { ...evidence, startLine: null, endLine: null } + : evidence, + ); export const findingHistoryEntrySchema = z.object({ runId: z.string().nullable(), @@ -358,11 +366,17 @@ export const reviewOutputSchema = z.object({ confidence: z.enum(["high", "medium", "low"]), evidence: z.array(evidenceRefSchema), reasoning: z.string(), - reproduction: z.string().nullable(), + reproduction: z + .string() + .nullish() + .transform((v) => v ?? null), recommendation: z.string(), whyTestsDoNotAlreadyCoverThis: z.string(), suggestedRegressionTest: z.string().nullable(), - minimumFixScope: z.string(), + minimumFixScope: z + .string() + .nullish() + .transform((v) => v ?? ""), }), ), inspected: z.object({