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
This commit is contained in:
Dotta authored and GitHub committed 2026-08-30 00:27:19 -05:00
1 parent 83243d4b5d
commit 3942739906
2 files changed
+492

No files matched your search

@@ -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`,
}),
]);
});
});
@@ -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;
}