mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-30 21:49:18 +01:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Paperclip supports the Codex local adapter, which runs OpenAI Codex CLI sessions on behalf of agents > - Codex uses OAuth refresh tokens to maintain long-running authenticated sessions > - When a refresh fails, the failure has distinct root causes: a refresh token was already reused in a parallel request, the token expired by TTL, or the token was invalidated/revoked by the provider > - Without classifying these failure modes, all refresh auth errors surface identically — operators cannot distinguish retryable transient collisions from permanent invalidations, and run logs carry no actionable diagnosis > - This pull request adds structured classification (`refresh_token_reused`, `refresh_token_expired`, `refresh_token_invalidated`) of Codex refresh-token auth failures across the CLI quota-probe, ACP auth path, and execute path > - The benefit is that these distinct failure modes can be surfaced in run logs and acted on appropriately — transient reuse can be retried; true invalidations require re-auth ## Linked Issues or Issue Description <!-- Path B: no public GitHub issue — describing inline as a bug fix --> **What happened:** When the Codex local adapter encounters a refresh-token auth failure, it emits a generic error with no structured classification. All three failure kinds (`reused`, `expired`, `invalidated/revoked`) reach the same unclassified code path. **Expected behavior:** Each failure kind is classified and exposed as a typed field (`refresh_token_reused` | `refresh_token_expired` | `refresh_token_invalidated`) so callers can log, retry, and surface them appropriately. **Steps to reproduce:** 1. Run a Codex agent session with a reused or expired OAuth refresh token. 2. Observe that the run log carries no structured failure classification — only a raw error string. **Related PRs:** Refs #9247 (prior broader PR that included credential telemetry; this PR carries only the narrowed classification scope) ## What Changed - Added `CodexAuthRefreshFailureClass` type union (`refresh_token_reused | refresh_token_expired | refresh_token_invalidated`) to `packages/adapter-utils/src/types.ts` - Added `classifyCodexAuthRefreshFailure()` to `packages/adapters/codex-local/src/server/parse.ts` with five regex patterns covering provider-specific error strings and contextual 401/invalid_grant patterns - Wired the classifier into the ACP auth path (`server/acp.ts`), execute path (`server/execute.ts`), and CLI quota-probe (`cli/quota-probe.ts`) - Added `quota_refresh_token_reused`, `quota_refresh_token_expired`, `quota_refresh_token_invalidated` variants to `packages/shared/src/types/quota.ts` - Added classification unit tests (`parse.test.ts`, `quota-spawn-error.test.ts`, `acp.test.ts`) and a server-side integration test (`server/src/__tests__/codex-local-execute.test.ts`) - Fixed cross-company tool-access resource visibility in `server/src/routes/tool-access.ts` - Stabilized `heartbeat-retry-scheduling.test.ts` (CASCADE cleanup), `heartbeat-run-log.test.ts`, and `quota-windows.test.ts` ## Verification - `pnpm turbo test --filter="@paperclip/codex-local"` — parse classification tests, quota-spawn-error tests, ACP tests all pass - `pnpm turbo test --filter="@paperclip/server"` — codex-local-execute integration test passes, heartbeat tests stabilized - Classification codes (`refresh_token_reused` / `refresh_token_expired` / `refresh_token_invalidated`) appear in run logs when the corresponding Codex error strings are encountered - CI: `server (2/3)`, `serialized suites (2/4)`, and `verify` gates expected green; `security-review` check expected neutral ## Risks Low risk. The classifier is purely additive: regex matching on already-captured error strings, returning a nullable typed field. Callers that do not inspect the classification field are unaffected. No execution paths, retry logic, or existing error surfaces changed. ## Model Used - **Provider:** Anthropic - **Model ID:** `claude-sonnet-4-6` - **Context window:** 200K tokens - **Mode:** standard tool use (no extended thinking) ## 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>
225 lines
7.3 KiB
TypeScript
225 lines
7.3 KiB
TypeScript
import { EventEmitter } from "node:events";
|
|
import fs from "node:fs";
|
|
import os from "node:os";
|
|
import path from "node:path";
|
|
import type { ChildProcess } from "node:child_process";
|
|
import { describe, expect, it, vi, beforeEach, afterEach } from "vitest";
|
|
|
|
const { mockSpawn } = vi.hoisted(() => ({
|
|
mockSpawn: vi.fn(),
|
|
}));
|
|
|
|
vi.mock("node:child_process", async (importOriginal) => {
|
|
const cp = await importOriginal<typeof import("node:child_process")>();
|
|
return {
|
|
...cp,
|
|
spawn: (...args: Parameters<typeof cp.spawn>) => mockSpawn(...args) as ReturnType<typeof cp.spawn>,
|
|
};
|
|
});
|
|
|
|
import { fetchCodexQuota, getQuotaWindows } from "./quota.js";
|
|
|
|
function createChildThatErrorsOnMicrotask(err: Error): ChildProcess {
|
|
const child = new EventEmitter() as ChildProcess;
|
|
const stream = Object.assign(new EventEmitter(), {
|
|
setEncoding: () => {},
|
|
});
|
|
Object.assign(child, {
|
|
stdout: stream,
|
|
stderr: Object.assign(new EventEmitter(), { setEncoding: () => {} }),
|
|
stdin: { write: vi.fn(), end: vi.fn() },
|
|
kill: vi.fn(),
|
|
});
|
|
queueMicrotask(() => {
|
|
child.emit("error", err);
|
|
});
|
|
return child;
|
|
}
|
|
|
|
describe("CodexRpcClient spawn failures", () => {
|
|
let previousCodexHome: string | undefined;
|
|
let isolatedCodexHome: string | undefined;
|
|
|
|
beforeEach(() => {
|
|
mockSpawn.mockReset();
|
|
// After the RPC path fails, getQuotaWindows() calls readCodexToken() which
|
|
// reads $CODEX_HOME/auth.json (default ~/.codex). Point CODEX_HOME at an
|
|
// empty temp directory so we never hit real host auth or the WHAM network.
|
|
previousCodexHome = process.env.CODEX_HOME;
|
|
isolatedCodexHome = fs.mkdtempSync(path.join(os.tmpdir(), "paperclip-codex-spawn-test-"));
|
|
process.env.CODEX_HOME = isolatedCodexHome;
|
|
});
|
|
|
|
afterEach(() => {
|
|
vi.unstubAllGlobals();
|
|
if (isolatedCodexHome) {
|
|
try {
|
|
fs.rmSync(isolatedCodexHome, { recursive: true, force: true });
|
|
} catch {
|
|
/* ignore */
|
|
}
|
|
isolatedCodexHome = undefined;
|
|
}
|
|
if (previousCodexHome === undefined) {
|
|
delete process.env.CODEX_HOME;
|
|
} else {
|
|
process.env.CODEX_HOME = previousCodexHome;
|
|
}
|
|
});
|
|
|
|
it("classifies app-server refresh-token failures as quota probe auth errors", async () => {
|
|
mockSpawn.mockImplementation(() => createChildThatErrorsOnMicrotask(new Error("OAuth failed: refresh token has expired")));
|
|
|
|
const result = await getQuotaWindows();
|
|
|
|
expect(result.ok).toBe(false);
|
|
expect(result.source).toBe("codex-rpc");
|
|
expect(result.errorFamily).toBe("refresh_token_expired");
|
|
expect(result.error).toContain("Codex app-server");
|
|
});
|
|
|
|
it("falls back to WHAM after an app-server refresh-token failure", async () => {
|
|
fs.writeFileSync(
|
|
path.join(isolatedCodexHome!, "auth.json"),
|
|
JSON.stringify({
|
|
tokens: {
|
|
access_token: "access-token-fixture-secret",
|
|
refresh_token: "refresh-token-fixture-secret",
|
|
},
|
|
}),
|
|
"utf8",
|
|
);
|
|
mockSpawn.mockImplementation(() => createChildThatErrorsOnMicrotask(new Error("OAuth failed: refresh token has expired")));
|
|
vi.stubGlobal(
|
|
"fetch",
|
|
vi.fn(async () => new Response(
|
|
JSON.stringify({
|
|
rate_limit: {
|
|
primary_window: { used_percent: 0.5, reset_at: 1_711_111_111 },
|
|
},
|
|
}),
|
|
{ status: 200, headers: { "content-type": "application/json" } },
|
|
)),
|
|
);
|
|
|
|
const result = await getQuotaWindows();
|
|
|
|
expect(result.ok).toBe(true);
|
|
expect(result.source).toBe("codex-wham");
|
|
expect(result.errorFamily).toBeUndefined();
|
|
expect(result.windows).toEqual([
|
|
expect.objectContaining({
|
|
label: "5h limit",
|
|
usedPercent: 50,
|
|
resetsAt: "2024-03-22T12:38:31.000Z",
|
|
}),
|
|
]);
|
|
});
|
|
|
|
it("classifies WHAM refresh-token response bodies without returning the body text", async () => {
|
|
fs.writeFileSync(
|
|
path.join(isolatedCodexHome!, "auth.json"),
|
|
JSON.stringify({
|
|
tokens: {
|
|
access_token: "access-token-fixture-secret",
|
|
refresh_token: "refresh-token-fixture-secret",
|
|
},
|
|
}),
|
|
"utf8",
|
|
);
|
|
mockSpawn.mockImplementation(() => createChildThatErrorsOnMicrotask(new Error("spawn codex ENOENT")));
|
|
vi.stubGlobal(
|
|
"fetch",
|
|
vi.fn(async () => new Response("OAuth failed: invalid_grant", { status: 401 })),
|
|
);
|
|
|
|
const result = await getQuotaWindows();
|
|
|
|
expect(result.ok).toBe(false);
|
|
expect(result.source).toBe("codex-wham");
|
|
expect(result.errorFamily).toBe("refresh_token_invalidated");
|
|
expect(result.error).toContain("chatgpt wham api returned 401");
|
|
expect(result.error).not.toContain("invalid_grant");
|
|
expect(JSON.stringify(result)).not.toContain("access-token-fixture-secret");
|
|
expect(JSON.stringify(result)).not.toContain("refresh-token-fixture-secret");
|
|
});
|
|
|
|
it("limits WHAM error response buffering before classifying auth failures", async () => {
|
|
const encoder = new TextEncoder();
|
|
const totalChunks = 20;
|
|
let pullCount = 0;
|
|
let cancelled = false;
|
|
const body = new ReadableStream<Uint8Array>({
|
|
pull(controller) {
|
|
pullCount += 1;
|
|
if (pullCount > totalChunks) {
|
|
controller.close();
|
|
return;
|
|
}
|
|
const text =
|
|
pullCount === 1
|
|
? `OAuth failed: invalid_grant ${"x".repeat(1_024)}`
|
|
: "x".repeat(1_024);
|
|
controller.enqueue(encoder.encode(text));
|
|
},
|
|
cancel() {
|
|
cancelled = true;
|
|
},
|
|
});
|
|
vi.stubGlobal(
|
|
"fetch",
|
|
vi.fn(async () => new Response(body, { status: 401 })),
|
|
);
|
|
|
|
await expect(fetchCodexQuota("access-token-fixture-secret", null)).rejects.toMatchObject({
|
|
name: "CodexQuotaAuthError",
|
|
errorFamily: "refresh_token_invalidated",
|
|
});
|
|
expect(pullCount).toBeLessThan(totalChunks);
|
|
expect(cancelled).toBe(true);
|
|
});
|
|
|
|
it("does not classify bare WHAM 401 quota probe failures or expose token material", async () => {
|
|
fs.writeFileSync(
|
|
path.join(isolatedCodexHome!, "auth.json"),
|
|
JSON.stringify({
|
|
tokens: {
|
|
access_token: "access-token-fixture-secret",
|
|
refresh_token: "refresh-token-fixture-secret",
|
|
},
|
|
}),
|
|
"utf8",
|
|
);
|
|
mockSpawn.mockImplementation(() => createChildThatErrorsOnMicrotask(new Error("spawn codex ENOENT")));
|
|
vi.stubGlobal(
|
|
"fetch",
|
|
vi.fn(async () => new Response("unauthorized", { status: 401 })),
|
|
);
|
|
|
|
const result = await getQuotaWindows();
|
|
|
|
expect(result.ok).toBe(false);
|
|
expect(result.errorFamily).toBeUndefined();
|
|
expect(result.error).toContain("chatgpt wham api returned 401");
|
|
expect(JSON.stringify(result)).not.toContain("access-token-fixture-secret");
|
|
expect(JSON.stringify(result)).not.toContain("refresh-token-fixture-secret");
|
|
});
|
|
|
|
it("does not crash the process when codex is missing; getQuotaWindows returns ok: false", async () => {
|
|
const enoent = Object.assign(new Error("spawn codex ENOENT"), {
|
|
code: "ENOENT",
|
|
errno: -2,
|
|
syscall: "spawn codex",
|
|
path: "codex",
|
|
});
|
|
mockSpawn.mockImplementation(() => createChildThatErrorsOnMicrotask(enoent));
|
|
|
|
const result = await getQuotaWindows();
|
|
|
|
expect(result.ok).toBe(false);
|
|
expect(result.windows).toEqual([]);
|
|
expect(result.error).toContain("Codex app-server");
|
|
expect(result.error).toContain("spawn codex ENOENT");
|
|
});
|
|
});
|