Use output validator to retry aggregator on parse failures and GitHub 422s

AGENTS.md

@@ -208,7 +208,7 @@ Examples: `runAnalysis`, `runOnCommentTrigger`, `runOnLocalDiff`.
 
 Pure helper functions contain parsing, validation, formatting, transformation, and decision-making logic. They are directly imported wherever needed and never injected.
 
-Examples: `parseAggregatorOutput`, `buildAgentPrompt`, `formatReviewForConsole`, `isPrFile`.
+Examples: `tryParseAggregatorOutput`, `buildAgentPrompt`, `formatReviewForConsole`, `isPrFile`.
 
 ### Important Rules
 

source/agent-loop.mts

@@ -1,9 +1,12 @@
-import { completions, type CompletionsMessage, type CompletionsToolCall, type CompletionDelta, type CompletionUsage, type CompletionResult, type CompletionsRequest } from './completions.mts'
+import { completions, type CompletionDelta, type CompletionResult, type CompletionsMessage, type CompletionsRequest, type CompletionsToolCall, type CompletionUsage } from './completions.mts'
 import type { ProviderProfile } from './provider-profiles.mts'
 import { isArrayOf, isRecord, isString } from './typescript-helpers.mts'
 
 export type Fetch = (signal: AbortSignal, body: string, headers?: Record<string, string>) => Promise<Response>
 
+// Return a string to feed it back to the model as a user turn and continue the loop; return null to terminate.
+export type OutputValidator = (content: string) => Promise<string | null>
+
 export interface Tool {
 	readonly name: string
 	readonly description?: string
@@ -83,7 +86,7 @@ function toWireTools(tools: readonly Tool[]): CompletionsRequest['tools'] {
 	})
 }
 
