diff --git a/packages/coding-agent/src/tools/output-schema-validator.ts b/packages/coding-agent/src/tools/output-schema-validator.ts index ae9e9afd7..f09b469c9 100644 --- a/packages/coding-agent/src/tools/output-schema-validator.ts +++ b/packages/coding-agent/src/tools/output-schema-validator.ts @@ -139,14 +139,38 @@ interface SectionLabelMetadata { isKnown(label: string): boolean; } +/** + * Derive incremental-label metadata from top-level schema closure. + * + * The unknown-label gate (`rejectUnknownSections`) engages when the schema constrains top-level + * property names anywhere: a closed conjunct (root or recursive `allOf` child with + * `additionalProperties: false`) or a `oneOf`/`anyOf` union whose EVERY variant is closed. A label + * is known iff every closed conjunct accepts it AND, per closed union, at least one variant + * accepts it (union semantics are disjunctive — the assembled output only has to match one + * variant). Unions containing any open variant never gate: the open variant accepts arbitrary + * labels, so rejection would be a false positive. + */ function buildSectionLabelMetadata(jsonSchema: Record): SectionLabelMetadata { - const closedSchemas = collectClosedTopLevelSchemas(jsonSchema); - const labels = [...new Set(closedSchemas.flatMap(schema => declaredPropertyLabels(schema)))]; + const closedConjuncts = collectClosedTopLevelSchemas(jsonSchema); + const closedUnions = collectClosedTopLevelUnions(jsonSchema); + const closed = closedConjuncts.length > 0 || closedUnions.length > 0; + const acceptedByAll = (conjuncts: readonly Record[], label: string): boolean => + conjuncts.every(schema => schemaAcceptsSectionLabel(schema, label)); + const labels = [ + ...new Set([ + ...closedConjuncts.flatMap(schema => declaredPropertyLabels(schema)), + ...closedUnions.flatMap(variants => + variants.flatMap(conjuncts => conjuncts.flatMap(schema => declaredPropertyLabels(schema))), + ), + ]), + ]; return { labels, - rejectUnknownSections: closedSchemas.length > 0, + rejectUnknownSections: closed, isKnown: label => - closedSchemas.length === 0 || closedSchemas.every(schema => schemaAcceptsSectionLabel(schema, label)), + !closed || + (acceptedByAll(closedConjuncts, label) && + closedUnions.every(variants => variants.some(conjuncts => acceptedByAll(conjuncts, label)))), }; } @@ -164,6 +188,43 @@ function collectClosedTopLevelSchemas(jsonSchema: Record): Reco return schemas; } +/** One fully-closed `oneOf`/`anyOf` union: per variant, that variant's closed conjunct schemas. */ +type ClosedUnionVariants = Record[][]; + +/** + * Collect top-level `oneOf`/`anyOf` unions in which EVERY variant is closed — i.e. each variant + * (or one of its `allOf` conjuncts, resolved via `collectClosedTopLevelSchemas`) carries + * `additionalProperties: false`. JTD discriminator output schemas compile to exactly this shape: + * a root `oneOf` of closed object variants. Unions with any open (or non-object) variant are + * skipped entirely so the unknown-label gate cannot fire false rejections. Unions nested under + * `allOf` conjuncts gate identically (intersection semantics). + */ +function collectClosedTopLevelUnions(jsonSchema: Record): ClosedUnionVariants[] { + const unions: ClosedUnionVariants[] = []; + for (const key of ["oneOf", "anyOf"] as const) { + const rawVariants = jsonSchema[key]; + if (!Array.isArray(rawVariants) || rawVariants.length === 0) continue; + const variants: ClosedUnionVariants = []; + let allClosed = true; + for (const raw of rawVariants) { + const conjuncts = isRecord(raw) ? collectClosedTopLevelSchemas(raw) : []; + if (conjuncts.length === 0) { + allClosed = false; + break; + } + variants.push(conjuncts); + } + if (allClosed) unions.push(variants); + } + const allOf = jsonSchema.allOf; + if (Array.isArray(allOf)) { + for (const raw of allOf) { + if (isRecord(raw)) unions.push(...collectClosedTopLevelUnions(raw)); + } + } + return unions; +} + function declaredPropertyLabels(jsonSchema: Record): string[] { const properties = jsonSchema.properties; if (properties === null || typeof properties !== "object" || Array.isArray(properties)) return []; diff --git a/packages/coding-agent/src/tools/yield.ts b/packages/coding-agent/src/tools/yield.ts index a4d5448e9..184645d94 100644 --- a/packages/coding-agent/src/tools/yield.ts +++ b/packages/coding-agent/src/tools/yield.ts @@ -390,8 +390,9 @@ export class YieldTool implements AgentTool { } /** - * Return incremental yield labels that are not declared as top-level properties of a closed - * caller schema. Open schemas (no `additionalProperties: false`) accept any label. + * Return incremental yield labels the closed caller schema does not accept. Closure covers the + * root, `allOf` conjuncts, and `oneOf`/`anyOf` unions whose every variant is closed (e.g. JTD + * discriminators). Open schemas accept any label. */ #unknownIncrementalLabels(labels: string[]): string[] { if (!this.#rejectUnknownSections) return []; diff --git a/packages/coding-agent/test/tools/yield.test.ts b/packages/coding-agent/test/tools/yield.test.ts index 234a26f07..bca9213d2 100644 --- a/packages/coding-agent/test/tools/yield.test.ts +++ b/packages/coding-agent/test/tools/yield.test.ts @@ -410,6 +410,89 @@ describe("YieldTool", () => { ); }); + it("rejects unknown incremental labels for JTD discriminator (oneOf-of-closed) caller schemas", async () => { + // JTD discriminator output schemas compile to a top-level `oneOf` of closed object + // variants (no root `additionalProperties: false`, no `allOf`). Closure MUST be derived + // across the union — otherwise a stale label is accepted here and, once a sibling section + // exhausts MAX_SCHEMA_RETRIES, finalizeSubprocessOutput honors schemaOverridden and the + // stale label lands in a "successful" result (PR #3927 review). + const tool = new YieldTool( + createSession({ + outputSchema: { + discriminator: "verdict", + mapping: { + clean: { + properties: { issue_key: { type: "string" } }, + }, + blockers: { + properties: { issue_key: { type: "string" } }, + optionalProperties: { + blockers: { elements: { properties: { title: { type: "string" } } } }, + }, + }, + }, + }, + }), + ); + + await expect( + tool.execute("call-jtd-discriminator-stale-label", { + type: ["findings"], + result: { data: { title: "native reviewer finding" } }, + } as never), + ).rejects.toThrow( + /Section "findings" uses unknown incremental yield label\(s\): "findings"\. Resubmit with one of the schema's labels: "issue_key", "verdict", "blockers"\./, + ); + + // A label declared by only ONE variant is known: union semantics are disjunctive, the + // assembled output only has to match one variant. + const singleVariantLabel = await tool.execute("call-jtd-discriminator-variant-label", { + type: ["blockers"], + result: { data: { title: "blocker from the blockers variant" } }, + } as never); + expect(singleVariantLabel.details?.data).toEqual({ title: "blocker from the blockers variant" }); + + // The discriminator property itself is declared by every variant. + const discriminatorLabel = await tool.execute("call-jtd-discriminator-tag-label", { + type: ["verdict"], + result: { data: "blockers" }, + } as never); + expect(discriminatorLabel.details?.data).toBe("blockers"); + }); + + it("does not gate incremental labels when a oneOf variant is open", async () => { + // One open variant (`additionalProperties` not false) accepts arbitrary top-level + // properties, so the union places no constraint on labels — engaging the gate would + // reject labels the schema actually allows. + const tool = new YieldTool( + createSession({ + outputSchema: { + oneOf: [ + { + type: "object", + properties: { + issue_key: { type: "string" }, + verdict: { enum: ["clean", "blockers"] }, + }, + required: ["issue_key", "verdict"], + additionalProperties: false, + }, + { + type: "object", + properties: { notes: { type: "string" } }, + }, + ], + }, + }), + ); + + const result = await tool.execute("call-open-variant-label", { + type: ["findings"], + result: { data: { title: "accepted because one variant is open" } }, + } as never); + expect(result.details?.data).toEqual({ title: "accepted because one variant is open" }); + }); + it("detects array-valued labels when the closed caller schema is a root $ref", () => { const labels = arrayValuedLabels({ $ref: "#/$defs/Closed",