From 39427399063eaf36436efb3267a4459310215fd6 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Sun, 30 Aug 2026 00:27:19 -0500 Subject: [PATCH] feat(runner): parse bounded Codex turn diffs (#12363) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Runner events need provider-neutral workspace change facts > - Codex reports one complete unified diff snapshot per turn > - Provider diff text can be large and can contain unsafe paths > - This pull request adds only the bounded pure parser > - A later pull request will connect it to the Codex driver > - The benefit is an independently tested workspace boundary ## Linked Issues or Issue Description **Subsystem affected** `packages/paperclip-runner` Codex event normalization. **Problem or motivation** Codex turn diffs need stable file operations and statistics. Raw diff input must not escape the workspace or grow without bounds. **Proposed solution** Parse complete unified diff snapshots into normalized file records. Bound file count and retained text, reject unsafe paths, and represent binary changes without text. **Alternatives considered** Parsing diffs inside the main driver would make provider lifecycle review larger and harder to test in isolation. **Roadmap alignment** This supports the Codex-first experimental runner. It does not enable an adapter. ## What Changed - Added create, modify, delete, rename, mode-change, and binary parsing. - Added workspace-relative path validation. - Added file-count and per-file text bounds. - Added focused rename, binary, hostile path, and size tests. ## Verification - `pnpm --filter @paperclipai/paperclip-runner test:typescript` - `pnpm -r typecheck` - `pnpm build` - The focused parser test has 2 passing cases. ## Risks Low risk. This is a pure parser with no file-system access and no production caller yet. ## Model Used OpenAI Codex with GPT-5.6 and repository tool use. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --- .../src/drivers/codex/codex-turn-diff.test.ts | 285 ++++++++++++++++++ .../src/drivers/codex/codex-turn-diff.ts | 207 +++++++++++++ 2 files changed, 492 insertions(+) create mode 100644 packages/paperclip-runner/src/drivers/codex/codex-turn-diff.test.ts create mode 100644 packages/paperclip-runner/src/drivers/codex/codex-turn-diff.ts diff --git a/packages/paperclip-runner/src/drivers/codex/codex-turn-diff.test.ts b/packages/paperclip-runner/src/drivers/codex/codex-turn-diff.test.ts new file mode 100644 index 0000000000..be19c053b6 --- /dev/null +++ b/packages/paperclip-runner/src/drivers/codex/codex-turn-diff.test.ts @@ -0,0 +1,285 @@ +import { describe, expect, it } from "vitest"; + +import { parseCodexTurnDiff } from "./codex-turn-diff.js"; + +describe("Codex turn diff parser", () => { + it("parses a complete snapshot with bounded file statistics", () => { + expect(parseCodexTurnDiff([ + "diff --git a/src/old.ts b/src/new.ts", + "similarity index 95%", + "rename from src/old.ts", + "rename to src/new.ts", + "--- a/src/old.ts", + "+++ b/src/new.ts", + "@@ -1 +1,2 @@", + "-old", + "+new", + "+another", + "diff --git a/assets/image.png b/assets/image.png", + "Binary files a/assets/image.png and b/assets/image.png differ", + ].join("\n"))).toEqual([ + expect.objectContaining({ + path: "src/new.ts", + previousPath: "src/old.ts", + operation: "rename", + additions: 2, + deletions: 1, + binary: false, + }), + expect.objectContaining({ + path: "assets/image.png", + operation: "modify", + additions: null, + deletions: null, + binary: true, + }), + ]); + }); + + it("bounds aggregate diffs and rejects unsafe workspace paths", () => { + const patches = Array.from({ length: 2_001 }, (_, index) => [ + `diff --git a/src/file-${index}.ts b/src/file-${index}.ts`, + `--- a/src/file-${index}.ts`, + `+++ b/src/file-${index}.ts`, + "@@ -1 +1 @@", + "-old", + "+new", + ].join("\n")); + expect(parseCodexTurnDiff(patches.join("\n"))).toHaveLength(2_000); + + const oversized = parseCodexTurnDiff([ + "diff --git a/src/large.ts b/src/large.ts", + "--- a/src/large.ts", + "+++ b/src/large.ts", + "@@ -0,0 +1 @@", + `+${"x".repeat(300_000)}`, + ].join("\n")); + expect(oversized[0]?.diff).toHaveLength(256 * 1_024); + + expect(parseCodexTurnDiff([ + "diff --git a/../../secret.txt b/../../secret.txt", + "--- a/../../secret.txt", + "+++ b/../../secret.txt", + "@@ -1 +1 @@", + "-old", + "+new", + ].join("\n"))).toEqual([]); + + expect(parseCodexTurnDiff([ + String.raw`diff --git a/C:\Windows\secret.txt b/C:\Windows\secret.txt`, + String.raw`--- a/C:\Windows\secret.txt`, + String.raw`+++ b/C:\Windows\secret.txt`, + "@@ -1 +1 @@", + "-old", + "+new", + ].join("\n"))).toEqual([]); + }); + + it("keeps file headers distinct from hunk content with marker prefixes", () => { + expect(parseCodexTurnDiff([ + "diff --git a/src/markers.ts b/src/markers.ts", + "--- a/src/markers.ts", + "+++ b/src/markers.ts", + "@@ -1 +1 @@", + "--- old content", + "+++ new content", + ].join("\n"))).toEqual([ + expect.objectContaining({ + path: "src/markers.ts", + operation: "modify", + additions: 1, + deletions: 1, + }), + ]); + }); + + it("does not split file records for diff headers embedded in hunk lines", () => { + const firstPatch = [ + "diff --git a/src/first.ts b/src/first.ts", + "--- a/src/first.ts", + "+++ b/src/first.ts", + "@@ -1,2 +1,2 @@", + " diff --git a/context.ts b/context.ts", + "-diff --git a/deleted.ts b/deleted.ts", + "+diff --git a/added.ts b/added.ts", + ]; + const secondPatch = [ + "diff --git a/src/second.ts b/src/second.ts", + "--- a/src/second.ts", + "+++ b/src/second.ts", + "@@ -1 +1 @@", + "-old", + "+new", + ]; + + expect(parseCodexTurnDiff([...firstPatch, ...secondPatch].join("\n"))).toEqual([ + expect.objectContaining({ + path: "src/first.ts", + additions: 1, + deletions: 1, + diff: `${firstPatch.join("\n")}\n`, + }), + expect.objectContaining({ + path: "src/second.ts", + additions: 1, + deletions: 1, + diff: `${secondPatch.join("\n")}\n`, + }), + ]); + }); + + it("fails closed for malformed, overflowing, and incomplete hunks", () => { + const completeFile = [ + "diff --git a/src/complete.ts b/src/complete.ts", + "--- a/src/complete.ts", + "+++ b/src/complete.ts", + "@@ -1 +1 @@", + "-old", + "+new", + ]; + + expect(parseCodexTurnDiff([ + ...completeFile, + "diff --git a/src/malformed.ts b/src/malformed.ts", + "--- a/src/malformed.ts", + "+++ b/src/malformed.ts", + "@@ -1 +not-a-count @@", + "diff --git a/src/fabricated.ts b/src/fabricated.ts", + ].join("\n"))).toEqual([ + expect.objectContaining({ path: "src/complete.ts" }), + ]); + + expect(parseCodexTurnDiff([ + ...completeFile, + "diff --git a/src/incomplete-tail.ts b/src/incomplete-tail.ts", + "--- a/src/incomplete-tail.ts", + "+++ b/src/incomplete-tail.ts", + "@@ -1,2 +1,2 @@", + "diff --git a/src/fake.ts b/src/fake.ts", + " unchanged", + "-old", + "+new", + "diff --git a/src/later.ts b/src/later.ts", + "--- a/src/later.ts", + "+++ b/src/later.ts", + "@@ -1 +1 @@", + "-old", + "+new", + ].join("\n"))).toEqual([ + expect.objectContaining({ path: "src/complete.ts" }), + ]); + + expect(parseCodexTurnDiff([ + ...completeFile, + "diff --git a/src/overflow.ts b/src/overflow.ts", + "--- a/src/overflow.ts", + "+++ b/src/overflow.ts", + "@@ -9007199254740992 +1 @@", + ].join("\n"))).toEqual([ + expect.objectContaining({ path: "src/complete.ts" }), + ]); + + expect(parseCodexTurnDiff([ + "diff --git a/src/incomplete.ts b/src/incomplete.ts", + "--- a/src/incomplete.ts", + "+++ b/src/incomplete.ts", + "@@ -1,2 +1,2 @@", + "-old", + "+new", + ].join("\n"))).toEqual([]); + }); + + it.each([ + { label: "an added-line overrun", header: "@@ -1 +1,0 @@", lines: ["+unexpected", "-old"] }, + { label: "a deleted-line overrun", header: "@@ -1,0 +1 @@", lines: ["-unexpected", "+new"] }, + { label: "a context-line side overrun", header: "@@ -1,0 +1 @@", lines: [" unchanged", "+new"] }, + { label: "content after both sides are complete", header: "@@ -1 +1 @@", lines: ["-old", "+new", "+extra"] }, + ])("rejects $label", ({ header, lines }) => { + expect(parseCodexTurnDiff([ + "diff --git a/src/overrun.ts b/src/overrun.ts", + "--- a/src/overrun.ts", + "+++ b/src/overrun.ts", + header, + ...lines, + ].join("\n"))).toEqual([]); + }); + + it("supports multiple hunks, zero-count sides, and no-newline markers", () => { + const patch = [ + "diff --git a/src/multiple.ts b/src/multiple.ts", + "--- a/src/multiple.ts", + "+++ b/src/multiple.ts", + "@@ -0,0 +1 @@", + "+added", + "\\ No newline at end of file", + "@@ -2 +2,0 @@", + "-removed", + "\\ No newline at end of file", + "diff --git a/src/next.ts b/src/next.ts", + "--- a/src/next.ts", + "+++ b/src/next.ts", + "@@ -0,0 +1 @@", + "+next", + ].join("\n"); + + expect(parseCodexTurnDiff(patch)).toEqual([ + expect.objectContaining({ + path: "src/multiple.ts", + additions: 1, + deletions: 1, + }), + expect.objectContaining({ + path: "src/next.ts", + additions: 1, + deletions: 0, + }), + ]); + }); + + it("does not interpret rename or mode metadata after a hunk begins", () => { + expect(parseCodexTurnDiff([ + "diff --git a/src/markers.ts b/src/markers.ts", + "--- a/src/markers.ts", + "+++ b/src/markers.ts", + "@@ -1,5 +1,5 @@", + " rename from ../../outside.ts", + " rename to src/renamed.ts", + " old mode 100644", + " new mode 100755", + "-old", + "+new", + ].join("\n"))).toEqual([ + expect.objectContaining({ + path: "src/markers.ts", + previousPath: null, + operation: "modify", + additions: 1, + deletions: 1, + }), + ]); + }); + + it("does not interpret binary markers after a text hunk begins", () => { + const patch = [ + "diff --git a/src/markers.ts b/src/markers.ts", + "--- a/src/markers.ts", + "+++ b/src/markers.ts", + "@@ -1,3 +1,3 @@", + " Binary files are described in this text hunk", + " GIT binary patch", + "-old", + "+new", + ].join("\n"); + + expect(parseCodexTurnDiff(patch)).toEqual([ + expect.objectContaining({ + path: "src/markers.ts", + operation: "modify", + additions: 1, + deletions: 1, + binary: false, + diff: `${patch}\n`, + }), + ]); + }); +}); diff --git a/packages/paperclip-runner/src/drivers/codex/codex-turn-diff.ts b/packages/paperclip-runner/src/drivers/codex/codex-turn-diff.ts new file mode 100644 index 0000000000..c92bf713f0 --- /dev/null +++ b/packages/paperclip-runner/src/drivers/codex/codex-turn-diff.ts @@ -0,0 +1,207 @@ +export interface ParsedCodexTurnDiffFile { + path: string; + operation: "create" | "modify" | "delete" | "rename" | "mode_change"; + previousPath: string | null; + additions: number | null; + deletions: number | null; + binary: boolean; + diff: string | null; +} + +const MAX_TURN_DIFF_FILES = 2_000; +const MAX_TURN_DIFF_CHARS_PER_FILE = 256 * 1024; + +function gitDiffHunkCounts(line: string): { old: number; new: number } | null { + const match = line.match(/^@@ -(\d+)(?:,(\d+))? \+(\d+)(?:,(\d+))? @@(?: .*)?$/); + if (!match) return null; + const oldStart = Number(match[1]); + const oldCount = match[2] === undefined ? 1 : Number(match[2]); + const newStart = Number(match[3]); + const newCount = match[4] === undefined ? 1 : Number(match[4]); + if ( + !Number.isSafeInteger(oldStart) || !Number.isSafeInteger(oldCount) || + !Number.isSafeInteger(newStart) || !Number.isSafeInteger(newCount) + ) return null; + return { old: oldCount, new: newCount }; +} + +function gitDiffPath(value: string): string | null { + let candidate = value.trim(); + if (candidate === "/dev/null") return null; + if (candidate.startsWith('"') && candidate.endsWith('"')) { + try { + candidate = JSON.parse(candidate) as string; + } catch { + return null; + } + } + if (candidate.startsWith("a/") || candidate.startsWith("b/")) candidate = candidate.slice(2); + // Reject a native Windows drive path before slash normalization so the + // workspace boundary is explicit for either separator spelling. + if (/^[A-Za-z]:[\\/]/u.test(candidate)) return null; + candidate = candidate.replaceAll("\\", "/"); + if (candidate.startsWith("a/") || candidate.startsWith("b/")) candidate = candidate.slice(2); + if ( + !candidate || + candidate.length > 1_024 || + candidate.startsWith("/") || + candidate.startsWith("//") || + /^[A-Za-z]:\//u.test(candidate) || + candidate.split("/").some((part) => part === ".." || part.length === 0) + ) return null; + return candidate; +} + +/** Parse one complete Codex `turn/diff/updated` snapshot without consulting git or the live workspace. */ +export function parseCodexTurnDiff(value: unknown): ParsedCodexTurnDiffFile[] { + const patch = typeof value === "string" ? value : ""; + if (!patch.trim()) return []; + const files: ParsedCodexTurnDiffFile[] = []; + let current: { + lines: string[]; + oldPath: string | null; + newPath: string | null; + renameFrom: string | null; + renameTo: string | null; + additions: number; + deletions: number; + binary: boolean; + modeChange: boolean; + inHunk: boolean; + oldHunkLinesRemaining: number | null; + newHunkLinesRemaining: number | null; + valid: boolean; + } | null = null; + + const finish = () => { + const incompleteHunk = current !== null && current.inHunk && ( + current.oldHunkLinesRemaining !== 0 || current.newHunkLinesRemaining !== 0 + ); + if (!current || !current.valid || incompleteHunk || files.length >= MAX_TURN_DIFF_FILES) return; + const path = current.renameTo ?? current.newPath ?? current.oldPath; + if (!path) return; + const previousPath = current.renameFrom ?? (current.renameTo ? current.oldPath : null); + const operation = current.renameTo && previousPath + ? "rename" + : current.oldPath === null + ? "create" + : current.newPath === null + ? "delete" + : current.modeChange && current.additions === 0 && current.deletions === 0 + ? "mode_change" + : "modify"; + const completeDiff = `${current.lines.join("\n")}\n`; + files.push({ + path, + operation, + previousPath, + additions: current.binary ? null : current.additions, + deletions: current.binary ? null : current.deletions, + binary: current.binary, + diff: current.binary ? null : completeDiff.slice(0, MAX_TURN_DIFF_CHARS_PER_FILE), + }); + }; + + for (const line of patch.split("\n")) { + const hunkComplete = current !== null && current.inHunk && + current.oldHunkLinesRemaining === 0 && current.newHunkLinesRemaining === 0; + const header = line.startsWith("diff --git ") + ? line.match(/^diff --git ("(?:\\.|[^"])*"|\S+) ("(?:\\.|[^"])*"|\S+)$/) + : null; + if ((!current || !current.inHunk || hunkComplete) && header) { + finish(); + current = { + lines: [line], + oldPath: gitDiffPath(header[1] ?? ""), + newPath: gitDiffPath(header[2] ?? ""), + renameFrom: null, + renameTo: null, + additions: 0, + deletions: 0, + binary: false, + modeChange: false, + inHunk: false, + oldHunkLinesRemaining: null, + newHunkLinesRemaining: null, + valid: true, + }; + continue; + } + if (!current) continue; + current.lines.push(line); + if (!current.inHunk && line.startsWith("diff --git ") && !header) { + current.valid = false; + continue; + } + if (!current.inHunk && line.startsWith("--- ")) current.oldPath = gitDiffPath(line.slice(4)); + else if (!current.inHunk && line.startsWith("+++ ")) current.newPath = gitDiffPath(line.slice(4)); + else if (!current.inHunk && line.startsWith("rename from ")) current.renameFrom = gitDiffPath(line.slice(12)); + else if (!current.inHunk && line.startsWith("rename to ")) current.renameTo = gitDiffPath(line.slice(10)); + else if (!current.inHunk && (line.startsWith("old mode ") || line.startsWith("new mode "))) current.modeChange = true; + else if (!current.inHunk && (line.startsWith("Binary files ") || line === "GIT binary patch")) current.binary = true; + else if ((!current.inHunk || hunkComplete) && line.startsWith("@@")) { + const counts = gitDiffHunkCounts(line); + if (!counts) { + current.valid = false; + current.inHunk = true; + current.oldHunkLinesRemaining = null; + current.newHunkLinesRemaining = null; + continue; + } + current.inHunk = true; + current.oldHunkLinesRemaining = counts.old; + current.newHunkLinesRemaining = counts.new; + } else if (current.inHunk) { + if (!current.valid || line === "\\ No newline at end of file") { + continue; + } else if (line.startsWith("+")) { + if (current.newHunkLinesRemaining === null || current.newHunkLinesRemaining === 0) { + current.valid = false; + current.oldHunkLinesRemaining = null; + current.newHunkLinesRemaining = null; + continue; + } + current.additions += 1; + current.newHunkLinesRemaining -= 1; + } else if (line.startsWith("-")) { + if (current.oldHunkLinesRemaining === null || current.oldHunkLinesRemaining === 0) { + current.valid = false; + current.oldHunkLinesRemaining = null; + current.newHunkLinesRemaining = null; + continue; + } + current.deletions += 1; + current.oldHunkLinesRemaining -= 1; + } else if (line.startsWith(" ")) { + if ( + current.oldHunkLinesRemaining === null || current.oldHunkLinesRemaining === 0 || + current.newHunkLinesRemaining === null || current.newHunkLinesRemaining === 0 + ) { + current.valid = false; + current.oldHunkLinesRemaining = null; + current.newHunkLinesRemaining = null; + continue; + } + current.oldHunkLinesRemaining -= 1; + current.newHunkLinesRemaining -= 1; + } else if (line.startsWith("@@")) { + current.valid = false; + current.oldHunkLinesRemaining = null; + current.newHunkLinesRemaining = null; + continue; + } else if (!hunkComplete) { + current.valid = false; + current.oldHunkLinesRemaining = null; + current.newHunkLinesRemaining = null; + continue; + } else if (hunkComplete && line.startsWith("diff --git ")) { + current.valid = false; + current.oldHunkLinesRemaining = null; + current.newHunkLinesRemaining = null; + continue; + } + } + } + finish(); + return files; +}