From 0f8750627f11d855552abce9a837d7f3b67c9ddf Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Thu, 24 Sep 2026 10:03:19 -0700 Subject: [PATCH] fix(codex): preserve typed ACP quota classification and reset time (#13945) ## Thinking Path > - Paperclip runs agents and recovers failed tasks. > - Codex ACP can report usage exhaustion as a typed terminal failure. > - The shared engine removes provider text before it stores the result. > - Codex did not use the existing terminal classifier hook, so a quota failure became a generic turn failure. > - This change classifies explicit usage exhaustion before that text is removed. > - Recovery can then use quota backoff and the supported reset clock. ## Linked Issues or Issue Description **What happened?** A Codex ACP `limit` failure with explicit usage-exhaustion text produced `acpx_turn_failed`. Recovery could schedule the ordinary short retries because the quota classification and reset time were lost. **What did you expect to happen?** Keep the failure visible and use the existing provider-quota wait. Preserve a supported reset timestamp without storing provider text. **Steps to reproduce** Return a terminal ACP session failure with category `limit` and title `You've hit your usage limit for GPT-5. Switch to another model now, or try again at 4:30 PM (America/Chicago).` The real child-process regression tests exercise both pinned ACPX versions in persistent and oneshot modes. **Paperclip version** Base commit: `32573876d4`. Related work: #13651 and #13831 provide the shared hook and Claude classification. #11854 includes quota handling as part of optional credential rotation, but reads the already-sanitized result; this patch handles the typed terminal boundary without adding rotation. #13549 reads recovery text after this boundary and cannot recover discarded provider text. #9011 concerns the Codex CLI backoff. This change leaves those other mechanisms in place. ## What Changed - Register a Codex terminal-failure classifier with the existing ACP engine hook. - Recognize explicit usage exhaustion only in a typed `limit` failure. - Reuse the Codex reset-time parser and existing provider-quota recovery fields. - Leave context, turn, rate, budget, storage-capacity, and unknown failures on their existing paths. - Test real ACP children, both dependency patches, privacy, and recovery classification. Document the boundary. ## Verification - 88 focused tests passed across Codex ACP, parsing, and server recovery classification. - An initial cross-adapter run passed 53 tests, including the existing Claude quota suite. - Removing only the classifier registration makes five new integration tests fail; restoring it passes all 19 new tests. - `pnpm -r typecheck` and `pnpm build` passed. - Full Linux CI passed on `b10d60e002`, including all unit/integration shards, browser shards, typecheck/build, runner checks, and release/package gates. - The duplicate local `pnpm test:run` reported three skill-cache failures and two runner-suite failures. It is not claimed as a full local pass. - The three skill-cache failures reproduce in a clean worktree at the unchanged base commit (3 failed, 107 passed, 3 skipped across the skills and runner files). The cache rename reports `EACCES` on macOS. - An isolated runner-suite run reports two embedded PostgreSQL startup failures before its assertions (37 tests pass). No runner source is changed; the Linux CI runner and server suites passed. - Greptile reviewed the current head at 5/5 with no actionable findings or unresolved threads. ## Risks - Explicit usage-exhaustion failures now wait for quota recovery instead of short generic retries. - Unknown wording retains the existing behavior. The generic historical terminal-limit message alone cannot establish quota exhaustion. - Reset parsing keeps the existing Codex clock formats. A missing or unsupported reset uses the existing quota backoff. - No schema, credential, UI, dependency, or deployment changes. Provider text stays in memory and is absent from results and logs. ## Model Used OpenAI GPT-6 via Codex. Exact runtime model variant and context-window size were not exposed. Used reasoning, repository inspection, editing, and local tests. ## 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 - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: Paperclip --- doc/connections/AI-CONNECTIONS.md | 9 ++ .../codex-local/src/server/acp.quota.test.ts | 132 ++++++++++++++++++ .../adapters/codex-local/src/server/acp.ts | 25 +++- 3 files changed, 165 insertions(+), 1 deletion(-) create mode 100644 packages/adapters/codex-local/src/server/acp.quota.test.ts diff --git a/doc/connections/AI-CONNECTIONS.md b/doc/connections/AI-CONNECTIONS.md index f80623f5f0..6c55738c5e 100644 --- a/doc/connections/AI-CONNECTIONS.md +++ b/doc/connections/AI-CONNECTIONS.md @@ -99,6 +99,15 @@ Sending `environmentId: null` tests a change back to the instance default. ## Runtime isolation +Codex ACP terminal failures with category `limit` and explicit usage-exhaustion +wording enter provider-quota recovery. A supported reset clock uses the existing +Codex parser; when none is available, recovery uses its existing quota backoff. +Context, turn, rate, storage-capacity and configured-budget limits retain their +existing handling. The adapter inspects bounded provider text only in memory +and retains recovery labels and a parsed timestamp, without copying the text to +run results or logs. A historical generic terminal-limit message alone does not +establish quota exhaustion. + `prepareManagedAiRuntime` is shared by runs, environment tests, and adoption. Claude ACP validates working directories on the selected execution target. A sandbox directory does not need to exist on the Paperclip server. When the agent diff --git a/packages/adapters/codex-local/src/server/acp.quota.test.ts b/packages/adapters/codex-local/src/server/acp.quota.test.ts new file mode 100644 index 0000000000..67b228664a --- /dev/null +++ b/packages/adapters/codex-local/src/server/acp.quota.test.ts @@ -0,0 +1,132 @@ +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { createRequire } from "node:module"; +import { fileURLToPath } from "node:url"; +import { afterEach, expect, it } from "vitest"; +import { classifyCodexTerminalSessionFailure, createCodexAcpExecutor } from "./acp.js"; +import type { AcpxEngineExecutorOptions } from "@paperclipai/adapter-utils/acpx-engine/execute"; + +const repoRoot = fileURLToPath(new URL("../../../../..", import.meta.url)); +const fixture = path.join(repoRoot, "scripts/mcp-fixtures/servers/acp-echo-agent.mjs"); +const roots: string[] = []; +const now = new Date("2026-07-15T20:00:00.000Z"); +// Exercise both pinned dependency patches through the same real ACP child. +const runnerRequire = createRequire(path.join(repoRoot, "packages/paperclip-runner/package.json")); +const runnerAcpx = await import(runnerRequire.resolve("acpx/runtime")); + +afterEach(async () => { + await Promise.all(roots.splice(0).map((root) => fs.rm(root, { recursive: true, force: true }))); +}); + +async function executeFailure( + title: string, + category = "limit", + mode = "oneshot", + createRuntime?: AcpxEngineExecutorOptions["createRuntime"], +) { + const root = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-codex-acp-quota-")); + roots.push(root); + const logs: string[] = []; + const execute = createCodexAcpExecutor({ now: () => now.getTime(), createRuntime }); + const result = await execute({ + runId: "typed-quota", + agent: { id: "quota-agent", companyId: "quota-company" }, + runtime: {}, + config: { + agentCommand: `${JSON.stringify(process.execPath.replaceAll("\\", "/"))} ${JSON.stringify(fixture.replaceAll("\\", "/"))}`, + mode, + warmHandleIdleMs: 0, + cwd: repoRoot, + stateDir: path.join(root, "state"), + env: { + CODEX_HOME: path.join(root, "codex-home"), + PAPERCLIP_ACPX_TYPED_FAILURE_CANARY: title, + PAPERCLIP_ACPX_TYPED_FAILURE_CATEGORY: category, + }, + }, + context: {}, + onLog: async (_stream: string, text: string) => logs.push(text), + onMeta: async () => {}, + } as never); + return { result, logs: logs.join("\n") }; +} + +it.each([ + ["0.12.0", "oneshot"], + ["0.12.0", "persistent"], + ["0.13.1", "oneshot"], + ["0.13.1", "persistent"], +])("waits for a typed Codex quota reset with ACPX %s in %s mode without exposing provider text", async (version, mode) => { + const title = "You've hit your usage limit for GPT-5. Switch to another model now, or try again at 4:30 PM (America/Chicago)."; + const { result, logs } = await executeFailure( + title, "limit", mode, version === "0.13.1" ? runnerAcpx.createAcpRuntime : undefined, + ); + expect(result).toMatchObject({ + exitCode: 1, + errorCode: "provider_quota", + errorFamily: "provider_quota", + retryNotBefore: "2026-07-15T21:30:00.000Z", + resultJson: { + errorFamily: "provider_quota", + retryNotBefore: "2026-07-15T21:30:00.000Z", + providerQuotaRetryNotBefore: "2026-07-15T21:30:00.000Z", + }, + }); + expect(JSON.stringify(result)).not.toContain(title); + expect(logs).not.toContain(title); +}); + +it("classifies quota without a reset time for the existing recovery backoff", async () => { + const title = "You've hit your usage limit. Visit https://example.invalid/usage for account details."; + const { result, logs } = await executeFailure(title); + expect(result).toMatchObject({ errorCode: "provider_quota", errorFamily: "provider_quota" }); + expect(result.retryNotBefore).toBeUndefined(); + expect(JSON.stringify(result)).not.toContain(title); + expect(logs).not.toContain(title); +}); + +it.each([ + ["Context window limit exceeded", "limit"], + ["Maximum number of turns reached", "limit"], + ["Configured budget limit reached", "limit"], + ["Rate limit exceeded; retry later", "limit"], + ["You've hit your usage limit", "request"], + ["Context window capacity limit reached", "limit"], + ["Workspace storage capacity limit reached", "limit"], + ["The account has available quota", "limit"], + ["The worker connection closed", "connection"], +])("keeps a non-quota typed failure out of quota recovery: %s", async (title, category) => { + const { result, logs } = await executeFailure(title, category); + expect(result).toMatchObject({ exitCode: 1, errorCode: "acpx_turn_failed" }); + expect(result.errorFamily).not.toBe("provider_quota"); + expect(result.retryNotBefore).toBeUndefined(); + expect(JSON.stringify(result)).not.toContain(title); + expect(logs).not.toContain(title); +}); + +it("does not infer quota from the historical generic terminal-limit error", () => { + expect(classifyCodexTerminalSessionFailure({ + category: "limit", + title: "ACP agent reported a terminal limit failure.", + }, now)).toBeNull(); +}); + +it("reads usage exhaustion and its reset clock from terminal details", () => { + expect(classifyCodexTerminalSessionFailure({ + category: "limit", + title: "Codex could not complete the turn", + details: "You've hit your usage limit for GPT-5. Switch to another model now, or try again at 4:30 PM (America/Chicago).", + }, now)).toEqual({ + errorCode: "provider_quota", + errorFamily: "provider_quota", + retryNotBefore: "2026-07-15T21:30:00.000Z", + }); +}); + +it.each(["Usage limit reached", "Usage limit exceeded", "You’ve hit your usage limit"])( + "classifies explicit usage exhaustion without a reset clock: %s", (title) => { + expect(classifyCodexTerminalSessionFailure({ category: "limit", title }, now)) + .toEqual({ errorCode: "provider_quota", errorFamily: "provider_quota" }); + }, +); diff --git a/packages/adapters/codex-local/src/server/acp.ts b/packages/adapters/codex-local/src/server/acp.ts index ad13d427c3..e9b19def48 100644 --- a/packages/adapters/codex-local/src/server/acp.ts +++ b/packages/adapters/codex-local/src/server/acp.ts @@ -29,6 +29,8 @@ import type { AcpxEngineExecutorOptions, AcpxRemoteManagedHomeContext, AcpxRemoteManagedHomeResult, + AcpxTerminalFailureClassification, + AcpxTerminalSessionFailure, } from "@paperclipai/adapter-utils/acpx-engine/execute"; import { asNumber, @@ -38,7 +40,7 @@ import { } from "@paperclipai/adapter-utils/server-utils"; import { createWorkspaceRestoreTeardown } from "@paperclipai/adapter-utils/workspace-restore-teardown"; import { normalizeCodexModel } from "../index.js"; -import { classifyCodexAuthRefreshFailure } from "./parse.js"; +import { classifyCodexAuthRefreshFailure, extractCodexRetryNotBefore } from "./parse.js"; import { copyBackCodexAuth } from "./codex-auth-copyback.js"; import { buildCodexAuthInboundProvision } from "./codex-auth-merge-scripts.js"; import { @@ -257,10 +259,31 @@ async function prepareCodexRemoteManagedHome( }; } +export function classifyCodexTerminalSessionFailure( + failure: AcpxTerminalSessionFailure, + now: Date, +): AcpxTerminalFailureClassification | null { + // ACP's `limit` also covers context, turn, rate and configured budget limits. + // Require explicit usage exhaustion; the CLI's broader capacity matcher would + // also match a context/storage capacity limit and defer the wrong failure. + if (failure.category !== "limit") return null; + const surface = { errorMessage: [failure.title, failure.details].filter(Boolean).join("\n") }; + if (!/\b(?:you(?:'|’)ve hit your usage limit|usage limit (?:reached|exceeded))\b/i.test(surface.errorMessage)) { + return null; + } + const retryNotBefore = extractCodexRetryNotBefore(surface, now)?.toISOString(); + return { + errorCode: "provider_quota", + errorFamily: "provider_quota", + ...(retryNotBefore ? { retryNotBefore } : {}), + }; +} + function withCodexAcpDefaults(options: CodexAcpExecutorOptions): AcpxEngineExecutorOptions { return { resolveBillingIdentity: resolveCodexAcpBillingIdentity, prepareRemoteManagedHome: prepareCodexRemoteManagedHome, + classifyTerminalSessionFailure: classifyCodexTerminalSessionFailure, ...options, adapterType: "codex_local", moduleDir,