mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-06 10:48:12 +02:00
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 <noreply@paperclip.ing>
This commit is contained in:
1 parent
32573876d4
commit
0f8750627f
3 files changed
+165
-1
No files matched your search
@@ -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
|
||||
|
||||
@@ -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" });
|
||||
},
|
||||
);
|
||||
@@ -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,
|
||||
|
||||
Reference in new issue
Block a user