diff --git a/src/codex.ts b/src/codex.ts index 1f2916cc..a58e9ae4 100644 --- a/src/codex.ts +++ b/src/codex.ts @@ -110,9 +110,6 @@ async function runCodex( config: ResolvedProjectConfig, args: string[], ): Promise { - const isDevMode = - process.env.PIV_DEV_MODE === "1" || - process.env.PIV_PRINT_CODEX_LOGS === "1"; const outputFile = args[args.indexOf("--output-last-message") + 1] ?? ""; const envOverrides = config.codex.codexHome ? { CODEX_HOME: config.codex.codexHome } @@ -120,8 +117,8 @@ async function runCodex( const result = await runCommand(config.codex.binary, args, { cwd: config.executionPath, env: envOverrides, - streamStdout: isDevMode, - streamStderr: isDevMode, + streamStdout: config.codex.streamLogs, + streamStderr: config.codex.streamLogs, stdinMode: "ignore", }); diff --git a/src/comments.ts b/src/comments.ts new file mode 100644 index 00000000..70509337 --- /dev/null +++ b/src/comments.ts @@ -0,0 +1,87 @@ +import type { BugRecord } from "./types"; + +export interface TokenUsage { + inputTokens?: number; + outputTokens?: number; + totalTokens?: number; +} + +export function formatCodexUsageLine(usage?: TokenUsage): string { + if (!usage) { + return "Token usage: unknown"; + } + const input = usage.inputTokens ?? "unknown"; + const output = usage.outputTokens ?? "unknown"; + const total = + usage.totalTokens ?? + (typeof usage.inputTokens === "number" && + typeof usage.outputTokens === "number" + ? usage.inputTokens + usage.outputTokens + : "unknown"); + return `Token usage: input ${input}, output ${output}, total ${total}`; +} + +export function buildPlanComment( + issueKey: string, + planSummary: string, + usage?: TokenUsage, +): string { + const maxSummaryLength = 6000; + const normalized = planSummary.trim(); + const truncated = + normalized.length > maxSummaryLength + ? `${normalized.slice(0, maxSummaryLength)}\n\n[truncated]` + : normalized; + + return [ + `PIV loop plan for ${issueKey}`, + "", + "Planning completed; implementation started.", + "", + formatCodexUsageLine(usage), + "", + "Plan:", + truncated || "(No plan summary returned by planning agent.)", + ].join("\n"); +} + +export function buildImplementationComment( + draftPrUrl: string | undefined, + usage?: TokenUsage, +): string { + return [ + `Implementation completed. Draft PR: ${draftPrUrl ?? "(created)"}`, + formatCodexUsageLine(usage), + ].join("\n"); +} + +export function buildReviewComment(input: { + issueKey: string; + passed: boolean; + summary: string; + usage?: TokenUsage; + bugs: BugRecord[]; +}): string { + return [ + `PIV loop review for ${input.issueKey}`, + "", + `Result: ${input.passed ? "PASS" : "FAIL"}`, + "", + formatCodexUsageLine(input.usage), + "", + input.summary, + "", + input.bugs.length > 0 + ? "Bugs were detected and converted to GitHub issues." + : "No bugs found.", + ].join("\n"); +} + +export function buildBugsCanceledComment(bugs: BugRecord[]): string { + return [ + "Review/testing found bugs. Moved issue to Canceled.", + ...bugs.map( + (bug) => `- ${bug.title}${bug.issueUrl ? ` (${bug.issueUrl})` : ""}`, + ), + ].join("\n"); +} diff --git a/src/config.ts b/src/config.ts index 6993708a..ef492896 100644 --- a/src/config.ts +++ b/src/config.ts @@ -84,6 +84,7 @@ function buildEnvBase(cwd: string): ProjectRuntimeConfig { }, codex: { binary: env.CODEX_BINARY ?? "codex", + streamLogs: env.PIV_DEV_MODE === "1" || env.PIV_PRINT_CODEX_LOGS === "1", model: env.CODEX_MODEL, models: { plan: env.CODEX_MODEL_PLAN, diff --git a/src/review.ts b/src/review.ts new file mode 100644 index 00000000..0da4c142 --- /dev/null +++ b/src/review.ts @@ -0,0 +1,74 @@ +import type { BugRecord } from "./types"; + +export interface ReviewOutcome { + passed: boolean; + summary: string; + bugs: BugRecord[]; +} + +export function parseReviewOutcome(text: string): ReviewOutcome { + const upper = text.toUpperCase(); + const passed = + upper.includes("RESULT: PASS") && !upper.includes("RESULT: FAIL"); + const summary = extractSummary(text); + const bugs = extractBugs(text); + return { + passed: passed && bugs.length === 0, + summary, + bugs, + }; +} + +function extractSummary(text: string): string { + const match = text.match(/SUMMARY:\s*([\s\S]*?)(?:\nBUGS_JSON:|$)/i); + if (match?.[1]) { + return match[1].trim(); + } + return text.trim().slice(0, 1200); +} + +function extractBugs(text: string): BugRecord[] { + const jsonFromLabel = text.match(/BUGS_JSON:\s*([\s\S]*)$/i)?.[1]?.trim(); + if (jsonFromLabel) { + const parsed = parseBugJson(jsonFromLabel); + if (parsed.length > 0) { + return parsed; + } + } + + const fenced = text.match(/```json\s*([\s\S]*?)```/i)?.[1]?.trim(); + if (fenced) { + const parsed = parseBugJson(fenced); + if (parsed.length > 0) { + return parsed; + } + } + + return []; +} + +function parseBugJson(input: string): BugRecord[] { + try { + const parsed = JSON.parse(input) as unknown; + if (Array.isArray(parsed)) { + return parsed + .map((item) => { + if (!item || typeof item !== "object") { + return null; + } + const bug = item as Record; + if (typeof bug.title !== "string" || typeof bug.body !== "string") { + return null; + } + return { + title: bug.title, + body: bug.body, + } satisfies BugRecord; + }) + .filter((item): item is BugRecord => item !== null); + } + } catch { + return []; + } + return []; +} diff --git a/src/types.ts b/src/types.ts index 5bf27157..ea40b258 100644 --- a/src/types.ts +++ b/src/types.ts @@ -65,6 +65,7 @@ export interface ProjectRuntimeConfig { }; codex: { binary: string; + streamLogs: boolean; model?: string; models?: { plan?: string; diff --git a/src/workflow.ts b/src/workflow.ts index 6b7bad32..9abf6876 100644 --- a/src/workflow.ts +++ b/src/workflow.ts @@ -1,4 +1,10 @@ import { runPlanSession, runResumeSession, runReviewSession } from "./codex"; +import { + buildBugsCanceledComment, + buildImplementationComment, + buildPlanComment, + buildReviewComment, +} from "./comments"; import { type LoadedConfig, getProjectById } from "./config"; import { commentOnPr, @@ -12,6 +18,7 @@ import { buildPlanPrompt, buildReviewPrompt, } from "./prompts"; +import { parseReviewOutcome } from "./review"; import { loadRunState, normalizeIssueKey, @@ -19,7 +26,6 @@ import { transitionStage, } from "./state"; import type { - BugRecord, CodexUsageRecord, PollingConfig, ResolvedProjectConfig, @@ -427,10 +433,7 @@ async function executeIssue( await linear.applyStageLabel(state.issue.id, "pr_created"); await linear.comment( state.issue.id, - [ - `Implementation completed. Draft PR: ${state.pullRequest.url ?? "(created)"}`, - formatCodexUsageLine(result.usage), - ].join("\n"), + buildImplementationComment(state.pullRequest.url, result.usage), ); logger.info( buildIssueJobLogFields(state, "implementing"), @@ -466,19 +469,13 @@ async function executeIssue( state.bugs = outcome.bugs; await saveRunState(config.workspacePath, state); - const reviewComment = [ - `PIV loop review for ${state.issue.key}`, - "", - `Result: ${outcome.passed ? "PASS" : "FAIL"}`, - "", - formatCodexUsageLine(review.usage), - "", - outcome.summary, - "", - outcome.bugs.length > 0 - ? "Bugs were detected and converted to GitHub issues." - : "No bugs found.", - ].join("\n"); + const reviewComment = buildReviewComment({ + issueKey: state.issue.key, + passed: outcome.passed, + summary: outcome.summary, + usage: review.usage, + bugs: outcome.bugs, + }); if (!config.dryRun && state.pullRequest) { await commentOnPr(config, state.pullRequest, reviewComment); @@ -501,13 +498,7 @@ async function executeIssue( await linear.markCanceled(state.issue.id); await linear.comment( state.issue.id, - [ - "Review/testing found bugs. Moved issue to Canceled.", - ...state.bugs.map( - (bug) => - `- ${bug.title}${bug.issueUrl ? ` (${bug.issueUrl})` : ""}`, - ), - ].join("\n"), + buildBugsCanceledComment(state.bugs), ); return; } @@ -545,126 +536,6 @@ export function appendCodexUsage( ]; } -export interface ReviewOutcome { - passed: boolean; - summary: string; - bugs: BugRecord[]; -} - -export function buildPlanComment( - issueKey: string, - planSummary: string, - usage?: { - inputTokens?: number; - outputTokens?: number; - totalTokens?: number; - }, -): string { - const maxSummaryLength = 6000; - const normalized = planSummary.trim(); - const truncated = - normalized.length > maxSummaryLength - ? `${normalized.slice(0, maxSummaryLength)}\n\n[truncated]` - : normalized; - - return [ - `PIV loop plan for ${issueKey}`, - "", - "Planning completed; implementation started.", - "", - formatCodexUsageLine(usage), - "", - "Plan:", - truncated || "(No plan summary returned by planning agent.)", - ].join("\n"); -} - -export function formatCodexUsageLine(usage?: { - inputTokens?: number; - outputTokens?: number; - totalTokens?: number; -}): string { - if (!usage) { - return "Token usage: unknown"; - } - const input = usage.inputTokens ?? "unknown"; - const output = usage.outputTokens ?? "unknown"; - const total = - usage.totalTokens ?? - (typeof usage.inputTokens === "number" && - typeof usage.outputTokens === "number" - ? usage.inputTokens + usage.outputTokens - : "unknown"); - return `Token usage: input ${input}, output ${output}, total ${total}`; -} - -export function parseReviewOutcome(text: string): ReviewOutcome { - const upper = text.toUpperCase(); - const passed = - upper.includes("RESULT: PASS") && !upper.includes("RESULT: FAIL"); - const summary = extractSummary(text); - const bugs = extractBugs(text); - return { - passed: passed && bugs.length === 0, - summary, - bugs, - }; -} - -function extractSummary(text: string): string { - const match = text.match(/SUMMARY:\s*([\s\S]*?)(?:\nBUGS_JSON:|$)/i); - if (match?.[1]) { - return match[1].trim(); - } - return text.trim().slice(0, 1200); -} - -function extractBugs(text: string): BugRecord[] { - const jsonFromLabel = text.match(/BUGS_JSON:\s*([\s\S]*)$/i)?.[1]?.trim(); - if (jsonFromLabel) { - const parsed = parseBugJson(jsonFromLabel); - if (parsed.length > 0) { - return parsed; - } - } - - const fenced = text.match(/```json\s*([\s\S]*?)```/i)?.[1]?.trim(); - if (fenced) { - const parsed = parseBugJson(fenced); - if (parsed.length > 0) { - return parsed; - } - } - - return []; -} - -function parseBugJson(input: string): BugRecord[] { - try { - const parsed = JSON.parse(input) as unknown; - if (Array.isArray(parsed)) { - return parsed - .map((item) => { - if (!item || typeof item !== "object") { - return null; - } - const bug = item as Record; - if (typeof bug.title !== "string" || typeof bug.body !== "string") { - return null; - } - return { - title: bug.title, - body: bug.body, - } satisfies BugRecord; - }) - .filter((item): item is BugRecord => item !== null); - } - } catch { - return []; - } - return []; -} - async function safeLinearComment( linear: LinearClient, issueId: string, @@ -681,22 +552,6 @@ async function safeLinearComment( } } -async function safeLinearStageUpdate( - linear: LinearClient, - issueId: string, - stage: keyof ResolvedProjectConfig["linear"]["statusMap"], -): Promise { - const runLogger = logger.child({ issueId, stage }); - try { - await linear.markStage(issueId, stage); - } catch (error) { - runLogger.error( - { err: normalizeError(error) }, - "Failed to update Linear stage", - ); - } -} - async function safeLinearMoveToCanceled( linear: LinearClient, issueId: string, diff --git a/tests/codex.test.ts b/tests/codex.test.ts index 713a3bf8..d0ff09db 100644 --- a/tests/codex.test.ts +++ b/tests/codex.test.ts @@ -37,6 +37,7 @@ const config: ResolvedProjectConfig = { github: { useGhCli: true, defaultBugLabel: "bug" }, codex: { binary: "codex", + streamLogs: false, model: "gpt-5.4", models: { plan: "gpt-5.5", diff --git a/tests/comments.test.ts b/tests/comments.test.ts new file mode 100644 index 00000000..ea2a0472 --- /dev/null +++ b/tests/comments.test.ts @@ -0,0 +1,96 @@ +import { describe, expect, it } from "bun:test"; +import { + buildBugsCanceledComment, + buildImplementationComment, + buildPlanComment, + buildReviewComment, + formatCodexUsageLine, +} from "../src/comments"; + +describe("buildPlanComment", () => { + it("includes header and plan summary", () => { + const comment = buildPlanComment("ENG-1", "1. Do A\n2. Do B", { + inputTokens: 12, + outputTokens: 8, + }); + expect(comment).toContain("PIV loop plan for ENG-1"); + expect(comment).toContain("Planning completed; implementation started."); + expect(comment).toContain("Token usage: input 12, output 8, total 20"); + expect(comment).toContain("1. Do A"); + }); + + it("uses fallback when no summary is returned", () => { + const comment = buildPlanComment("ENG-1", " "); + expect(comment).toContain("(No plan summary returned by planning agent.)"); + expect(comment).toContain("Token usage: unknown"); + }); +}); + +describe("formatCodexUsageLine", () => { + it("formats full usage values", () => { + expect( + formatCodexUsageLine({ + inputTokens: 3, + outputTokens: 7, + totalTokens: 10, + }), + ).toBe("Token usage: input 3, output 7, total 10"); + }); + + it("derives total when missing", () => { + expect( + formatCodexUsageLine({ + inputTokens: 9, + outputTokens: 4, + }), + ).toBe("Token usage: input 9, output 4, total 13"); + }); + + it("handles missing fields", () => { + expect(formatCodexUsageLine({ inputTokens: 5 })).toBe( + "Token usage: input 5, output unknown, total unknown", + ); + expect(formatCodexUsageLine()).toBe("Token usage: unknown"); + }); +}); + +describe("buildImplementationComment", () => { + it("includes draft PR URL and usage", () => { + const comment = buildImplementationComment("https://example.com/pr/1", { + inputTokens: 2, + outputTokens: 3, + }); + expect(comment).toContain("Implementation completed. Draft PR:"); + expect(comment).toContain("https://example.com/pr/1"); + expect(comment).toContain("Token usage: input 2, output 3, total 5"); + }); +}); + +describe("buildReviewComment", () => { + it("renders pass review summary", () => { + const comment = buildReviewComment({ + issueKey: "ENG-1", + passed: true, + summary: "Looks good.", + usage: { inputTokens: 1, outputTokens: 2 }, + bugs: [], + }); + expect(comment).toContain("PIV loop review for ENG-1"); + expect(comment).toContain("Result: PASS"); + expect(comment).toContain("No bugs found."); + }); +}); + +describe("buildBugsCanceledComment", () => { + it("includes bug list with issue urls when available", () => { + const comment = buildBugsCanceledComment([ + { title: "Bug A", body: "x", issueUrl: "https://example.com/issues/1" }, + { title: "Bug B", body: "y" }, + ]); + expect(comment).toContain( + "Review/testing found bugs. Moved issue to Canceled.", + ); + expect(comment).toContain("- Bug A (https://example.com/issues/1)"); + expect(comment).toContain("- Bug B"); + }); +}); diff --git a/tests/config.test.ts b/tests/config.test.ts index cd77c734..9fadae25 100644 --- a/tests/config.test.ts +++ b/tests/config.test.ts @@ -21,6 +21,8 @@ const envKeys = [ "CODEX_MODEL_PLAN", "CODEX_MODEL_IMPLEMENT", "CODEX_MODEL_REVIEW_TEST", + "PIV_DEV_MODE", + "PIV_PRINT_CODEX_LOGS", "PIV_POLL_INTERVAL_MS", "PIV_MAX_POLL_CYCLES", "PIV_EXIT_WHEN_IDLE", @@ -41,9 +43,11 @@ describe("loadConfig", () => { ? "30000" : key === "PIV_MAX_POLL_CYCLES" ? "" - : key === "PIV_EXIT_WHEN_IDLE" - ? "1" - : key.toLowerCase(); + : key === "PIV_DEV_MODE" || key === "PIV_PRINT_CODEX_LOGS" + ? "0" + : key === "PIV_EXIT_WHEN_IDLE" + ? "1" + : key.toLowerCase(); } }); @@ -137,4 +141,22 @@ describe("loadConfig", () => { expect(config.projects[0]?.codex.models?.implement).toBe("gpt-5.3-codex"); expect(config.projects[0]?.codex.models?.reviewTest).toBe("gpt-5.3-codex"); }); + + it("does not stream codex logs by default", async () => { + process.env.PIV_DEV_MODE = "0"; + process.env.PIV_PRINT_CODEX_LOGS = "0"; + const config = await loadConfig(process.cwd()); + expect(config.projects[0]?.codex.streamLogs).toBe(false); + }); + + it("streams codex logs when enabled by env", async () => { + process.env.PIV_DEV_MODE = "1"; + const configFromDevMode = await loadConfig(process.cwd()); + expect(configFromDevMode.projects[0]?.codex.streamLogs).toBe(true); + + process.env.PIV_DEV_MODE = "0"; + process.env.PIV_PRINT_CODEX_LOGS = "1"; + const configFromLegacyFlag = await loadConfig(process.cwd()); + expect(configFromLegacyFlag.projects[0]?.codex.streamLogs).toBe(true); + }); }); diff --git a/tests/review.test.ts b/tests/review.test.ts new file mode 100644 index 00000000..50966e98 --- /dev/null +++ b/tests/review.test.ts @@ -0,0 +1,66 @@ +import { describe, expect, it } from "bun:test"; +import { parseReviewOutcome } from "../src/review"; + +describe("parseReviewOutcome", () => { + it("parses pass with no bugs", () => { + const text = ` +RESULT: PASS +SUMMARY: Looks good. +BUGS_JSON: +[] +`; + const outcome = parseReviewOutcome(text); + expect(outcome.passed).toBe(true); + expect(outcome.bugs).toHaveLength(0); + }); + + it("parses fail with bugs", () => { + const text = ` +RESULT: FAIL +SUMMARY: Found regressions. +BUGS_JSON: +[{"title":"Bug A","body":"Details"}] +`; + const outcome = parseReviewOutcome(text); + expect(outcome.passed).toBe(false); + expect(outcome.bugs).toHaveLength(1); + expect(outcome.bugs[0]?.title).toBe("Bug A"); + }); + + it("treats mixed pass/fail markers as failure", () => { + const text = ` +RESULT: PASS +RESULT: FAIL +SUMMARY: Mixed output. +BUGS_JSON: +[] +`; + const outcome = parseReviewOutcome(text); + expect(outcome.passed).toBe(false); + }); + + it("falls back to empty bug list when BUGS_JSON is malformed", () => { + const text = ` +RESULT: FAIL +SUMMARY: Bad json. +BUGS_JSON: +[{"title":"Bug A","body":"oops"} +`; + const outcome = parseReviewOutcome(text); + expect(outcome.bugs).toEqual([]); + expect(outcome.passed).toBe(false); + }); + + it("parses bug list from fenced json when BUGS_JSON is missing", () => { + const text = ` +RESULT: FAIL +SUMMARY: Found issues. +\`\`\`json +[{"title":"Bug A","body":"Details"}] +\`\`\` +`; + const outcome = parseReviewOutcome(text); + expect(outcome.bugs).toHaveLength(1); + expect(outcome.bugs[0]?.title).toBe("Bug A"); + }); +}); diff --git a/tests/workflow.test.ts b/tests/workflow.test.ts index 10dbbaed..083841b0 100644 --- a/tests/workflow.test.ts +++ b/tests/workflow.test.ts @@ -7,88 +7,11 @@ import type { import { appendCodexUsage, buildIssueJobLogFields, - buildPlanComment, - formatCodexUsageLine, - parseReviewOutcome, resolvePollingSettings, routeProjectsForIssueProjectId, shouldStopPolling, } from "../src/workflow"; -describe("parseReviewOutcome", () => { - it("parses pass with no bugs", () => { - const text = ` -RESULT: PASS -SUMMARY: Looks good. -BUGS_JSON: -[] -`; - const outcome = parseReviewOutcome(text); - expect(outcome.passed).toBe(true); - expect(outcome.bugs).toHaveLength(0); - }); - - it("parses fail with bugs", () => { - const text = ` -RESULT: FAIL -SUMMARY: Found regressions. -BUGS_JSON: -[{"title":"Bug A","body":"Details"}] -`; - const outcome = parseReviewOutcome(text); - expect(outcome.passed).toBe(false); - expect(outcome.bugs).toHaveLength(1); - expect(outcome.bugs[0]?.title).toBe("Bug A"); - }); -}); - -describe("buildPlanComment", () => { - it("includes header and plan summary", () => { - const comment = buildPlanComment("ENG-1", "1. Do A\n2. Do B", { - inputTokens: 12, - outputTokens: 8, - }); - expect(comment).toContain("PIV loop plan for ENG-1"); - expect(comment).toContain("Planning completed; implementation started."); - expect(comment).toContain("Token usage: input 12, output 8, total 20"); - expect(comment).toContain("1. Do A"); - }); - - it("uses fallback when no summary is returned", () => { - const comment = buildPlanComment("ENG-1", " "); - expect(comment).toContain("(No plan summary returned by planning agent.)"); - expect(comment).toContain("Token usage: unknown"); - }); -}); - -describe("formatCodexUsageLine", () => { - it("formats full usage values", () => { - expect( - formatCodexUsageLine({ - inputTokens: 3, - outputTokens: 7, - totalTokens: 10, - }), - ).toBe("Token usage: input 3, output 7, total 10"); - }); - - it("derives total when missing", () => { - expect( - formatCodexUsageLine({ - inputTokens: 9, - outputTokens: 4, - }), - ).toBe("Token usage: input 9, output 4, total 13"); - }); - - it("handles missing fields", () => { - expect(formatCodexUsageLine({ inputTokens: 5 })).toBe( - "Token usage: input 5, output unknown, total unknown", - ); - expect(formatCodexUsageLine()).toBe("Token usage: unknown"); - }); -}); - describe("resolvePollingSettings", () => { const polling: PollingConfig = { intervalMs: 30000, @@ -361,6 +284,7 @@ function createProject( }, codex: { binary: "codex", + streamLogs: false, }, skills: { plan: "/tmp/plan.md",