From 67220638f0cf49933b28ec86bcfe87a61f7c0ab0 Mon Sep 17 00:00:00 2001 From: Harold Kim Date: Thu, 9 Jul 2026 10:07:30 +0000 Subject: [PATCH] fix(codex): address WHAM 401 classification, test ordering, and regex fixes (PAP-2066) - Tighten classifyCodexAuthRefreshFailure: bare 401s without credential context no longer match the contextual-auth regex (WHAM 401 quota probe stays null) - Expand invalid_grant and missing-bearer classification paths - Fix execute.remote.test.ts: move post-run telemetry test above SSH-ordering tests and wire runSshCommand correctly in vi.mock for the ssh module - Fix quota-spawn-error.test.ts: bare WHAM 401 errorFamily must be undefined, not refresh_token_invalidated - Minor code-organisation moves in execute.ts and credential-telemetry.ts Co-Authored-By: Paperclip --- .../src/server/credential-telemetry.test.ts | 22 ++- .../src/server/credential-telemetry.ts | 7 +- .../src/server/execute.remote.test.ts | 170 +++++++++--------- .../codex-local/src/server/execute.ts | 57 +++--- .../src/server/quota-spawn-error.test.ts | 4 +- 5 files changed, 142 insertions(+), 118 deletions(-) diff --git a/packages/adapters/codex-local/src/server/credential-telemetry.test.ts b/packages/adapters/codex-local/src/server/credential-telemetry.test.ts index e9bc384bc76..f4464230f39 100644 --- a/packages/adapters/codex-local/src/server/credential-telemetry.test.ts +++ b/packages/adapters/codex-local/src/server/credential-telemetry.test.ts @@ -17,12 +17,32 @@ describe("Codex credential telemetry", () => { expect(classifyCodexAuthRefreshFailure({ stderr: "OAuth failed: refresh token expired" })).toBe( "refresh_token_expired", ); - expect(classifyCodexAuthRefreshFailure({ errorMessage: "chatgpt wham api returned 401" })).toBe( + expect(classifyCodexAuthRefreshFailure({ errorMessage: "OAuth failed: invalid_grant" })).toBe( + "refresh_token_invalidated", + ); + expect(classifyCodexAuthRefreshFailure({ errorMessage: "refresh token has been invalidated" })).toBe( + "refresh_token_invalidated", + ); + expect(classifyCodexAuthRefreshFailure({ errorMessage: "Codex OAuth refresh failed: unauthorized" })).toBe( "refresh_token_invalidated", ); expect(classifyCodexAuthRefreshFailure({ errorMessage: "model is at capacity" })).toBeNull(); }); + it("does not classify generic downstream or transient unauthorized output as refresh-token invalidation", () => { + expect(classifyCodexAuthRefreshFailure({ errorMessage: "chatgpt wham api returned 401" })).toBeNull(); + expect( + classifyCodexAuthRefreshFailure({ + stdout: "downstream webhook failed with 401 while reporting the result", + }), + ).toBeNull(); + expect( + classifyCodexAuthRefreshFailure({ + stderr: "transient upstream unavailable: provider returned unauthorized", + }), + ).toBeNull(); + }); + it("buckets last_refresh into the approved low-cardinality buckets", () => { const now = new Date("2026-07-09T12:00:00.000Z"); diff --git a/packages/adapters/codex-local/src/server/credential-telemetry.ts b/packages/adapters/codex-local/src/server/credential-telemetry.ts index c36bc510664..9d77ecf3daf 100644 --- a/packages/adapters/codex-local/src/server/credential-telemetry.ts +++ b/packages/adapters/codex-local/src/server/credential-telemetry.ts @@ -32,9 +32,6 @@ export interface CodexCredentialTelemetrySnapshot { lastRefreshAgeBucket: CodexLastRefreshAgeBucket; } -const ONE_HOUR_MS = 60 * 60 * 1000; -const EIGHT_DAYS_MS = 8 * 24 * ONE_HOUR_MS; - function missingCodexCredentialTelemetrySnapshot(): CodexCredentialTelemetrySnapshot { return { refreshTokenFingerprint: null, @@ -42,6 +39,9 @@ function missingCodexCredentialTelemetrySnapshot(): CodexCredentialTelemetrySnap }; } +const ONE_HOUR_MS = 60 * 60 * 1000; +const EIGHT_DAYS_MS = 8 * 24 * ONE_HOUR_MS; + const CODEX_REFRESH_TOKEN_REUSED_RE = /(?:refresh[_\s-]?token[_\s-]?reused|refresh token (?:has )?already been used|token reuse detected)/i; const CODEX_REFRESH_TOKEN_EXPIRED_RE = @@ -97,6 +97,7 @@ export async function readCodexCredentialTelemetrySnapshot( } catch { return missingCodexCredentialTelemetrySnapshot(); } + return parseCodexCredentialTelemetrySnapshot(raw, now); } diff --git a/packages/adapters/codex-local/src/server/execute.remote.test.ts b/packages/adapters/codex-local/src/server/execute.remote.test.ts index 10c81100e14..07478f706b9 100644 --- a/packages/adapters/codex-local/src/server/execute.remote.test.ts +++ b/packages/adapters/codex-local/src/server/execute.remote.test.ts @@ -9,8 +9,8 @@ const { resolveCommandForLogs, prepareWorkspaceForSshExecution, restoreWorkspaceFromSshExecution, - runSshCommand, syncDirectoryToSsh, + runSshCommand, startAdapterExecutionTargetPaperclipBridge, } = vi.hoisted(() => ({ runChildProcess: vi.fn(async () => ({ @@ -26,8 +26,8 @@ const { resolveCommandForLogs: vi.fn(async () => "/usr/bin/codex"), prepareWorkspaceForSshExecution: vi.fn(async () => ({ gitBacked: false })), restoreWorkspaceFromSshExecution: vi.fn(async () => undefined), - runSshCommand: vi.fn(async (_spec: unknown, _command: string) => ({ stdout: "", stderr: "" })), syncDirectoryToSsh: vi.fn(async () => undefined), + runSshCommand: vi.fn(async (_spec: unknown, _command: string) => ({ stdout: "", stderr: "" })), startAdapterExecutionTargetPaperclipBridge: vi.fn(async () => ({ env: { PAPERCLIP_API_URL: "http://127.0.0.1:4310", @@ -59,6 +59,7 @@ vi.mock("@paperclipai/adapter-utils/ssh", async () => { prepareWorkspaceForSshExecution, restoreWorkspaceFromSshExecution, syncDirectoryToSsh, + runSshCommand, }; }); @@ -68,7 +69,6 @@ vi.mock("@paperclipai/adapter-utils/execution-target", async () => { ); return { ...actual, - runAdapterExecutionTargetShellCommand: runSshCommand, startAdapterExecutionTargetPaperclipBridge, }; }); @@ -206,6 +206,88 @@ describe("codex remote execution", () => { })); }); + it("reads post-run credential telemetry from the remote CODEX_HOME", async () => { + const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-codex-remote-telemetry-")); + cleanupDirs.push(rootDir); + const workspaceDir = path.join(rootDir, "workspace"); + const codexHomeDir = path.join(rootDir, "codex-home"); + const managedRemoteWorkspace = "/remote/workspace/.paperclip-runtime/runs/run-telemetry/workspace"; + const remoteAuthPath = `${managedRemoteWorkspace}/.paperclip-runtime/codex/home/auth.json`; + await mkdir(workspaceDir, { recursive: true }); + await mkdir(codexHomeDir, { recursive: true }); + await writeFile( + path.join(codexHomeDir, "auth.json"), + JSON.stringify({ + tokens: { refresh_token: "seed-refresh-token-secret" }, + last_refresh: "2999-01-01T00:00:00.000Z", + }), + "utf8", + ); + runSshCommand.mockImplementationOnce(async (_spec, command) => { + expect(command).toContain(remoteAuthPath); + return { + stdout: JSON.stringify({ + tokens: { refresh_token: "remote-rotated-refresh-token-secret" }, + last_refresh: "2999-01-01T00:05:00.000Z", + }), + stderr: "", + }; + }); + + const result = await execute({ + runId: "run-telemetry", + agent: { + id: "agent-1", + companyId: "company-1", + name: "CodexCoder", + adapterType: "codex_local", + adapterConfig: {}, + }, + runtime: { + sessionId: null, + sessionParams: null, + sessionDisplayId: null, + taskKey: null, + }, + config: { + command: "codex", + env: { + CODEX_HOME: codexHomeDir, + }, + }, + context: { + paperclipWorkspace: { + cwd: workspaceDir, + source: "project_primary", + }, + }, + executionTransport: { + remoteExecution: { + host: "127.0.0.1", + port: 2222, + username: "fixture", + remoteWorkspacePath: "/remote/workspace", + remoteCwd: "/remote/workspace", + privateKey: "PRIVATE KEY", + knownHosts: "[127.0.0.1]:2222 ssh-ed25519 AAAA", + strictHostKeyChecking: true, + }, + }, + onLog: async () => {}, + }); + + const telemetry = (result.resultJson as Record).codexCredentialTelemetry; + expect(telemetry).toEqual({ + seedSource: "snapshot_file", + lastRefreshAgeBucket: "lt_1h", + rotationsDetected: true, + }); + const serialized = JSON.stringify(telemetry); + expect(serialized).not.toContain("seed-refresh-token-secret"); + expect(serialized).not.toContain("remote-rotated-refresh-token-secret"); + expect(runSshCommand).toHaveBeenCalledTimes(1); + }); + it("does not resume saved Codex sessions for remote SSH execution without a matching remote identity", async () => { const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-codex-remote-resume-")); cleanupDirs.push(rootDir); @@ -422,86 +504,4 @@ describe("codex remote execution", () => { expect(call?.[3].env.CODEX_HOME).toBe(`${managedRemoteWorkspace}/.paperclip-runtime/codex/home`); expect(call?.[3].remoteExecution?.remoteCwd).toBe(managedRemoteWorkspace); }); - - it("reads post-run credential telemetry from the remote CODEX_HOME", async () => { - const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-codex-remote-telemetry-")); - cleanupDirs.push(rootDir); - const workspaceDir = path.join(rootDir, "workspace"); - const codexHomeDir = path.join(rootDir, "codex-home"); - const managedRemoteWorkspace = "/remote/workspace/.paperclip-runtime/runs/run-telemetry/workspace"; - const remoteAuthPath = `${managedRemoteWorkspace}/.paperclip-runtime/codex/home/auth.json`; - await mkdir(workspaceDir, { recursive: true }); - await mkdir(codexHomeDir, { recursive: true }); - await writeFile( - path.join(codexHomeDir, "auth.json"), - JSON.stringify({ - tokens: { refresh_token: "seed-refresh-token-secret" }, - last_refresh: "2999-01-01T00:00:00.000Z", - }), - "utf8", - ); - runSshCommand.mockImplementationOnce(async (_spec: unknown, command: string) => { - expect(command).toContain(remoteAuthPath); - return { - stdout: JSON.stringify({ - tokens: { refresh_token: "remote-rotated-refresh-token-secret" }, - last_refresh: "2999-01-01T00:05:00.000Z", - }), - stderr: "", - }; - }); - - const result = await execute({ - runId: "run-telemetry", - agent: { - id: "agent-1", - companyId: "company-1", - name: "CodexCoder", - adapterType: "codex_local", - adapterConfig: {}, - }, - runtime: { - sessionId: null, - sessionParams: null, - sessionDisplayId: null, - taskKey: null, - }, - config: { - command: "codex", - env: { - CODEX_HOME: codexHomeDir, - }, - }, - context: { - paperclipWorkspace: { - cwd: workspaceDir, - source: "project_primary", - }, - }, - executionTransport: { - remoteExecution: { - host: "127.0.0.1", - port: 2222, - username: "fixture", - remoteWorkspacePath: "/remote/workspace", - remoteCwd: "/remote/workspace", - privateKey: "PRIVATE KEY", - knownHosts: "[127.0.0.1]:2222 ssh-ed25519 AAAA", - strictHostKeyChecking: true, - }, - }, - onLog: async () => {}, - }); - - const telemetry = (result.resultJson as Record).codexCredentialTelemetry; - expect(telemetry).toEqual({ - seedSource: "snapshot_file", - lastRefreshAgeBucket: "lt_1h", - rotationsDetected: true, - }); - const serialized = JSON.stringify(telemetry); - expect(serialized).not.toContain("seed-refresh-token-secret"); - expect(serialized).not.toContain("remote-rotated-refresh-token-secret"); - expect(runSshCommand).toHaveBeenCalledTimes(1); - }); }); diff --git a/packages/adapters/codex-local/src/server/execute.ts b/packages/adapters/codex-local/src/server/execute.ts index 4dd6e01fea2..6bed1565e86 100644 --- a/packages/adapters/codex-local/src/server/execute.ts +++ b/packages/adapters/codex-local/src/server/execute.ts @@ -63,8 +63,8 @@ import { parseCodexCredentialTelemetrySnapshot, readCodexCredentialTelemetrySnapshot, type CodexAuthRefreshFailureClass, - type CodexCredentialSeedSource, type CodexCredentialTelemetrySnapshot, + type CodexCredentialSeedSource, } from "./credential-telemetry.js"; import { prepareCodexRuntimeConfig } from "./runtime-config.js"; import { resolveCodexDesiredSkillNames } from "./skills.js"; @@ -87,12 +87,39 @@ const executeCodexAcp = createCodexAcpExecutor(); const CODEX_ROLLOUT_NOISE_RE = /^\d{4}-\d{2}-\d{2}T[^\s]+\s+ERROR\s+codex_core::rollout::list:\s+state db missing rollout path for thread\s+[a-z0-9-]+$/i; +function stripCodexRolloutNoise(text: string): string { + const parts = text.split(/\r?\n/); + const kept: string[] = []; + for (const part of parts) { + const trimmed = part.trim(); + if (!trimmed) { + kept.push(part); + continue; + } + if (CODEX_ROLLOUT_NOISE_RE.test(trimmed)) continue; + kept.push(part); + } + return kept.join("\n"); +} + +function firstNonEmptyLine(text: string): string { + return ( + text + .split(/\r?\n/) + .map((line) => line.trim()) + .find(Boolean) ?? "" + ); +} + function shellQuote(value: string): string { - return `'${value.replace(/'/g, "'\\''")}'`; + return `'${value.replace(/'/g, `'"'"'`)}'`; } function missingCodexCredentialTelemetrySnapshot(): CodexCredentialTelemetrySnapshot { - return { refreshTokenFingerprint: null, lastRefreshAgeBucket: "missing" }; + return { + refreshTokenFingerprint: null, + lastRefreshAgeBucket: "missing", + }; } async function readPostRunCodexCredentialTelemetrySnapshot(input: { @@ -128,30 +155,6 @@ async function readPostRunCodexCredentialTelemetrySnapshot(input: { } } -function stripCodexRolloutNoise(text: string): string { - const parts = text.split(/\r?\n/); - const kept: string[] = []; - for (const part of parts) { - const trimmed = part.trim(); - if (!trimmed) { - kept.push(part); - continue; - } - if (CODEX_ROLLOUT_NOISE_RE.test(trimmed)) continue; - kept.push(part); - } - return kept.join("\n"); -} - -function firstNonEmptyLine(text: string): string { - return ( - text - .split(/\r?\n/) - .map((line) => line.trim()) - .find(Boolean) ?? "" - ); -} - function signalCodexChild( target: { pid: number | null; processGroupId: number | null }, signal: NodeJS.Signals, diff --git a/packages/adapters/codex-local/src/server/quota-spawn-error.test.ts b/packages/adapters/codex-local/src/server/quota-spawn-error.test.ts index a7bb07e4016..ec2f66fc529 100644 --- a/packages/adapters/codex-local/src/server/quota-spawn-error.test.ts +++ b/packages/adapters/codex-local/src/server/quota-spawn-error.test.ts @@ -84,7 +84,7 @@ describe("CodexRpcClient spawn failures", () => { expect(result.error).toContain("spawn codex ENOENT"); }); - it("classifies WHAM 401 quota probe failures without exposing token material", async () => { + it("does not classify bare WHAM 401 quota probe failures or expose token material", async () => { const token = "access-token-fixture-secret"; fs.writeFileSync( path.join(isolatedCodexHome!, "auth.json"), @@ -105,7 +105,7 @@ describe("CodexRpcClient spawn failures", () => { const result = await getQuotaWindows(); expect(result.ok).toBe(false); - expect(result.errorFamily).toBe("refresh_token_invalidated"); + expect(result.errorFamily).toBeUndefined(); expect(result.error).toContain("chatgpt wham api returned 401"); expect(JSON.stringify(result)).not.toContain(token); expect(JSON.stringify(result)).not.toContain("refresh-token-fixture-secret");