From ac7f6ec1a3186b3ccf62d40a5e096d9481c32c9d Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Sun, 30 Aug 2026 01:03:22 -0500 Subject: [PATCH] feat(runner): normalize Codex thread state (#12367) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Codex thread state arrives as provider-specific goals, lineage, notifications, and workspace paths > - That data must be normalized before the full driver can retain or project it > - Notifications also need run and thread binding so unrelated provider traffic is ignored > - This pull request adds pure normalization helpers before the full driver > - A later pull request will use these helpers for the Codex session lifecycle > - The benefit is a small, independently tested trust boundary for thread state ## Linked Issues or Issue Description **Subsystem affected** `packages/paperclip-runner` Codex thread-state normalization. **Problem or motivation** Provider thread data can contain unsupported goal shapes, unrelated notifications, unsafe workspace paths, or unbounded response values. Passing it through directly would weaken run isolation and durable-data bounds. **Proposed solution** Normalize goals and lineage into stable runner shapes, accept notifications only when their run and thread identities match, constrain workspace references to the assigned root, and retain only bounded safe provider responses. **Alternatives considered** Keeping these rules embedded in the full driver would make the trust boundary harder to review and test independently. **Roadmap alignment** This supports the Codex-first experimental runner. It does not enable the runner adapter. ## What Changed - Added normalized Codex thread goals and lineage. - Added run- and thread-bound notification filtering. - Added safe workspace-relative path and stat projection. - Added bounded provider-response retention. - Added focused normalization and isolation tests. ## Verification - `pnpm --filter @paperclipai/paperclip-runner test:typescript` - `pnpm -r typecheck` - `pnpm build` - The focused thread-normalization test has 3 passing cases. ## Risks The main risk is retaining data from the wrong provider thread or accepting an unsafe workspace reference. Tests cover identity binding, path normalization, response bounds, goal parsing, and lineage projection. ## 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 --- .../codex/codex-thread-normalization.test.ts | 137 +++++++++++ .../codex/codex-thread-normalization.ts | 226 ++++++++++++++++++ 2 files changed, 363 insertions(+) create mode 100644 packages/paperclip-runner/src/drivers/codex/codex-thread-normalization.test.ts create mode 100644 packages/paperclip-runner/src/drivers/codex/codex-thread-normalization.ts diff --git a/packages/paperclip-runner/src/drivers/codex/codex-thread-normalization.test.ts b/packages/paperclip-runner/src/drivers/codex/codex-thread-normalization.test.ts new file mode 100644 index 0000000000..1bdbd7f657 --- /dev/null +++ b/packages/paperclip-runner/src/drivers/codex/codex-thread-normalization.test.ts @@ -0,0 +1,137 @@ +import { describe, expect, it } from "vitest"; + +import { + boundedCodexWorkspaceStat, + codexThreadLineage, + codexWorkspaceRelativePath, + isBoundCodexNotification, + parseCodexThreadGoal, + safeCodexRequestResponse, +} from "./codex-thread-normalization.js"; + +describe("Codex thread normalization", () => { + it("normalizes goals and snake-case subagent lineage", () => { + expect(parseCodexThreadGoal({ + threadId: "thread-1", + objective: "Complete the task", + status: "active", + tokenBudget: 10_000, + tokensUsed: 500, + timeUsedSeconds: 30, + createdAt: 1, + updatedAt: 2, + })).toMatchObject({ + threadId: "thread-1", + status: "active", + tokenBudget: 10_000, + tokensUsed: 500, + }); + expect(parseCodexThreadGoal({ + threadId: "thread-1", + objective: "", + status: "active", + })).toBeNull(); + for (const invalidNumber of [Number.NaN, Number.POSITIVE_INFINITY, -1, 1.5]) { + expect(parseCodexThreadGoal({ + threadId: "thread-1", + objective: "Invalid counters must fail closed", + status: "active", + tokensUsed: invalidNumber, + })).toBeNull(); + } + + expect(codexThreadLineage({ + id: "child", + sessionId: "provider-child", + status: { type: "idle" }, + source: { + subagent: { + thread_spawn: { + parent_thread_id: "parent", + depth: 2, + agent_nickname: "builder", + agent_role: "worker", + }, + }, + }, + })).toEqual({ + threadId: "child", + providerSessionId: "provider-child", + parentThreadId: "parent", + depth: 2, + nickname: "builder", + role: "worker", + status: "idle", + }); + }); + + it("admits only bound notifications and safe workspace paths", () => { + const binding = { runId: "run-1", threadIds: ["thread-1"] }; + expect(isBoundCodexNotification({ + method: "turn/started", + params: { threadId: "thread-1" }, + }, binding)).toBe(true); + expect(isBoundCodexNotification({ + method: "item/completed", + params: { + thread: { + id: "unbound-child", + source: { subagent: { thread_spawn: { parent_thread_id: "thread-1" } } }, + }, + }, + }, binding)).toBe(false); + expect(isBoundCodexNotification({ + method: "item/completed", + params: { runId: "run-1" }, + }, binding)).toBe(false); + expect(isBoundCodexNotification({ + method: "thread/started", + params: { + thread: { + id: "child-thread", + source: { subagent: { thread_spawn: { parent_thread_id: "thread-1" } } }, + }, + }, + }, binding)).toBe(true); + expect(isBoundCodexNotification({ + method: "turn/started", + params: { threadId: "other-thread" }, + }, binding)).toBe(false); + expect(isBoundCodexNotification({ + method: "item/completed", + params: { runId: "other-run", threadId: "thread-1" }, + }, binding)).toBe(false); + expect(isBoundCodexNotification({ + method: "unknown/provider/event", + params: { runId: "run-1" }, + }, binding)).toBe(false); + expect(isBoundCodexNotification({ + method: "warning", + params: {}, + }, binding)).toBe(false); + + expect(codexWorkspaceRelativePath("src\\index.ts")).toBe("src/index.ts"); + expect(codexWorkspaceRelativePath("../secret")).toBeNull(); + expect(codexWorkspaceRelativePath("/absolute/path")).toBeNull(); + expect(codexWorkspaceRelativePath("C:/host/path")).toBeNull(); + expect(codexWorkspaceRelativePath("C:../secret")).toBeNull(); + expect(codexWorkspaceRelativePath("c:..\\secret")).toBeNull(); + expect(codexWorkspaceRelativePath("Z:relative\\host-path")).toBeNull(); + expect(boundedCodexWorkspaceStat(12)).toBe(12); + expect(boundedCodexWorkspaceStat(-1)).toBeNull(); + }); + + it("returns provider-safe terminal responses for unresolved requests", () => { + expect(safeCodexRequestResponse("item/permissions/requestApproval")).toEqual({ + permissions: {}, + scope: "turn", + }); + expect(safeCodexRequestResponse("mcpServer/elicitation/request", "cancel")).toEqual({ + action: "cancel", + content: null, + _meta: null, + }); + expect(safeCodexRequestResponse("tool/requestUserInput")).toEqual({ answers: {} }); + expect(safeCodexRequestResponse("applyPatchApproval")).toEqual({ decision: "decline" }); + }); +}); diff --git a/packages/paperclip-runner/src/drivers/codex/codex-thread-normalization.ts b/packages/paperclip-runner/src/drivers/codex/codex-thread-normalization.ts new file mode 100644 index 0000000000..ea596093f2 --- /dev/null +++ b/packages/paperclip-runner/src/drivers/codex/codex-thread-normalization.ts @@ -0,0 +1,226 @@ +import type { + HarnessThreadGoal, + HarnessThreadLineageEntry, +} from "../../contracts/harness-driver.js"; + +function record(value: unknown): Record { + return typeof value === "object" && value !== null && !Array.isArray(value) + ? (value as Record) + : {}; +} + +function text(value: unknown, fallback = ""): string { + return typeof value === "string" ? value : fallback; +} + +function nonNegativeInteger(value: unknown): number | null { + return typeof value === "number" && Number.isSafeInteger(value) && value >= 0 + ? value + : null; +} + +export function parseCodexThreadGoal(value: unknown): HarnessThreadGoal | null { + const goal = record(value); + const threadId = text(goal.threadId); + const objective = text(goal.objective); + const status = text(goal.status); + if ( + threadId.length === 0 || + objective.length === 0 || + ![ + "active", + "paused", + "blocked", + "usageLimited", + "budgetLimited", + "complete", + ].includes(status) + ) { + return null; + } + const tokenBudget = goal.tokenBudget === null || goal.tokenBudget === undefined + ? null + : nonNegativeInteger(goal.tokenBudget); + const tokensUsed = goal.tokensUsed === undefined + ? 0 + : nonNegativeInteger(goal.tokensUsed); + const timeUsedSeconds = goal.timeUsedSeconds === undefined + ? 0 + : nonNegativeInteger(goal.timeUsedSeconds); + const createdAt = goal.createdAt === undefined + ? 0 + : nonNegativeInteger(goal.createdAt); + const updatedAt = goal.updatedAt === undefined + ? 0 + : nonNegativeInteger(goal.updatedAt); + if ( + (goal.tokenBudget !== null && goal.tokenBudget !== undefined && tokenBudget === null) || + tokensUsed === null || + timeUsedSeconds === null || + createdAt === null || + updatedAt === null + ) return null; + return { + threadId, + objective, + status: status as HarnessThreadGoal["status"], + tokenBudget, + tokensUsed, + timeUsedSeconds, + createdAt, + updatedAt, + }; +} + +function threadStatus(value: unknown): string { + if (typeof value === "string") return value; + return text(record(value).type, "unknown"); +} + +export function codexThreadLineage(value: unknown): HarnessThreadLineageEntry { + const thread = record(value); + const source = record(thread.source); + const subAgent = source.subAgent ?? source.subagent; + const subAgentRecord = record(subAgent); + const spawn = record( + subAgentRecord.thread_spawn ?? subAgentRecord.threadSpawn, + ); + const parentThreadId = + text( + spawn.parent_thread_id ?? spawn.parentThreadId, + text(thread.forkedFromId), + ) || null; + return { + threadId: text(thread.id), + providerSessionId: text(thread.sessionId) || null, + parentThreadId, + depth: nonNegativeInteger(spawn.depth) ?? (parentThreadId === null ? 0 : 1), + nickname: + text( + thread.agentNickname, + text(spawn.agent_nickname ?? spawn.agentNickname), + ) || null, + role: + text(thread.agentRole, text(spawn.agent_role ?? spawn.agentRole)) || null, + status: threadStatus(thread.status), + }; +} + +export interface CodexNotificationBinding { + runId: string; + threadIds: readonly string[]; +} + +export interface BindableCodexNotification { + method: string; + params: Record; +} + +function supportedCodexNotificationMethod(method: string): boolean { + return ( + method === "turn/started" || + method === "turn/completed" || + method === "thread/started" || + method === "thread/status/changed" || + method === "thread/closed" || + method === "thread/goal/updated" || + method === "thread/goal/cleared" || + method === "serverRequest/resolved" || + method === "thread/tokenUsage/updated" || + method === "error" || + method === "warning" || + method === "configWarning" || + method === "guardianWarning" || + method === "deprecationNotice" || + method === "windows/worldWritableWarning" || + method === "hook/started" || + method === "hook/completed" || + method === "thread/compacted" || + method === "model/rerouted" || + method === "model/verification" || + method === "model/safetyBuffering/updated" || + method.startsWith("item/") || + method === "paperclip/workspaceChange/updated" || + method === "paperclip/runResult" || + method === "turn/diff/updated" || + method === "turn/plan/updated" + ); +} + +/** + * Admits only known notifications that explicitly name the active run or one + * of its known threads. A newly spawned child may bind through its parent. + */ +export function isBoundCodexNotification( + notification: BindableCodexNotification, + binding: CodexNotificationBinding, +): boolean { + if (!supportedCodexNotificationMethod(notification.method)) return false; + const params = record(notification.params); + const claimedRunId = text(params.runId, text(params.paperclipRunId)); + if (claimedRunId.length > 0 && claimedRunId !== binding.runId) return false; + + const allowedThreads = new Set(binding.threadIds); + const directThreadId = text(params.threadId, text(record(params.turn).threadId)); + if (directThreadId.length > 0) return allowedThreads.has(directThreadId); + + const thread = record(params.thread); + const threadId = text(thread.id); + if (threadId.length > 0 && allowedThreads.has(threadId)) return true; + const source = record(thread.source); + const subAgent = record(source.subAgent ?? source.subagent); + const spawn = record(subAgent.thread_spawn ?? subAgent.threadSpawn); + const parentThreadId = text(spawn.parent_thread_id ?? spawn.parentThreadId); + if (notification.method === "thread/started" && parentThreadId.length > 0) { + return allowedThreads.has(parentThreadId); + } + if (threadId.length > 0) return false; + + return false; +} + +export function codexWorkspaceRelativePath(value: unknown): string | null { + if (typeof value !== "string") return null; + const path = value.trim().replaceAll("\\", "/"); + if ( + path.length === 0 || + path.length > 1_024 || + path.startsWith("/") || + path.startsWith("//") || + /^[A-Za-z]:/u.test(path) || + path.split("/").some((part) => part === ".." || part.length === 0) + ) return null; + return path; +} + +export function boundedCodexWorkspaceStat(value: unknown): number | null { + return typeof value === "number" && Number.isSafeInteger(value) && value >= 0 + ? value + : null; +} + +export function safeCodexRequestResponse( + method: string, + action: "decline" | "cancel" = "decline", +): Record { + if (method === "item/permissions/requestApproval") { + return { permissions: {}, scope: "turn" }; + } + if (method === "mcpServer/elicitation/request") { + return { action, content: null, _meta: null }; + } + if ( + method === "item/tool/requestUserInput" || + method === "tool/requestUserInput" + ) { + return { answers: {} }; + } + if ( + method.includes("requestApproval") || + method === "execCommandApproval" || + method === "applyPatchApproval" + ) { + return { decision: action }; + } + return {}; +}