-export async function* agentLoop(dependencies: { fetch: Fetch },  model: string, messages: readonly CompletionsMessage[], tools: readonly Tool[], profile: ProviderProfile, signal?: AbortSignal, idleTimeoutMilliseconds: number = 300_000): AsyncGenerator<AgentLoopEvent, AgentLoopResult> {
+export async function* agentLoop(dependencies: { fetch: Fetch },  model: string, messages: readonly CompletionsMessage[], tools: readonly Tool[], profile: ProviderProfile, signal?: AbortSignal, outputValidator?: OutputValidator, idleTimeoutMilliseconds: number = 300_000): AsyncGenerator<AgentLoopEvent, AgentLoopResult> {
 	const toolMap = new Map(tools.map(tool => [tool.name, tool]))
 	const wireTools = toWireTools(tools)
 	const mutableMessages: CompletionsMessage[] = [...messages]
@@ -180,6 +183,13 @@ export async function* agentLoop(dependencies: { fetch: Fetch },  model: string,
 			continue
 		}
 
+		const feedback = await outputValidator?.(message.content)
+		if (feedback) {
+			emptyTurnCount = 0
+			mutableMessages.push({ role: 'user', content: feedback })
+			continue
+		}
+
 		return {
 			finishReason: completionResult.finishReason,
 			usage: completionResult.usage,

source/ai.mts

@@ -1,19 +1,22 @@
+import { agentLoop, type AgentLoopResult, type Fetch, type OutputValidator } from "./agent-loop.mts"
 import { buildAgentPrompt, type Agent } from "./agents.mts"
-import { agentLoop, type Fetch, type AgentLoopResult } from "./agent-loop.mts"
-import type { CompletionsMessage } from "./completions.mts"
-import type { ProviderProfile } from "./provider-profiles.mts"
 import type { BaseCommitContext } from "./base-commit.mts"
-import type { SpawnGit } from "./diff.mts"
+import type { CompletionsMessage } from "./completions.mts"
+import type { TryResult } from "./typescript-helpers.mts"
 import type { DebugWriter } from "./debug.mts"
+import type { SpawnGit } from "./diff.mts"
 import type { LineComment } from "./github-types.mts"
 import { SIDES } from "./github-types.mts"
 import type { Logger } from "./logger.mts"
+import type { ProviderProfile } from "./provider-profiles.mts"
 import { readReasoningFromDelta } from "./reasoning.mts"
 import type { AiReviewResult } from "./review.mts"
 import { createTools } from "./tool-executor.mts"
 import { createTraceWriter } from "./trace-writer.mts"
 import { includes, isReadonlyArray, sleepWithSignal } from "./typescript-helpers.mts"
 
+export type AggregatorSubmitResult = { kind: "ok" } | { kind: "retry"; feedback: string } | { kind: "fatal"; message: string }
+
 export interface AiConfiguration {
 	apiUrl: string
 	model: string
@@ -72,7 +75,7 @@ export function createFetch(configuration: AiConfiguration): Fetch {
 	}
 }
 
-async function runAgent(dependencies: { fetch: Fetch; spawnGit: SpawnGit; logger: Logger; debugWriter: DebugWriter }, agent: Agent, baseCommit: string, baseCommitContext: BaseCommitContext, diffText: string, model: string, profile: ProviderProfile, agentInputs?: Map<string, string>): Promise<string> {
+async function runAgent(dependencies: { fetch: Fetch; spawnGit: SpawnGit; logger: Logger; debugWriter: DebugWriter }, agent: Agent, baseCommit: string, baseCommitContext: BaseCommitContext, diffText: string, model: string, profile: ProviderProfile, agentInputs?: Map<string, string>, outputValidator?: OutputValidator): Promise<string> {
 	dependencies.logger.log(`Building prompt for ${agent.name}`)
 	const promptMessages = buildAgentPrompt(agent, baseCommitContext, diffText, agentInputs)
 	const debugText = promptMessages.map(m => `--- ${m.role} ---\n${m.content}`).join("\n\n")
@@ -81,9 +84,9 @@ async function runAgent(dependencies: { fetch: Fetch; spawnGit: SpawnGit; logger
 
 	const tools = createTools(dependencies, baseCommit)
 	const messages: CompletionsMessage[] = [...promptMessages]
-	const writer = createTraceWriter(dependencies.debugWriter, agent.name)
+	const traceWriter = createTraceWriter(dependencies.debugWriter, agent.name)
 
-	const generator = agentLoop(dependencies, model, messages, tools, profile, undefined)
+	const generator = agentLoop(dependencies, model, messages, tools, profile, undefined, outputValidator)
 
 	let result: AgentLoopResult
 	while (true) {
@@ -96,16 +99,16 @@ async function runAgent(dependencies: { fetch: Fetch; spawnGit: SpawnGit; logger
 		const event = iteratorResult.value
 		switch (event.type) {
 			case "delta":
-				await writer.delta(readReasoningFromDelta(event.delta, profile), event.delta.content ?? undefined)
+				await traceWriter.delta(readReasoningFromDelta(event.delta, profile), event.delta.content ?? undefined)
 				break
 			case "tool_call":
-				await writer.toolCall(event.toolCall.function.name, event.toolCall.function.arguments)
+				await traceWriter.toolCall(event.toolCall.function.name, event.toolCall.function.arguments)
 				break
 			case "tool_result":
-				await writer.toolResult(event.name, event.result)
+				await traceWriter.toolResult(event.name, event.result)
 				break
 			case "completion":
-				await writer.completion(event.finishReason)
+				await traceWriter.completion(event.finishReason)
 				break
 		}
 	}
@@ -143,26 +146,39 @@ function isValidAiReviewResult(data: unknown): data is AiReviewResult {
 	return true
 }
 
-function parseAggregatorOutput(output: string): AiReviewResult {
-	const stripped = output.replace(/^```(?:json)?\s*\n?/i, "").replace(/\n?```\s*$/, "")
-	let parsed: unknown
+function tryParseAggregatorOutput(output: string): TryResult<AiReviewResult> {
+	const stripped = output.trim().replace(/^```(?:json)?\s*\n?/i, "").replace(/\n?```\s*$/, "")
+	let parsed
 	try {
 		parsed = JSON.parse(stripped)
 	} catch (error) {
-		const originalMessage = error instanceof Error ? error.message : String(error)
-		throw new Error(`Failed to parse aggregator output as JSON: ${originalMessage}\nAggregator output:\n${output}`, { cause: error })
+		return { ok: false, reason: error instanceof Error ? error.message : typeof error === 'string' ? error : JSON.stringify(error) }
+	}
+	if (!isValidAiReviewResult(parsed)) return { ok: false, reason: `Parsed output does not match expected shape:\n${output}` }
+	return { ok: true, value: parsed }
+}
+
+async function aggregatorOutputValidator(submit: ((result: AiReviewResult) => Promise<AggregatorSubmitResult>) | undefined, content: string): Promise<string | null> {
+	const parseResult = tryParseAggregatorOutput(content)
+	if (!parseResult.ok) {
+		return `Your previous output failed JSON parsing and validation:\n${parseResult.reason}`
 	}
-	if (!isValidAiReviewResult(parsed)) throw new Error(`Parsed output does not match expected AiReviewResult shape: ${output}`)
-	return parsed
+	if (submit === undefined) return null
+	const outcome = await submit(parseResult.value)
+	if (outcome.kind === "ok") return null
+	if (outcome.kind === "retry") return outcome.feedback
+	throw new Error(outcome.message)
 }
 
-export async function analyze(dependencies: { fetch: Fetch; spawnGit: SpawnGit; logger: Logger; debugWriter: DebugWriter }, baseCommitContext: BaseCommitContext, diffText: string, agents: Agent[], aggregator: Agent, baseCommit: string, model: string, profile: ProviderProfile): Promise<AiReviewResult> {
+export async function analyze(dependencies: { fetch: Fetch; spawnGit: SpawnGit; logger: Logger; debugWriter: DebugWriter }, baseCommitContext: BaseCommitContext, diffText: string, agents: Agent[], aggregator: Agent, baseCommit: string, model: string, profile: ProviderProfile, submit?: (result: AiReviewResult) => Promise<AggregatorSubmitResult>): Promise<AiReviewResult> {
 	dependencies.logger.log(`Using agents: ${agents.length > 0 ? agents.map(a => a.name).join(", ") : "Default"}`)
 
 	const agentOutputs = await runAgents(dependencies, baseCommit, baseCommitContext, diffText, agents, model, profile)
-	const finalOutput = await runAgent(dependencies, aggregator, baseCommit, baseCommitContext, diffText, model, profile, agentOutputs)
+	const finalOutput = await runAgent(dependencies, aggregator, baseCommit, baseCommitContext, diffText, model, profile, agentOutputs, content => aggregatorOutputValidator(submit, content))
 
 	dependencies.logger.log("Agent analysis complete")
 
-	return parseAggregatorOutput(finalOutput)
+	const result = tryParseAggregatorOutput(finalOutput)
+	if (!result.ok) throw new Error(result.reason)
+	return result.value
 }

source/configuration.mts

@@ -1,5 +1,6 @@
 import type { AgentNames } from "./agents.mts"
 import type { GitHubConfiguration } from "./github-types.mts"
+import type { TryResult } from "./typescript-helpers.mts"
 import { parseCommaSeparatedList } from "./typescript-helpers.mts"
 
 export interface CommentTriggerConfiguration {
@@ -23,8 +24,6 @@ export interface LocalDiffConfiguration {
 
 export type Configuration = CommentTriggerConfiguration | PullRequestConfiguration | LocalDiffConfiguration
 
-type TryResult<T> = { ok: true; value: T } | { ok: false; reason: string }
-
 function tryParseGitHubConfiguration(environment: Record<string, string | undefined>): TryResult<GitHubConfiguration> {
 	const token = environment.GITHUB_TOKEN
 	const pullRequestNumberString = environment.PR_NUMBER

source/github.mts

@@ -75,7 +75,8 @@ export async function fetchPullRequestDiff(dependencies: { githubFetch: GitHubFe
 	return response.text()
 }
 
-export async function submitReview(dependencies: { githubFetch: GitHubFetch }, configuration: GitHubConfiguration, review: GitHubReviewPayload): Promise<void> {
+export type SubmitReviewResult = { ok: true } | { ok: false, status: number, body: string }
+export async function submitReview(dependencies: { githubFetch: GitHubFetch }, configuration: GitHubConfiguration, review: GitHubReviewPayload): Promise<SubmitReviewResult> {
 	const { apiUrl, token, owner, repositoryName, pullRequestNumber } = configuration
 
 	const response = await dependencies.githubFetch(`${apiUrl}/repos/${owner}/${repositoryName}/pulls/${pullRequestNumber}/reviews`, {
@@ -86,8 +87,9 @@ export async function submitReview(dependencies: { githubFetch: GitHubFetch }, c
 
 	if (!response.ok) {
 		const body = await response.text().catch(() => "")
-		throw new Error(`Failed to submit review: ${response.status} ${response.statusText}${body ? `\n${body}` : ""}`)
+		return { ok: false, status: response.status, body }
 	}
+	return { ok: true }
 }
 
 function isValidPrMetadata(data: unknown): data is { base: { sha: string } } {

source/orchestrator.mts

@@ -1,20 +1,21 @@
+import type { Fetch } from "./agent-loop.mts"
 import type { AgentNames, AgentReader } from "./agents.mts"
 import { loadAgents, loadAggregator } from "./agents.mts"
 import { analyze } from "./ai.mts"
-import type { Fetch } from "./agent-loop.mts"
-import type { ProviderProfile } from "./provider-profiles.mts"
 import { getBaseCommitContext } from "./base-commit.mts"
 import type { CommentTriggerConfiguration, LocalDiffConfiguration, PullRequestConfiguration } from "./configuration.mts"
-import { ensureCommitAvailable, generateLocalDiff, validateGitEnvironment } from "./diff.mts"
-import type { SpawnGit } from "./diff.mts"
 import type { DebugWriter } from "./debug.mts"
-import { fetchPullRequestDiff, fetchPullRequestBaseCommit, submitReview } from "./github.mts"
-import type { GitHubFetch } from "./github.mts"
+import type { SpawnGit } from "./diff.mts"
+import { ensureCommitAvailable, generateLocalDiff, validateGitEnvironment } from "./diff.mts"
 import type { GitHubConfiguration } from "./github-types.mts"
+import type { GitHubFetch } from "./github.mts"
+import { fetchPullRequestBaseCommit, fetchPullRequestDiff, submitReview } from "./github.mts"
 import type { Logger } from "./logger.mts"
+import type { ProviderProfile } from "./provider-profiles.mts"
 import type { AiReviewResult } from "./review.mts"
 import { buildReviewPayload, formatReviewForConsole } from "./review.mts"
 import { getAgentsFromComment } from "./trigger.mts"
+import type { AggregatorSubmitResult } from "./ai.mts"
 
 type OrchestratorDependencies = {
 	spawnGit: SpawnGit
@@ -25,12 +26,12 @@ type OrchestratorDependencies = {
 	debugWriter: DebugWriter
 }
 
-async function runAnalysis(dependencies: OrchestratorDependencies, agentNames: AgentNames, userAgentsDirectory: string, builtinAgentsDirectory: string, diffText: string, baseCommit: string, model: string, profile: ProviderProfile): Promise<AiReviewResult> {
+async function runAnalysis(dependencies: OrchestratorDependencies, agentNames: AgentNames, userAgentsDirectory: string, builtinAgentsDirectory: string, diffText: string, baseCommit: string, model: string, profile: ProviderProfile, submit?: (result: AiReviewResult) => Promise<AggregatorSubmitResult>): Promise<AiReviewResult> {
 	const agents = await loadAgents(dependencies, userAgentsDirectory, builtinAgentsDirectory, agentNames)
 	const aggregator = await loadAggregator(dependencies, userAgentsDirectory, builtinAgentsDirectory)
 	await ensureCommitAvailable(dependencies, baseCommit)
 	const baseCommitContext = await getBaseCommitContext(dependencies, baseCommit)
-	return analyze(dependencies, baseCommitContext, diffText, agents, aggregator, baseCommit, model, profile)
+	return analyze(dependencies, baseCommitContext, diffText, agents, aggregator, baseCommit, model, profile, submit)
 }
 
 async function submitPrReview(dependencies: OrchestratorDependencies, agentNames: AgentNames, userAgentsDirectory: string, builtinAgentsDirectory: string, githubConfiguration: GitHubConfiguration, model: string, profile: ProviderProfile): Promise<void> {
@@ -44,9 +45,16 @@ async function submitPrReview(dependencies: OrchestratorDependencies, agentNames
 		return
 	}
 
-	const aiResult = await runAnalysis(dependencies, agentNames, userAgentsDirectory, builtinAgentsDirectory, diffText, baseCommit, model, profile)
-	const reviewPayload = buildReviewPayload(aiResult)
-	await submitReview(dependencies, githubConfiguration, reviewPayload)
+	const submit = async (result: AiReviewResult): Promise<AggregatorSubmitResult> => {
+		const outcome = await submitReview(dependencies, githubConfiguration, buildReviewPayload(result))
+		if (outcome.ok) return { kind: "ok" }
+		if (outcome.status === 422) {
+			dependencies.logger.log(`GitHub rejected aggregator output with ${outcome.status}, sending feedback to aggregator`)
+			return { kind: "retry", feedback: `Your previous output failed on submission to GitHub:\nHTTP Status: ${outcome.status}\n${outcome.body}` }
+		}
+		return { kind: "fatal", message: `Failed to submit review: ${outcome.status} ${outcome.body}` }
+	}
+	await runAnalysis(dependencies, agentNames, userAgentsDirectory, builtinAgentsDirectory, diffText, baseCommit, model, profile, submit)
 	dependencies.logger.log("PR review submitted successfully")
 }
 

source/typescript-helpers.mts

@@ -29,6 +29,8 @@ export function isRecord(value: unknown): value is Record<string, unknown> {
 	return true
 }
 
+export type TryResult<T> = { ok: true; value: T } | { ok: false; reason: string }
+
 export type Guard<T> = (value: unknown) => value is T
 export interface OptionalMarker<T> {
 	__optional: true