diff --git a/packages/code/CHANGELOG.md b/packages/code/CHANGELOG.md index 3adc1a4..731a898 100644 --- a/packages/code/CHANGELOG.md +++ b/packages/code/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- **Structured agent responses**: feasibility checks now accept the valid bare JSON commonly returned by Codex instead of requiring a Markdown code fence. Feasibility, estimation, and auto-review share brace-aware extraction that also tolerates narration and ignores unrelated braces in prose + ## [2.3.2] - 2026-08-18 ### Fixed diff --git a/packages/code/src/index.ts b/packages/code/src/index.ts index d24018e..0d68f92 100755 --- a/packages/code/src/index.ts +++ b/packages/code/src/index.ts @@ -42,6 +42,7 @@ import { buildSandboxDoctorReport, getSandbox, setSandboxOverride } from "./lib/ import { isMarkdownFilePath } from "@devintern/task-trackers"; import { findEnvFile, maybeOfferCliUpdate, resolveConfigDir } from "@devintern/utils"; import { ReadonlyAnalysisError, runAnalysisWithFallback } from "./lib/analysis-mode"; +import { parseAgentJsonObject } from "./lib/agent-json"; import { TaskFormatter } from "./lib/task-formatter"; import type { RetryPromptContext } from "./lib/task-formatter"; import { resolveOutputDir } from "./lib/output-dir"; @@ -2735,21 +2736,15 @@ async function runClarityCheck( * @throws When JSON is missing or required fields are invalid */ function parseClarityResponse(output: string): ClarityAssessment { - // Extract JSON from Agent's response - const jsonMatch = output.match(/```json\s*([\s\S]*?)\s*```/); - if (!jsonMatch) { - // Provide more specific error based on output content - if (detectMaxTurnsReached(output, "")) { - throw new Error("warn: Agent reached max turns - no JSON assessment available"); - } - if (output.trim().length === 0) { - throw new Error("warn: Empty response from Agent"); - } - throw new Error("warn: No JSON found in Agent response"); + if (detectMaxTurnsReached(output, "")) { + throw new Error("warn: Agent reached max turns - no JSON assessment available"); + } + if (output.trim().length === 0) { + throw new Error("warn: Empty response from Agent"); } try { - const assessment = JSON.parse(jsonMatch[1]); + const assessment = parseAgentJsonObject(output, "isImplementable"); // Validate required fields if ( @@ -2762,7 +2757,7 @@ function parseClarityResponse(output: string): ClarityAssessment { throw new Error("warn: Invalid assessment structure - missing required fields"); } - return assessment; + return assessment as unknown as ClarityAssessment; } catch (error) { if (error instanceof SyntaxError) { throw new Error(`warn: Malformed JSON in Agent response: ${error.message}`); @@ -3053,52 +3048,20 @@ async function runEstimation( * @throws When JSON is invalid or values are out of range */ function parseEstimationResponse(output: string): EstimationResult { - // Try to find JSON in the response — with or without code fences - let jsonStr: string | null = null; - - const fencedMatch = output.match(/```(?:json)?\s*([\s\S]*?)\s*```/); - if (fencedMatch) { - jsonStr = fencedMatch[1]; - } else { - // Try to extract a raw JSON object containing "storyPoints". - // We find the last "storyPoints" in the output, then try { positions - // before it (nearest first) paired with the last } after it, letting - // JSON.parse decide validity. This avoids greedy-regex issues when - // the surrounding text contains stray braces (e.g. URL templates). - const spIdx = output.lastIndexOf('"storyPoints"'); - if (spIdx !== -1) { - const endIdx = output.lastIndexOf("}"); - if (endIdx > spIdx) { - for (let i = output.lastIndexOf("{", spIdx); i >= 0; i = output.lastIndexOf("{", i - 1)) { - const candidate = output.substring(i, endIdx + 1); - try { - JSON.parse(candidate); - jsonStr = candidate; - break; - } catch { - continue; - } - } - } - } - } - - if (!jsonStr) { - throw new Error("No JSON found in estimation response"); - } - - const parsed = JSON.parse(jsonStr); + const parsed = parseAgentJsonObject(output, "storyPoints"); // Validate required fields const validPoints = [1, 2, 3, 5, 8, 13, 21]; - if (!validPoints.includes(parsed.storyPoints)) { + const storyPoints = parsed.storyPoints; + if (typeof storyPoints !== "number" || !validPoints.includes(storyPoints)) { throw new Error( - `Invalid story points value: ${parsed.storyPoints}. Must be one of: ${validPoints.join(", ")}`, + `Invalid story points value: ${storyPoints}. Must be one of: ${validPoints.join(", ")}`, ); } - if (!["high", "medium", "low"].includes(parsed.confidence)) { - throw new Error(`Invalid confidence level: ${parsed.confidence}. Must be high, medium, or low`); + const confidence = parsed.confidence; + if (confidence !== "high" && confidence !== "medium" && confidence !== "low") { + throw new Error(`Invalid confidence level: ${confidence}. Must be high, medium, or low`); } // Clamp implementationConfidence to 0-10, default to 5 if missing @@ -3107,13 +3070,13 @@ function parseEstimationResponse(output: string): EstimationResult { implConf = Math.max(0, Math.min(10, Math.round(implConf))); return { - storyPoints: parsed.storyPoints, - confidence: parsed.confidence, + storyPoints, + confidence, implementationConfidence: implConf, - reasoning: parsed.reasoning || "", + reasoning: typeof parsed.reasoning === "string" ? parsed.reasoning : "", risks: Array.isArray(parsed.risks) ? parsed.risks : [], unclearAreas: Array.isArray(parsed.unclearAreas) ? parsed.unclearAreas : [], - summary: parsed.summary || "", + summary: typeof parsed.summary === "string" ? parsed.summary : "", }; } diff --git a/packages/code/src/lib/agent-json.ts b/packages/code/src/lib/agent-json.ts new file mode 100644 index 0000000..1818ec1 --- /dev/null +++ b/packages/code/src/lib/agent-json.ts @@ -0,0 +1,98 @@ +/** + * Extract a structured JSON object from an agent's final response. + * + * Agent CLIs do not consistently preserve formatting instructions: the same + * prompt may produce fenced JSON, bare JSON, or JSON surrounded by narration. + * This parser accepts all three shapes and ignores unrelated braces in prose. + */ + +function balancedObjectCandidates(text: string): string[] { + const candidates: string[] = []; + let start = -1; + let depth = 0; + let inString = false; + let escaped = false; + + for (let index = 0; index < text.length; index += 1) { + const character = text[index]; + + if (start === -1) { + if (character === "{") { + start = index; + depth = 1; + } + continue; + } + + if (inString) { + if (escaped) { + escaped = false; + } else if (character === "\\") { + escaped = true; + } else if (character === '"') { + inString = false; + } + continue; + } + + if (character === '"') { + inString = true; + } else if (character === "{") { + depth += 1; + } else if (character === "}") { + depth -= 1; + if (depth === 0) { + candidates.push(text.slice(start, index + 1)); + start = -1; + } + } + } + + return candidates; +} + +/** + * Parse the expected JSON object from raw agent stdout. + * + * Fenced objects take precedence. For bare objects, the last matching object + * wins because agents commonly narrate before emitting their final answer. + * + * @param output - Raw agent stdout + * @param requiredKey - Key that identifies the expected response object + * @returns Parsed JSON object containing `requiredKey` + * @throws When no valid matching object is present + */ +export function parseAgentJsonObject(output: string, requiredKey: string): Record { + const candidates: string[] = []; + const fencedPattern = /```(?:json)?\s*([\s\S]*?)\s*```/gi; + + for (const match of output.matchAll(fencedPattern)) { + if (match[1]) { + candidates.push(match[1]); + } + } + + candidates.push(...balancedObjectCandidates(output).reverse()); + + let lastParseError: unknown; + for (const candidate of candidates) { + try { + const parsed = JSON.parse(candidate) as unknown; + if ( + typeof parsed === "object" && + parsed !== null && + !Array.isArray(parsed) && + Object.hasOwn(parsed, requiredKey) + ) { + return parsed as Record; + } + } catch (error) { + lastParseError = error; + } + } + + if (lastParseError instanceof SyntaxError && candidates.length === 1) { + throw lastParseError; + } + throw new Error(`No valid JSON object containing "${requiredKey}" found in Agent response`); +} diff --git a/packages/code/src/lib/auto-review-loop.ts b/packages/code/src/lib/auto-review-loop.ts index b9cb3f8..c0149e0 100644 --- a/packages/code/src/lib/auto-review-loop.ts +++ b/packages/code/src/lib/auto-review-loop.ts @@ -13,6 +13,7 @@ import { writeFileSync, readFileSync, existsSync, mkdirSync } from "fs"; import { join } from "path"; import { spawnAgent, reapTree, resolveExecutablePathWithRetry } from "@devintern/agent-harness"; import type { AgentHarness } from "@devintern/agent-harness"; +import { parseAgentJsonObject } from "./agent-json"; import { buildHeadlessAgentArgs, HEADLESS_AGENT_STDIO } from "./agent-spawn"; import { getSandbox } from "./sandbox"; import type { @@ -77,9 +78,8 @@ ${prDiff} - **low**: Style inconsistencies, minor optimizations - **info**: Suggestions, alternatives, educational feedback -4. Provide your feedback as JSON in the following format: +4. Provide your feedback as one JSON object in the following format: -\`\`\`json { "summary": "Brief overall assessment of the PR (2-3 sentences)", "items": [ @@ -87,20 +87,19 @@ ${prDiff} "priority": "critical|high|medium|low|info", "category": "code-quality|bug|performance|security|testing|documentation|style", "file": "path/to/file.ts", - "line": "42" or "42-45", + "line": "42-45", "issue": "Clear description of the issue", "suggestion": "Specific actionable fix or improvement" } ], "approved": false } -\`\`\` 5. Set "approved": true ONLY if all issues are low priority or informational 6. Be constructive and specific in your feedback 7. Focus on actionable improvements -**IMPORTANT**: Your response must be valid JSON only. Do not include any explanatory text outside the JSON block. +**IMPORTANT**: Return only the valid JSON object. Do not include Markdown fences or explanatory text. `; } @@ -281,18 +280,8 @@ function getPRDiff(baseBranch: string, workingDir: string): string { * @throws When no JSON is found or the structure is invalid */ function parseReviewFeedback(agentOutput: string): ReviewFeedback { - // Extract JSON from potential markdown code blocks - const jsonMatch = - agentOutput.match(/```json\s*([\s\S]*?)\s*```/) || agentOutput.match(/\{[\s\S]*\}/); - - if (!jsonMatch) { - throw new Error("No JSON found in Agent output"); - } - - const jsonStr = jsonMatch[1] || jsonMatch[0]; - try { - const feedback = JSON.parse(jsonStr) as ReviewFeedback; + const feedback = parseAgentJsonObject(agentOutput, "approved"); // Validate structure if ( @@ -303,7 +292,7 @@ function parseReviewFeedback(agentOutput: string): ReviewFeedback { throw new Error("Invalid feedback structure"); } - return feedback; + return feedback as unknown as ReviewFeedback; } catch (error) { throw new Error(`Failed to parse review feedback JSON: ${error}`); } diff --git a/packages/code/src/lib/task-formatter.ts b/packages/code/src/lib/task-formatter.ts index 7c3b696..18eaeb6 100644 --- a/packages/code/src/lib/task-formatter.ts +++ b/packages/code/src/lib/task-formatter.ts @@ -611,25 +611,20 @@ You are a senior software engineer reviewing a task before implementation. Your prompt += `## Assessment Instructions -Please assess this task for basic implementation feasibility. Respond with a JSON object containing: +Please assess this task for basic implementation feasibility. Return only one valid JSON object with this shape (no Markdown fences or explanatory text): -\`\`\`json { - "isImplementable": boolean, - "clarityScore": number, // 1-10 scale (10 = perfectly clear) - "issues": [ - { - "category": "missing_requirements" | "unclear_scope" | "missing_context" | "ambiguous_description" | "critical_gaps", - "description": "Specific issue description", - "severity": "critical" | "major" | "minor" - } - ], - "recommendations": [ - "Specific recommendation for improving clarity" - ], + "isImplementable": true, + "clarityScore": 8, + "issues": [], + "recommendations": [], "summary": "Brief summary of the assessment" } -\`\`\` + +Each issue must contain: +- "category": one of "missing_requirements", "unclear_scope", "missing_context", "ambiguous_description", or "critical_gaps" +- "description": a specific issue description +- "severity": one of "critical", "major", or "minor" ## Evaluation Criteria (Relaxed for Real-World Development) diff --git a/packages/code/tests/agent-json.test.ts b/packages/code/tests/agent-json.test.ts new file mode 100644 index 0000000..d55e5e0 --- /dev/null +++ b/packages/code/tests/agent-json.test.ts @@ -0,0 +1,54 @@ +import { describe, expect, test } from "bun:test"; +import { parseAgentJsonObject } from "../src/lib/agent-json"; + +describe("parseAgentJsonObject", () => { + test("parses fenced JSON", () => { + expect(parseAgentJsonObject('```json\n{"approved": true}\n```', "approved")).toEqual({ + approved: true, + }); + }); + + test("parses bare JSON returned by Codex", () => { + const output = `{ + "isImplementable": true, + "clarityScore": 8, + "issues": [], + "recommendations": [], + "summary": "The task is implementable." +}`; + + expect(parseAgentJsonObject(output, "isImplementable")).toEqual({ + isImplementable: true, + clarityScore: 8, + issues: [], + recommendations: [], + summary: "The task is implementable.", + }); + }); + + test("parses JSON surrounded by narration and unrelated braces", () => { + const output = `I checked PATCH /items/{id} first. +{"approved":false,"summary":"A string with {braces}","items":[]} +That is the final review.`; + + expect(parseAgentJsonObject(output, "approved")).toEqual({ + approved: false, + summary: "A string with {braces}", + items: [], + }); + }); + + test("prefers the last matching bare object", () => { + const output = '{"storyPoints":1}\nFinal: {"storyPoints":5,"confidence":"high"}'; + expect(parseAgentJsonObject(output, "storyPoints")).toEqual({ + storyPoints: 5, + confidence: "high", + }); + }); + + test("throws when the expected object is absent", () => { + expect(() => parseAgentJsonObject("No structured response.", "approved")).toThrow( + 'No valid JSON object containing "approved"', + ); + }); +});