From c9b3c23600d7b592c69dc2e2a90fd823c8f3a5ce Mon Sep 17 00:00:00 2001 From: can1357 Date: Mon, 8 Jun 2026 02:04:59 +0200 Subject: [PATCH] feat(task): enabled schema override flow to keep task payloads with warnings - Propagated `schemaOverridden` from `YieldTool` into executor `YieldItem` metadata. - Bypassed schema validation on override or schema-builder errors and kept payload output with success exit. - Emitted `SUBAGENT_WARNING_SCHEMA_OVERRIDDEN` so accepted override results no longer surface as `schema_violation`. --- packages/coding-agent/src/task/executor.ts | 56 ++++++++++------- packages/coding-agent/src/tools/yield.ts | 11 +++- .../test/task/executor-warnings.test.ts | 60 +++++++++++++++++++ .../coding-agent/test/tools/yield.test.ts | 7 ++- 4 files changed, 110 insertions(+), 24 deletions(-) diff --git a/packages/coding-agent/src/task/executor.ts b/packages/coding-agent/src/task/executor.ts index f6450f2f6..4b71c3da2 100644 --- a/packages/coding-agent/src/task/executor.ts +++ b/packages/coding-agent/src/task/executor.ts @@ -311,6 +311,15 @@ export interface YieldItem { data?: unknown; status?: "success" | "aborted"; error?: string; + /** + * Set by the in-tool yield validator when it exhausted its retry budget + * (MAX_SCHEMA_RETRIES) and accepted a schema-invalid payload anyway. + * `finalizeSubprocessOutput` honors this by serializing the payload and + * surfacing a stderr warning, instead of re-emitting `schema_violation` + * — which would silently swap the subagent's "accepted" view for a + * different, opaque error blob in the parent's view of the result. + */ + schemaOverridden?: boolean; } interface FinalizeSubprocessOutputArgs { @@ -331,7 +340,8 @@ interface FinalizeSubprocessOutputResult { abortedViaYield: boolean; hasYield: boolean; } - +export const SUBAGENT_WARNING_SCHEMA_OVERRIDDEN = + "SYSTEM WARNING: Subagent exhausted schema-retry budget; result was accepted despite failing the output schema."; export const SUBAGENT_WARNING_NULL_YIELD = "SYSTEM WARNING: Subagent called yield with null data."; export const SUBAGENT_WARNING_MISSING_YIELD = "SYSTEM WARNING: Subagent exited without calling yield tool after 3 reminders."; @@ -384,29 +394,31 @@ export function finalizeSubprocessOutput(args: FinalizeSubprocessOutputArgs): Fi rawOutput = rawOutput ? `${SUBAGENT_WARNING_NULL_YIELD}\n\n${rawOutput}` : SUBAGENT_WARNING_NULL_YIELD; } else { const { validator, error: schemaError } = buildOutputValidator(outputSchema); - if (schemaError) { - rawOutput = `{"error":"schema_violation","message":"invalid output schema: ${schemaError.replace(/"/g, '\\"')}"}`; - stderr = `schema_violation: invalid output schema: ${schemaError}`; - exitCode = 1; + const overridden = lastYield?.schemaOverridden === true; + const completeData = normalizeCompleteData(submitData, reportFindings, validator); + const result = + schemaError || overridden + ? { success: true as const } + : (validator?.validate(completeData) ?? { success: true as const }); + if (!result.success) { + const summary = summarizeValidationFailure(result, completeData, validator?.requiredFields ?? []); + const outcome = buildSchemaViolationOutcome(summary, completeData); + rawOutput = outcome.rawOutput; + stderr = outcome.stderr; + exitCode = outcome.exitCode; } else { - const completeData = normalizeCompleteData(submitData, reportFindings, validator); - const result = validator?.validate(completeData) ?? { success: true as const }; - if (!result.success) { - const summary = summarizeValidationFailure(result, completeData, validator?.requiredFields ?? []); - const outcome = buildSchemaViolationOutcome(summary, completeData); - rawOutput = outcome.rawOutput; - stderr = outcome.stderr; - exitCode = outcome.exitCode; - } else { - try { - rawOutput = JSON.stringify(completeData, null, 2) ?? "null"; - } catch (err) { - const errorMessage = err instanceof Error ? err.message : String(err); - rawOutput = `{"error":"Failed to serialize yield data: ${errorMessage}"}`; - } - exitCode = 0; - stderr = ""; + try { + rawOutput = JSON.stringify(completeData, null, 2) ?? "null"; + } catch (err) { + const errorMessage = err instanceof Error ? err.message : String(err); + rawOutput = `{"error":"Failed to serialize yield data: ${errorMessage}"}`; } + exitCode = 0; + stderr = overridden + ? SUBAGENT_WARNING_SCHEMA_OVERRIDDEN + : schemaError + ? `invalid output schema: ${schemaError}` + : ""; } } } diff --git a/packages/coding-agent/src/tools/yield.ts b/packages/coding-agent/src/tools/yield.ts index 10422dc8f..d4969611b 100644 --- a/packages/coding-agent/src/tools/yield.ts +++ b/packages/coding-agent/src/tools/yield.ts @@ -20,6 +20,14 @@ export interface YieldDetails { data: unknown; status: "success" | "aborted"; error?: string; + /** + * Set when the yield tool exhausted its in-tool schema-retry budget + * (MAX_SCHEMA_RETRIES) and accepted the data anyway. Surfaced so the + * executor's post-mortem finalizer can honor the override instead of + * re-rejecting the same payload with `schema_violation` — keeping the + * subagent's acceptance and the parent's view of the result in lockstep. + */ + schemaOverridden?: boolean; } function formatSchema(schema: unknown): string { @@ -237,7 +245,7 @@ export class YieldTool implements AgentTool { : "Result submitted."; return { content: [{ type: "text", text: responseText }], - details: { data, status, error: errorMessage }, + details: { data, status, error: errorMessage, schemaOverridden: schemaValidationOverridden || undefined }, }; } } @@ -254,6 +262,7 @@ subprocessToolRegistry.register("yield", { data: record.data, status, error: typeof record.error === "string" ? record.error : undefined, + schemaOverridden: record.schemaOverridden === true ? true : undefined, }; }, shouldTerminate: event => !event.isError, diff --git a/packages/coding-agent/test/task/executor-warnings.test.ts b/packages/coding-agent/test/task/executor-warnings.test.ts index 385151b69..dd14924b2 100644 --- a/packages/coding-agent/test/task/executor-warnings.test.ts +++ b/packages/coding-agent/test/task/executor-warnings.test.ts @@ -3,6 +3,7 @@ import { finalizeSubprocessOutput, SUBAGENT_WARNING_MISSING_YIELD, SUBAGENT_WARNING_NULL_YIELD, + SUBAGENT_WARNING_SCHEMA_OVERRIDDEN, } from "../../src/task/executor"; describe("subagent warning injection", () => { @@ -131,4 +132,63 @@ describe("subagent warning injection", () => { expect(result.rawOutput.includes("SYSTEM WARNING")).toBe(false); expect(result.exitCode).toBe(0); }); + + it("honors schemaOverridden flag from yield and surfaces data with warning", () => { + // Reviewer subagent exhausted its in-tool schema-retry budget, then was + // accepted with empty finding objects. Without honoring the override, the + // executor's post-mortem validator silently rejected the same payload with + // `schema_violation`, opaquely swapping the agent's accepted output for an + // error blob. Reports #2, #8, #11, #16, #17, #20. + const result = finalizeSubprocessOutput({ + rawOutput: "", + exitCode: 0, + stderr: "", + doneAborted: false, + signalAborted: false, + yieldItems: [{ status: "success", data: { findings: [{}, {}] }, schemaOverridden: true }], + outputSchema: { + type: "object", + required: ["findings"], + properties: { + findings: { + type: "array", + minItems: 1, + items: { + type: "object", + required: ["severity", "file", "line"], + properties: { + severity: { type: "string" }, + file: { type: "string" }, + line: { type: "number" }, + }, + }, + }, + }, + }, + }); + + expect(result.exitCode).toBe(0); + expect(result.stderr).toBe(SUBAGENT_WARNING_SCHEMA_OVERRIDDEN); + expect(JSON.parse(result.rawOutput)).toEqual({ findings: [{}, {}] }); + }); + + it("treats malformed output schemas as no validation instead of schema_violation", () => { + // Empty-string schema is a caller mistake; the yield tool already degrades + // to a loose schema and accepts the data. The executor's finalizer used to + // emit `schema_violation: invalid output schema` even though yield accepted + // it, which surprised users dispatching prose review batches. Report #60. + const result = finalizeSubprocessOutput({ + rawOutput: "", + exitCode: 0, + stderr: "", + doneAborted: false, + signalAborted: false, + yieldItems: [{ status: "success", data: { verdict: "looks good" } }], + outputSchema: "", + }); + + expect(result.exitCode).toBe(0); + expect(JSON.parse(result.rawOutput)).toEqual({ verdict: "looks good" }); + expect(result.stderr.startsWith("invalid output schema:")).toBe(true); + }); }); diff --git a/packages/coding-agent/test/tools/yield.test.ts b/packages/coding-agent/test/tools/yield.test.ts index c2664c71e..766593286 100644 --- a/packages/coding-agent/test/tools/yield.test.ts +++ b/packages/coding-agent/test/tools/yield.test.ts @@ -387,7 +387,12 @@ describe("YieldTool", () => { const overrideResult = await tool.execute("call-short-override", { result: { data: { token: "ab" } }, } as never); - expect(overrideResult.details).toEqual({ data: { token: "ab" }, status: "success", error: undefined }); + expect(overrideResult.details).toEqual({ + data: { token: "ab" }, + status: "success", + error: undefined, + schemaOverridden: true, + }); expect(overrideResult.content).toEqual([ { type: "text",