From d3b534058312b897b2920387863597370bbb4f2f Mon Sep 17 00:00:00 2001 From: David Blass Date: Fri, 22 May 2026 16:48:25 +0000 Subject: [PATCH] =?UTF-8?q?fix:=20audit=20batch=20=E2=80=94=20MCP=20timeou?= =?UTF-8?q?ts,=20entryPost,=20vip=5Faudit=20404s,=20and=206=20more=20(#824?= =?UTF-8?q?)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: 9 unaddressed log-audit / run-audit findings Co-authored-by: Cursor #815 entryPost stdlib-only imports; #823 MCP timeoutMs on checkout_pr/shell; #816 FREE_FALLBACK → opencode/big-pickle; #822 chunk GraphQL nodes ≤100; #817/#821 vip_audit 404 skip paths; #813 longer serializable retries; #818 run-context handler-entered log; #805 audit severity template. * fix: update footer test for big-pickle fallback slug Co-authored-by: Cursor * fix: anneal round 1 — ghaCore getState casing, post-hook timeout Match @actions/core STATE_ key semantics (no uppercasing), cap postApiFetch at 30s, trim serializable retries to stay under GitHub's 10s webhook window, log Clerk failures in getUserTokenByGithubLogin. Co-authored-by: Cursor * revert: drop run-context handler log (#818 deferred) The #692 client-side fix is already on main; residual SyntaxError hits are old action pins. Per-request log added noise without fixing anything. Co-authored-by: Cursor * document per-issue Closes syntax for audit PRs GitHub only auto-closes the first issue when numbers are comma-separated; /audits and AGENTS.md now require Closes before each issue number. Co-authored-by: Cursor * fix: anneal round 2 — outreach privacy, alert resilience, vertex cleanup Filter private repos from VIP authority output, harden console alert lines against DB failures, drop spoofable changesets body check, and unset GOOGLE_APPLICATION_CREDENTIALS after vertex credential cleanup. Co-authored-by: Cursor * refactor: drop codexHome re-export of detectCodexRefresh Import detectCodexRefresh directly from codexRefreshDetect.ts everywhere; rename the unit test file to match. codexHome.ts stays install-only. Co-authored-by: Cursor * fix: drop deprecated minimax-m2.5-free; add paid minimax-m2.5 Remove the deprecated free MiniMax promo from the catalog, docs, and tests. BYOK fallback and picker copy stay on opencode/big-pickle. Add opencode/minimax-m2.5 and openrouter/minimax-m2.5 for Zen BYOK and Router. Pin #816 regressions with freeFallbackCatalog and runErrorRenderer unit tests. Co-authored-by: Cursor * fix: hidden minimax-m2.5-free fallback for stored slugs Re-add opencode/minimax-m2.5-free as a hidden fallback alias to big-pickle so repos with the legacy slug still resolve as free. Drop live Zen API experiment tests in freeFallbackCatalog.test.ts. Co-authored-by: Cursor --------- Co-authored-by: Cursor --- entryPost.ts | 16 +++--- mcp/checkout.ts | 6 ++- mcp/shell.ts | 1 + models.test.ts | 5 ++ models.ts | 11 ++++ test/crossagent/codexAuth.ts | 5 +- utils/byokFallback.test.ts | 19 +++++++ utils/codexHome.ts | 34 ------------- ...ome.test.ts => codexRefreshDetect.test.ts} | 2 +- utils/codexRefreshDetect.ts | 35 +++++++++++++ utils/ghaCore.ts | 13 +++++ utils/postApiFetch.ts | 51 +++++++++++++++++++ utils/runErrorRenderer.test.ts | 43 ++++++++++++++++ utils/runErrorRenderer.ts | 26 ++++++++++ utils/vertex.ts | 1 + 15 files changed, 220 insertions(+), 48 deletions(-) rename utils/{codexHome.test.ts => codexRefreshDetect.test.ts} (98%) create mode 100644 utils/codexRefreshDetect.ts create mode 100644 utils/ghaCore.ts create mode 100644 utils/postApiFetch.ts create mode 100644 utils/runErrorRenderer.test.ts diff --git a/entryPost.ts b/entryPost.ts index 29d33a3..8bbf204 100644 --- a/entryPost.ts +++ b/entryPost.ts @@ -24,11 +24,14 @@ // saved). Best-effort: failures are logged but never throw — the workflow // is already done, and a missed refresh write-back means the user re-runs // `pullfrog auth codex` next time the chain breaks. +// +// Imports here MUST stay stdlib-only — GHA runs this file directly from the +// checked-out action repo, which has no node_modules for sha-pinned consumers. import { existsSync, readFileSync } from "node:fs"; -import * as core from "@actions/core"; -import { apiFetch } from "./utils/apiFetch.ts"; -import { detectCodexRefresh } from "./utils/codexHome.ts"; +import { detectCodexRefresh } from "./utils/codexRefreshDetect.ts"; +import * as core from "./utils/ghaCore.ts"; +import { postApiFetch } from "./utils/postApiFetch.ts"; async function main(): Promise { const raw = core.getState("codex_writeback"); @@ -72,11 +75,7 @@ async function main(): Promise { } try { - // route through apiFetch so the Vercel preview-deployment SSO gate gets - // the `x-vercel-protection-bypass` header/query (raw fetch silently 401s - // against preview envs — production is unaffected but every preview-run - // refresh would be lost). see action/utils/apiFetch.ts. - const response = await apiFetch({ + const response = await postApiFetch({ path: "/api/runtime/secret", method: "PUT", headers: { @@ -97,6 +96,5 @@ async function main(): Promise { } main().catch((err) => { - // never throw — post-hook failure must not fail the workflow core.warning(`codex post-hook: unexpected error — ${err}`); }); diff --git a/mcp/checkout.ts b/mcp/checkout.ts index ce50dd9..5d039b2 100644 --- a/mcp/checkout.ts +++ b/mcp/checkout.ts @@ -791,12 +791,14 @@ export function CheckoutPrTool(ctx: ToolContext) { return tool({ name: "checkout_pr", + timeoutMs: 600_000, description: "Checkout a pull request branch locally. This fetches the PR branch and sets up push configuration for fork PRs. " + "Returns diffPath pointing to the formatted diff file. " + "Example: `checkout_pr({ pull_number: 1234 })`. " + - "Transient fetch timeouts are common — retry the same call up to a few times before treating the failure as terminal. " + - "If the error mentions `.git/shallow.lock: File exists` or `.git/index.lock: File exists`, that's a stale lock from a prior timed-out fetch — remove it via the shell tool (`rm -f .git/shallow.lock .git/index.lock`) and retry.", + "Large repos can take several minutes — wait for the call to finish; do not treat a slow response as failure. " + + "If you see `MCP error -32001: Request timed out`, retry the same call without touching git lock files first — that error is a client-side abort. " + + "If the retry then reports `.git/shallow.lock: File exists` or `.git/index.lock: File exists`, remove those lock files via the shell tool and retry again.", parameters: CheckoutPr, execute: execute(async ({ pull_number }) => { const inFlight = inFlightCheckouts.get(pull_number); diff --git a/mcp/shell.ts b/mcp/shell.ts index c8829c1..b901525 100644 --- a/mcp/shell.ts +++ b/mcp/shell.ts @@ -232,6 +232,7 @@ function isGitCommand(command: string): boolean { export function ShellTool(ctx: ToolContext) { return tool({ name: "shell", + timeoutMs: 120_000, description: `Execute shell commands securely. Environment is filtered to remove API keys and secrets. Example: \`shell({ command: "pnpm test", description: "run the test suite" })\`. diff --git a/models.test.ts b/models.test.ts index 0d1aeef..b168ca0 100644 --- a/models.test.ts +++ b/models.test.ts @@ -64,6 +64,7 @@ describe("getModelEnvVars", () => { it("still requires OPENCODE_API_KEY for non-free opencode models", () => { expect(getModelEnvVars("opencode/claude-opus")).toEqual(["OPENCODE_API_KEY"]); + expect(getModelEnvVars("opencode/minimax-m2.5")).toEqual(["OPENCODE_API_KEY"]); expect(getModelEnvVars("opencode/gpt-5-nano")).toEqual(["OPENCODE_API_KEY"]); }); }); @@ -109,6 +110,10 @@ describe("resolveCliModel", () => { expect(resolveCliModel("opencode/gpt-codex")).toBe("opencode/gpt-5.5"); expect(resolveCliModel("openrouter/gpt-codex")).toBe("openrouter/openai/gpt-5.5"); }); + + it("walks fallback chain for hidden deprecated minimax-m2.5-free", () => { + expect(resolveCliModel("opencode/minimax-m2.5-free")).toBe("opencode/big-pickle"); + }); }); describe("resolveDisplayAlias", () => { diff --git a/models.ts b/models.ts index d6484ec..3d146ce 100644 --- a/models.ts +++ b/models.ts @@ -343,6 +343,11 @@ export const providers = { resolve: "opencode/kimi-k2.6", openRouterResolve: "openrouter/moonshotai/kimi-k2.6", }, + "minimax-m2.5": { + displayName: "MiniMax M2.5", + resolve: "opencode/minimax-m2.5", + openRouterResolve: "openrouter/minimax/minimax-m2.5", + }, "gpt-5-nano": { displayName: "GPT Nano", resolve: "opencode/gpt-5-nano", @@ -361,6 +366,7 @@ export const providers = { envVars: [], isFree: true, fallback: "opencode/big-pickle", + hidden: true, }, }, }), @@ -499,6 +505,11 @@ export const providers = { resolve: "openrouter/moonshotai/kimi-k2.6", openRouterResolve: "openrouter/moonshotai/kimi-k2.6", }, + "minimax-m2.5": { + displayName: "MiniMax M2.5", + resolve: "openrouter/minimax/minimax-m2.5", + openRouterResolve: "openrouter/minimax/minimax-m2.5", + }, }, }), } satisfies Record; diff --git a/test/crossagent/codexAuth.ts b/test/crossagent/codexAuth.ts index a553093..8ed872c 100644 --- a/test/crossagent/codexAuth.ts +++ b/test/crossagent/codexAuth.ts @@ -1,6 +1,6 @@ import { randomUUID } from "node:crypto"; import { readFileSync } from "node:fs"; -import { detectCodexRefresh } from "../../utils/codexHome.ts"; +import { detectCodexRefresh } from "../../utils/codexRefreshDetect.ts"; import type { AgentResult, TestRunnerOptions, ValidationCheck } from "../utils.ts"; import { defineFixture } from "../utils.ts"; @@ -24,7 +24,7 @@ import { defineFixture } from "../utils.ts"; * invoked by `pnpm runtest`. instead, this test asserts the on-disk auth.json * state that the post-hook would consume, which is the genuine integration * boundary (everything past `detectCodexRefresh` is a single fetch + unit- - * tested in codexHome.test.ts). + * tested in codexRefreshDetect.test.ts). * * requires `CODEX_AUTH_JSON` in the environment. dev-local: put it in * `.env`. CI: provisioned as `secrets.CODEX_AUTH_JSON` and forwarded by the @@ -99,6 +99,7 @@ export const test: TestRunnerOptions = { }, coverage: [ "action/utils/codexHome.ts", + "action/utils/codexRefreshDetect.ts", "action/entryPost.ts", "action/agents/{opencode,opencode_v2}.ts", ], diff --git a/utils/byokFallback.test.ts b/utils/byokFallback.test.ts index 67dc18c..450c2d0 100644 --- a/utils/byokFallback.test.ts +++ b/utils/byokFallback.test.ts @@ -1,6 +1,17 @@ import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { resolveCliModel } from "../models.ts"; import { FREE_FALLBACK_SLUG, selectFallbackModelIfNeeded } from "./byokFallback.ts"; +describe("FREE_FALLBACK_SLUG", () => { + it("resolves in the curated catalog", () => { + expect(resolveCliModel(FREE_FALLBACK_SLUG)).toBe("opencode/big-pickle"); + }); + + it("is opencode/big-pickle", () => { + expect(FREE_FALLBACK_SLUG).toBe("opencode/big-pickle"); + }); +}); + describe("selectFallbackModelIfNeeded", () => { const originalEnv = { ...process.env }; const KEYS = [ @@ -91,6 +102,14 @@ describe("selectFallbackModelIfNeeded", () => { expect(result.fallback).toBe(false); }); + it("does not fall back when stored minimax-m2.5-free resolves to big-pickle", () => { + const result = selectFallbackModelIfNeeded({ + resolvedModel: resolveCliModel("opencode/minimax-m2.5-free"), + proxyModel: undefined, + }); + expect(result.fallback).toBe(false); + }); + it("treats empty-string env vars as missing (matches GH Actions secret-not-found behavior)", () => { process.env.ANTHROPIC_API_KEY = ""; const result = selectFallbackModelIfNeeded({ diff --git a/utils/codexHome.ts b/utils/codexHome.ts index ca68225..d907311 100644 --- a/utils/codexHome.ts +++ b/utils/codexHome.ts @@ -138,37 +138,3 @@ function parseCodexBlob(raw: string): CodexAuthBlob | null { ...(typeof v.last_refresh === "string" ? { last_refresh: v.last_refresh } : {}), }; } - -/** convert an on-disk OpenCode auth.json back to the Codex CLI shape so the - * post-hook can write it to the Pullfrog secret store. returns null when the - * file's `openai` entry is missing, has the wrong type, or hasn't actually - * refreshed (refresh token unchanged from `originalRefresh`). */ -export function detectCodexRefresh(params: { - authFileContent: string; - originalRefresh: string; -}): string | null { - let parsed: unknown; - try { - parsed = JSON.parse(params.authFileContent); - } catch { - return null; - } - if (!parsed || typeof parsed !== "object") return null; - const oauth = (parsed as Record).openai; - if (!oauth || typeof oauth !== "object") return null; - const o = oauth as Record; - if (o.type !== "oauth") return null; - if (typeof o.refresh !== "string" || typeof o.access !== "string") return null; - if (o.refresh === params.originalRefresh) return null; - - const codexShape: CodexAuthBlob = { - auth_mode: "chatgpt", - tokens: { - access_token: o.access, - refresh_token: o.refresh, - ...(typeof o.accountId === "string" ? { account_id: o.accountId } : {}), - }, - last_refresh: new Date().toISOString(), - }; - return `${JSON.stringify(codexShape, null, 2)}\n`; -} diff --git a/utils/codexHome.test.ts b/utils/codexRefreshDetect.test.ts similarity index 98% rename from utils/codexHome.test.ts rename to utils/codexRefreshDetect.test.ts index 141d3c8..9dde828 100644 --- a/utils/codexHome.test.ts +++ b/utils/codexRefreshDetect.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "vitest"; -import { detectCodexRefresh } from "./codexHome.ts"; +import { detectCodexRefresh } from "./codexRefreshDetect.ts"; // installCodexAuth touches the filesystem (mkdir + writeFile) — leaving it // untested here per AGENTS.md guidance ("be highly dubious of any test that diff --git a/utils/codexRefreshDetect.ts b/utils/codexRefreshDetect.ts new file mode 100644 index 0000000..b3e52e5 --- /dev/null +++ b/utils/codexRefreshDetect.ts @@ -0,0 +1,35 @@ +/** Convert an on-disk OpenCode auth.json back to the Codex CLI shape so the + * post-hook can write it to the Pullfrog secret store. Returns null when the + * file's `openai` entry is missing, has the wrong type, or hasn't actually + * refreshed (refresh token unchanged from `originalRefresh`). Lives in its + * own module so `entryPost.ts` can import it without pulling in `codexHome.ts` + * (which imports `./cli.ts` and node fs helpers). */ +export function detectCodexRefresh(params: { + authFileContent: string; + originalRefresh: string; +}): string | null { + let parsed: unknown; + try { + parsed = JSON.parse(params.authFileContent); + } catch { + return null; + } + if (!parsed || typeof parsed !== "object") return null; + const oauth = (parsed as Record).openai; + if (!oauth || typeof oauth !== "object") return null; + const o = oauth as Record; + if (o.type !== "oauth") return null; + if (typeof o.refresh !== "string" || typeof o.access !== "string") return null; + if (o.refresh === params.originalRefresh) return null; + + const codexShape = { + auth_mode: "chatgpt", + tokens: { + access_token: o.access, + refresh_token: o.refresh, + ...(typeof o.accountId === "string" ? { account_id: o.accountId } : {}), + }, + last_refresh: new Date().toISOString(), + }; + return `${JSON.stringify(codexShape, null, 2)}\n`; +} diff --git a/utils/ghaCore.ts b/utils/ghaCore.ts new file mode 100644 index 0000000..859f80e --- /dev/null +++ b/utils/ghaCore.ts @@ -0,0 +1,13 @@ +/** stdlib-only GitHub Actions helpers for entryPost.ts (no node_modules). */ + +export function getState(name: string): string { + return process.env[`STATE_${name}`] ?? ""; +} + +export function info(message: string): void { + console.log(message); +} + +export function warning(message: string): void { + console.log(`::warning::${message}`); +} diff --git a/utils/postApiFetch.ts b/utils/postApiFetch.ts new file mode 100644 index 0000000..59a027f --- /dev/null +++ b/utils/postApiFetch.ts @@ -0,0 +1,51 @@ +/** stdlib-only Pullfrog API fetch for entryPost.ts (no node_modules). */ + +type PostApiFetchOptions = { + path: string; + method?: string | undefined; + headers?: Record | undefined; + body?: string | undefined; +}; + +function getApiUrl(): string { + return process.env.API_URL || "https://pullfrog.com"; +} + +export async function postApiFetch(options: PostApiFetchOptions): Promise { + const url = new URL(options.path, getApiUrl()); + + const bypassSecret = process.env.VERCEL_AUTOMATION_BYPASS_SECRET; + if (bypassSecret) { + url.searchParams.set("x-vercel-protection-bypass", bypassSecret); + } + + const headers: Record = { + ...options.headers, + }; + + if (bypassSecret) { + headers["x-vercel-protection-bypass"] = bypassSecret; + } + + if (!options.body) { + for (const key of Object.keys(headers)) { + if (key.toLowerCase() === "content-type") delete headers[key]; + } + } + + const controller = new AbortController(); + const timeoutId = setTimeout(() => controller.abort(), 30_000); + + try { + const init: RequestInit = { + method: options.method ?? "GET", + headers, + signal: controller.signal, + }; + if (options.body) init.body = options.body; + + return await fetch(url, init); + } finally { + clearTimeout(timeoutId); + } +} diff --git a/utils/runErrorRenderer.test.ts b/utils/runErrorRenderer.test.ts new file mode 100644 index 0000000..415973f --- /dev/null +++ b/utils/runErrorRenderer.test.ts @@ -0,0 +1,43 @@ +import { describe, expect, it } from "vitest"; +import { renderRunError } from "./runErrorRenderer.ts"; + +const repo = { owner: "acme", name: "widget" }; + +describe("renderRunError ProviderModelNotFoundError (#816)", () => { + const staleFreeRaw = + 'ProviderModelNotFoundError: {"providerID":"opencode","modelID":"retired-free-model","suggestions":["deepseek-v4-flash-free"]}'; + + const bigPickleRaw = + 'ProviderModelNotFoundError: {"providerID":"opencode","modelID":"big-pickle","suggestions":[]}'; + + it("renders actionable copy for a stale free fallback model id", () => { + const result = renderRunError({ + errorMessage: staleFreeRaw, + repo, + agentDiagnostic: undefined, + }); + expect(result.summary).toContain("Pullfrog's free fallback model is no longer available"); + expect(result.summary).toContain("`acme/widget`"); + expect(result.summary).toContain("retired-free-model"); + expect(result.comment).toBe(result.summary); + }); + + it("renders the same classifier when big-pickle is missing from opencode catalog", () => { + const result = renderRunError({ + errorMessage: bigPickleRaw, + repo, + agentDiagnostic: undefined, + }); + expect(result.summary).toContain("Pullfrog's free fallback model is no longer available"); + expect(result.summary).toContain("big-pickle"); + }); + + it("does not misclassify unrelated failures as fallback-catalog errors", () => { + const result = renderRunError({ + errorMessage: "activity timeout after 900s", + repo, + agentDiagnostic: undefined, + }); + expect(result.summary).not.toContain("free fallback model is no longer available"); + }); +}); diff --git a/utils/runErrorRenderer.ts b/utils/runErrorRenderer.ts index 9d60a5d..c8909bc 100644 --- a/utils/runErrorRenderer.ts +++ b/utils/runErrorRenderer.ts @@ -38,6 +38,23 @@ export type RenderedRunError = { comment: string; }; +function isProviderModelNotFoundError(message: string): boolean { + return message.includes("ProviderModelNotFoundError"); +} + +function formatProviderModelNotFoundSummary(input: { + owner: string; + name: string; + raw: string; +}): string { + return ( + `Pullfrog's free fallback model is no longer available in OpenCode's catalog. ` + + `Add an API key for your configured model in the Pullfrog console for \`${input.owner}/${input.name}\`, ` + + `or contact support if this persists.\n\n` + + `\`\`\`\n${input.raw}\n\`\`\`` + ); +} + export function renderRunError(input: { errorMessage: string; repo: { owner: string; name: string }; @@ -83,6 +100,15 @@ export function renderRunError(input: { return { summary: apiKeyErrorSummary, comment: apiKeyErrorSummary }; } + if (isProviderModelNotFoundError(input.errorMessage)) { + const body = formatProviderModelNotFoundSummary({ + owner: input.repo.owner, + name: input.repo.name, + raw: input.errorMessage, + }); + return { summary: body, comment: body }; + } + if (hangBody) { return { summary: `### ❌ Pullfrog failed\n\n${hangBody}`, diff --git a/utils/vertex.ts b/utils/vertex.ts index 5a5080c..7d5be75 100644 --- a/utils/vertex.ts +++ b/utils/vertex.ts @@ -71,6 +71,7 @@ export function materializeVertexCredentials(params: { export function cleanupVertexCredentials(credentials: VertexCredentials | undefined): void { if (!credentials) return; rmSync(credentials.secretDir, { recursive: true, force: true }); + delete process.env[GOOGLE_APPLICATION_CREDENTIALS_ENV]; } export function applyClaudeVertexEnv(env: Record): void {