diff --git a/agents/opencode.ts b/agents/opencode.ts index 69faa97..450a924 100644 --- a/agents/opencode.ts +++ b/agents/opencode.ts @@ -1114,7 +1114,7 @@ export const opencode = agent({ run: async (ctx) => { const cliPath = await installCli(); - const rawModel = ctx.payload.proxyModel ?? ctx.resolvedModel ?? autoSelectModel(cliPath); + const rawModel = ctx.payload.proxyModel ?? ctx.resolvedModel ?? autoSelectModel(); // bedrock route: opencode's `amazon-bedrock` provider expects the model // string in `amazon-bedrock/` form. the bare AWS model ID diff --git a/agents/opencodeShared.ts b/agents/opencodeShared.ts index d711caf..f99a121 100644 --- a/agents/opencodeShared.ts +++ b/agents/opencodeShared.ts @@ -6,10 +6,10 @@ // then it keeps both runners synchronized so a config drift can't make v1 a // silently-broken fallback. -import { execFileSync } from "node:child_process"; import { modelAliases } from "../models.ts"; import { log } from "../utils/cli.ts"; import { installFromNpmTarball } from "../utils/install.ts"; +import { getAuthorizedModels } from "../utils/openCodeModels.ts"; import { getDevDependencyVersion } from "../utils/version.ts"; import { REVIEWER_AGENT_NAME, REVIEWER_SYSTEM_PROMPT } from "./reviewer.ts"; import { deriveSubagentModels } from "./subagentModels.ts"; @@ -91,42 +91,22 @@ export async function installOpencodeCli(params: { binPath: string }): Promise line.trim()) - .filter(Boolean); - } catch (error) { - log.debug( - `» failed to run \`opencode models\`: ${error instanceof Error ? error.message : String(error)}` - ); - return []; - } -} - -export function autoSelectModel(cliPath: string): string | undefined { - const availableModels = getOpenCodeModels(cliPath); - const availableSet = new Set(availableModels); - if (availableSet.size > 0) { - log.debug(`» opencode models (${availableSet.size}): ${availableModels.join(", ")}`); +export function autoSelectModel(): string | undefined { + const authorized = getAuthorizedModels(); + if (authorized.size > 0) { // skip hidden aliases (internal subagent-tier targets like // opencode/gpt-5.4) — they should never surface as a user-facing // orchestrator pick. mirrors the selectable-list filter in // components/ModelSelector.tsx and action/commands/init.ts. const match = - modelAliases.find((a) => !a.hidden && a.preferred && availableSet.has(a.resolve)) ?? - modelAliases.find((a) => !a.hidden && availableSet.has(a.resolve)); + modelAliases.find((a) => !a.hidden && a.preferred && authorized.has(a.resolve)) ?? + modelAliases.find((a) => !a.hidden && authorized.has(a.resolve)); if (match) { log.info( `» model: ${match.resolve} (auto-selected${match.preferred ? " — preferred" : ""} curated match)` @@ -135,7 +115,7 @@ export function autoSelectModel(cliPath: string): string | undefined { return match.resolve; } log.info( - `» opencode has ${availableSet.size} models but none match curated aliases — letting OpenCode auto-select` + `» opencode has ${authorized.size} models but none match curated aliases — letting OpenCode auto-select` ); } diff --git a/agents/opencode_v2.ts b/agents/opencode_v2.ts index 0063481..dbb9fe4 100644 --- a/agents/opencode_v2.ts +++ b/agents/opencode_v2.ts @@ -890,7 +890,7 @@ export const opencode = agent({ run: async (ctx) => { const cliPath = await installCli(); - const rawModel = ctx.payload.proxyModel ?? ctx.resolvedModel ?? autoSelectModel(cliPath); + const rawModel = ctx.payload.proxyModel ?? ctx.resolvedModel ?? autoSelectModel(); // bedrock route: opencode's `amazon-bedrock` provider expects the model // string in `amazon-bedrock/` form. the bare AWS model ID diff --git a/main.ts b/main.ts index 5bfe52a..172c52e 100644 --- a/main.ts +++ b/main.ts @@ -3,6 +3,7 @@ import { existsSync, readdirSync } from "node:fs"; import { readFile } from "node:fs/promises"; import { join } from "node:path"; +import { agents } from "./agents/index.ts"; import { reportProgress } from "./mcp/comment.ts"; import { startInstallation } from "./mcp/dependencies.ts"; import { startMcpHttpServer, type ToolContext } from "./mcp/server.ts"; @@ -19,6 +20,7 @@ import { validateAgentApiKey } from "./utils/apiKeys.ts"; import { resolveBody } from "./utils/body.ts"; import { selectFallbackModelIfNeeded } from "./utils/byokFallback.ts"; import { log } from "./utils/cli.ts"; +import { installCodexAuth } from "./utils/codexHome.ts"; import { recordDiffReadFromToolUse } from "./utils/diffCoverage.ts"; import { onExitSignal } from "./utils/exitHandler.ts"; import { resolveGit, setGitAuthServer } from "./utils/gitAuth.ts"; @@ -28,7 +30,13 @@ import { resolveInstructions } from "./utils/instructions.ts"; import { persistLearnings, seedLearningsFile } from "./utils/learnings.ts"; import { executeLifecycleHook } from "./utils/lifecycle.ts"; import { normalizeEnv, sanitizeSecret } from "./utils/normalizeEnv.ts"; +import { + captureAuthorizedModels, + captureBaselineModels, + getAuthorizedModels, +} from "./utils/openCodeModels.ts"; import { applyOverrides } from "./utils/overrides.ts"; +import { ensurePackageManager, resolvePackageManagerSpec } from "./utils/packageManager.ts"; import { aggregateUsage, patchWorkflowRunFields } from "./utils/patchWorkflowRunFields.ts"; import { resolveOutputSchema, resolvePayload, resolvePromptInput } from "./utils/payload.ts"; import { type OidcCredentials, runProxyResolution } from "./utils/proxy.ts"; @@ -115,6 +123,20 @@ export async function main(): Promise { const runContext = await resolveRunContextData({ octokit: initialOctokit, token: jobToken }); timer.checkpoint("runContextData"); + // tmpdir hoisted out of the try block: `installFromNpmTarball` reads + // PULLFROG_TEMP_DIR (set as a side effect of createTempDirectory) when + // the opencode CLI install runs below for BYOK introspection. agent + + // mcp server setup further down also consume the same tmpdir. + const tmpdir = createTempDirectory(); + + // install OpenCode + capture the BASELINE model set BEFORE dbSecrets and + // Codex auth.json are in scope. this is the set of models OpenCode can + // route from the runner's pre-existing environment alone (workflow + // `env:` block + GH Actions secrets). install is fs-cached, so the + // duplicate call inside the opencode agent's run() is a no-op. + const opencodeCliPath = await agents.opencode.install(); + captureBaselineModels(opencodeCliPath); + // inject account-level secrets into process.env (YAML secrets take precedence). // sanitizeSecret trims + masks so accidental trailing whitespace doesn't leak // through GitHub Actions' line-based log masking. whitespace-only values @@ -130,6 +152,19 @@ export async function main(): Promise { if (count > 0) log.info(`» ${count} db secret(s) loaded`); } + // materialize Codex auth.json (idempotent — opencode agent re-calls inside + // run() and writes the same file). this has to land BEFORE + // captureAuthorizedModels so OpenCode's model introspection sees the + // OAuth-routed openai/* models. + installCodexAuth(); + + // capture the AUTHORIZED model set after dbSecrets + Codex auth.json are + // applied. this is the authoritative source for the BYOK fallback + // decision and the opencode-agent path of validateAgentApiKey — strictly + // more accurate than the static envVars/managedCredentials catalog, + // which can miss new auth shapes. + captureAuthorizedModels(opencodeCliPath); + // configure env allowlist for subprocess filtering if (runContext.repoSettings.envAllowlist) { setEnvAllowlist(runContext.repoSettings.envAllowlist); @@ -212,8 +247,6 @@ export async function main(): Promise { } } - const tmpdir = createTempDirectory(); - await using gitAuthServer = await startGitAuthServer(tmpdir); setGitAuthServer(gitAuthServer); @@ -228,9 +261,11 @@ export async function main(): Promise { // — exactly the silent-churn pattern that took out 15 accounts before // this landed. Router/proxy runs are skipped (Pullfrog mints the key); // see `selectFallbackModelIfNeeded` for the full skip set. + const authorized = getAuthorizedModels(); const fallback = selectFallbackModelIfNeeded({ resolvedModel: initialResolvedModel, proxyModel: payload.proxyModel, + authorized, }); // when fallback engages we bypass `resolveModel` for the new slug — // `PULLFROG_MODEL` has higher priority than the slug arg inside that @@ -256,12 +291,27 @@ export async function main(): Promise { // so the "Using `…`" badge reflects what actually ran. toolState.model = payload.proxyModel ?? resolvedModel ?? effectiveSlug; - validateAgentApiKey({ - agent, - model: payload.proxyModel ?? resolvedModel ?? effectiveSlug, - owner: runContext.repo.owner, - name: runContext.repo.name, - }); + // skip validation when fallback engaged: the effective model is the + // free fallback (`opencode/big-pickle`) and the fallback gate already + // authoritatively decided "this model is OK to run". re-validating + // would spuriously throw if `opencode models` doesn't list big-pickle. + // + // also skip when proxyModel is set: `runProxyResolution` already minted + // OPENROUTER_API_KEY and the server-side gate (`run-context/route.ts`) + // is the authority on "can this run use the router". the `authorized` + // set was captured BEFORE the proxy mint, so it doesn't see the + // openrouter slug — re-validating would spuriously throw. mirrors the + // analogous `if (input.proxyModel) return { fallback: false }` skip in + // `selectFallbackModelIfNeeded`. + if (!fallback.fallback && !payload.proxyModel) { + validateAgentApiKey({ + agent, + model: payload.proxyModel ?? resolvedModel ?? effectiveSlug, + authorized, + owner: runContext.repo.owner, + name: runContext.repo.name, + }); + } await setupGit({ gitToken: tokenRef.gitToken, @@ -274,12 +324,27 @@ export async function main(): Promise { }); timer.checkpoint("git"); + // pin the project's package manager via corepack BEFORE the setup hook + // runs. without this, a customer setup script like `npm i -g pnpm && + // pnpm install` installs whatever pnpm is "latest" today and writes + // unrelated lockfile drift (e.g. the `packageManagerDependencies` + // block pnpm 11.3 added) into our commit — see #844. resolution + // honors pnpm 11+ precedence (`devEngines.packageManager` over + // `packageManager`); failure is non-fatal — we fall back to whatever + // is on PATH with a warning. + const pmSpec = await resolvePackageManagerSpec(process.cwd()); + if (pmSpec) { + await ensurePackageManager(pmSpec); + } + timer.checkpoint("packageManager"); + // execute setup lifecycle hook (runs once at initialization). // setup is load-bearing — if it fails the rest of the run is in an // undefined state, so upgrade the soft-fail warning to a hard error. const setupHook = await executeLifecycleHook({ event: "setup", script: runContext.repoSettings.setupScript, + normalizeWorkingTreeAfter: true, }); if (setupHook.warning) { throw new Error(setupHook.warning); diff --git a/mcp/checkout.ts b/mcp/checkout.ts index 7a0ab93..f667d82 100644 --- a/mcp/checkout.ts +++ b/mcp/checkout.ts @@ -599,6 +599,7 @@ export async function checkoutPrBranch( const postCheckoutHook = await executeLifecycleHook({ event: "post-checkout", script: params.postCheckoutScript, + normalizeWorkingTreeAfter: true, }); return { hookWarning: postCheckoutHook.warning }; } diff --git a/prep/installNodeDependencies.ts b/prep/installNodeDependencies.ts index 034bb48..0593b5f 100644 --- a/prep/installNodeDependencies.ts +++ b/prep/installNodeDependencies.ts @@ -1,21 +1,12 @@ -import { existsSync, readFileSync } from "node:fs"; +import { existsSync } from "node:fs"; import { join } from "node:path"; -import { isKeyOf } from "@ark/util"; import { detect } from "package-manager-detector"; import { resolveCommand } from "package-manager-detector/commands"; import { log } from "../utils/cli.ts"; +import { ensurePackageManager, resolvePackageManagerSpec } from "../utils/packageManager.ts"; import { spawn } from "../utils/subprocess.ts"; import type { NodePackageManager, NodePrepResult, PrepDefinition, PrepOptions } from "./types.ts"; -// install command templates for each package manager (version placeholder: {version}) -const nodePackageManagers: Record = { - npm: ["echo", "npm is already installed"], - pnpm: ["npm", "install", "-g", "{version}"], - yarn: ["npm", "install", "-g", "{version}"], - bun: ["npm", "install", "-g", "{version}"], - deno: ["sh", "-c", "curl -fsSL https://deno.land/install.sh | sh"], -}; - async function isCommandAvailable(command: string): Promise { const result = await spawn({ cmd: "which", @@ -25,57 +16,33 @@ async function isCommandAvailable(command: string): Promise { return result.exitCode === 0; } -interface PackageManagerSpec { - name: NodePackageManager; - installSpec: string; // e.g., "pnpm@8.15.0" (without hash suffix) -} - -function getPackageManagerFromPackageJson(): PackageManagerSpec | null { - const packageJsonPath = join(process.cwd(), "package.json"); - try { - const content = readFileSync(packageJsonPath, "utf-8"); - const pkg = JSON.parse(content) as { packageManager?: string }; - if (!pkg.packageManager) return null; - - // format: "pnpm@8.15.0" or "pnpm@8.15.0+sha512.abc123..." - // strip the hash suffix (+sha256.xxx) as npm install doesn't understand it - const withoutHash = pkg.packageManager.split("+")[0]; - const name = withoutHash.split("@")[0]; - if (isKeyOf(name, nodePackageManagers)) { - return { name, installSpec: withoutHash }; - } - log.warning(`unknown packageManager in package.json: ${pkg.packageManager}`); - return null; - } catch { - return null; - } -} - -async function installPackageManager( +// fallback installers for managers corepack doesn't ship shims for. +// pnpm and yarn are handled by `ensurePackageManager` (corepack); bun and +// deno fall through to here because corepack ignores them. +async function installFallback( name: NodePackageManager, installSpec: string ): Promise { - if (name === "npm") return null; // npm is always available - log.info(`» installing ${installSpec}...`); - const [cmd, ...templateArgs] = nodePackageManagers[name]; - const args = templateArgs.map((arg) => (arg === "{version}" ? installSpec : arg)); + if (name === "npm") return null; + log.info(`» installing ${installSpec} via npm install -g (corepack does not manage ${name})`); + const args = + name === "deno" + ? ["-c", "curl -fsSL https://deno.land/install.sh | sh"] + : ["install", "-g", installSpec]; + const cmd = name === "deno" ? "sh" : "npm"; const result = await spawn({ cmd, args, env: { PATH: process.env.PATH || "", HOME: process.env.HOME || "" }, onStderr: (chunk) => process.stderr.write(chunk), }); - if (result.exitCode !== 0) { return result.stderr || `failed to install ${name}`; } - - // deno installs to $HOME/.deno/bin - add to PATH for subsequent commands if (name === "deno") { const denoPath = join(process.env.HOME || "", ".deno", "bin"); process.env.PATH = `${denoPath}:${process.env.PATH}`; } - log.info(`» installed ${name}`); return null; } @@ -89,30 +56,34 @@ export const installNodeDependencies: PrepDefinition = { }, run: async (options: PrepOptions): Promise => { - // check packageManager field in package.json first (takes priority) - const fromPackageJson = getPackageManagerFromPackageJson(); + // prefer the project's declared spec (devEngines.packageManager wins over + // packageManager). fall back to lockfile detection when nothing is declared. + // restrict detect() to the lockfile strategy: `detected` here doubles as + // the lockfile-presence gate below, and the default strategy set also + // returns positives off `packageManager`/`devEngines` fields (which would + // mask the very case we're trying to detect — declared manager but no + // lockfile committed). + const declared = await resolvePackageManagerSpec(process.cwd()); + const detected = await detect({ cwd: process.cwd(), strategies: ["lockfile"] }); - // detect from lockfile as fallback - const detected = await detect({ cwd: process.cwd() }); + const packageManager: NodePackageManager = + declared?.name ?? (detected?.name as NodePackageManager) ?? "npm"; + const agent = detected?.agent ?? packageManager; - // prefer package.json field, fall back to lockfile detection, default to npm - const packageManager = fromPackageJson?.name || (detected?.name as NodePackageManager) || "npm"; - const installSpec = fromPackageJson?.installSpec || packageManager; - const agent = detected?.agent || packageManager; - - if (fromPackageJson) { - log.info(`» using packageManager from package.json: ${fromPackageJson.installSpec}`); + if (declared) { + log.info( + `» using ${packageManager}@${declared.version} from package.json (${declared.source})` + ); } else if (detected) { log.info(`» detected package manager: ${packageManager} (${agent})`); } else { - log.info(`» no package manager detected, defaulting to npm`); + log.info(`» no package manager declared, defaulting to npm`); } - // check if package manager is available, install if needed + // provisioning: corepack for pnpm/yarn, legacy npm-install-g for bun/deno. + // when shell is disabled we can't run installers (they execute code), so + // we require the binary to already be on PATH. if (!(await isCommandAvailable(packageManager))) { - // SECURITY: when shell is disabled, don't install package managers. - // installPackageManager runs `npm install -g` or `curl | sh` (for deno), - // both of which execute code. the package manager must already be available. if (options.ignoreScripts) { return { language: "node", @@ -123,26 +94,50 @@ export const installNodeDependencies: PrepDefinition = { ], }; } - log.info(`» ${packageManager} not found, attempting to install...`); - const installError = await installPackageManager(packageManager, installSpec); - if (installError) { - return { - language: "node", - packageManager, - dependenciesInstalled: false, - issues: [installError], - }; + + let provisioned = false; + if (declared) provisioned = await ensurePackageManager(declared); + if (!provisioned) { + const fallbackSpec = declared ? `${declared.name}@${declared.version}` : packageManager; + const installError = await installFallback(packageManager, fallbackSpec); + if (installError) { + return { + language: "node", + packageManager, + dependenciesInstalled: false, + issues: [installError], + }; + } } + } else if (declared) { + // PATH already has the binary — but it may be the wrong version. + // ensurePackageManager is idempotent (caches on `--version` match) so + // this is cheap when main.ts already activated it. + await ensurePackageManager(declared); } // frozen-lockfile install only. eager prep is non-mutating by contract: // we run it before the agent starts and any artifact it leaves in the // tree (e.g. a generated `package-lock.json`) trips the dirty-tree // post-run gate and produces a spurious PR. `frozen` commands - // (`npm ci`, `pnpm install --frozen-lockfile`, etc.) fail cleanly - // without modifying state when there's no lockfile, which is exactly - // what we want — repos that need a non-frozen install must opt in via - // a `setup` lifecycle hook (`action/utils/lifecycle.ts`). + // (`npm ci`, `pnpm install --frozen-lockfile`, etc.) were assumed to + // fail cleanly without a lockfile — that assumption is false for + // pnpm 11.1.1 against a no-deps `package.json` (it silently writes an + // empty `pnpm-lock.yaml` despite the flag). gate on `detect()` having + // found a lockfile; it walks up the tree (so monorepo subpackages + // resolve to the workspace-root lockfile) and recognizes every + // manager's accepted lockfile variants (`bun.lockb` + `bun.lock`, + // `npm-shrinkwrap.json` + `package-lock.json`, etc.). when none is + // present, the project either has no installable dependencies or + // opts into install via a `setup` lifecycle hook + // (`action/utils/lifecycle.ts`); either way, eager prep should skip. + if (!detected) { + log.info( + `» skipping ${packageManager} install: no lockfile found (would otherwise risk lockfile drift)` + ); + return { language: "node", packageManager, dependenciesInstalled: false, issues: [] }; + } + const resolved = resolveCommand(agent, "frozen", []); if (!resolved) { return { diff --git a/test/agnostic/byokFallback.ts b/test/agnostic/byokFallback.ts index c8df3c9..4d7732a 100644 --- a/test/agnostic/byokFallback.ts +++ b/test/agnostic/byokFallback.ts @@ -69,6 +69,7 @@ export const test: TestRunnerOptions = { "action/utils/byokFallback.ts", "action/utils/apiKeys.ts", "action/utils/agent.ts", + "action/utils/openCodeModels.ts", "action/main.ts", "action/models.ts", ], diff --git a/utils/apiKeys.test.ts b/utils/apiKeys.test.ts index 4060a6f..d4ffe52 100644 --- a/utils/apiKeys.test.ts +++ b/utils/apiKeys.test.ts @@ -1,18 +1,9 @@ import { afterEach, beforeEach, describe, expect, it } from "vitest"; import { formatApiKeyErrorSummary, isApiKeyAuthError, validateAgentApiKey } from "./apiKeys.ts"; -const base = { - agent: { name: "opencode" }, - owner: "test-owner", - name: "test-repo", -}; - const savedEnv = { ...process.env }; -// keys that count as provider auth in `knownApiKeys` and would let the -// auto-select path pass without our intent. strip all of them at test setup -// so each `it` starts from a clean slate regardless of what's in the dev `.env`. -const STRIPPED_PREFIXES_OR_NAMES = [ +const ENV_KEYS_TO_STRIP = [ /_API_KEY$/, /^CLAUDE_CODE_OAUTH_TOKEN$/, /^CODEX_AUTH_JSON$/, @@ -31,7 +22,7 @@ const STRIPPED_PREFIXES_OR_NAMES = [ beforeEach(() => { for (const key of Object.keys(process.env)) { - if (STRIPPED_PREFIXES_OR_NAMES.some((re) => re.test(key))) delete process.env[key]; + if (ENV_KEYS_TO_STRIP.some((re) => re.test(key))) delete process.env[key]; } }); @@ -39,197 +30,182 @@ afterEach(() => { process.env = { ...savedEnv }; }); -describe("validateAgentApiKey", () => { - describe("free model (no keys required)", () => { - it("passes with zero env keys", () => { - expect(() => validateAgentApiKey({ ...base, model: "opencode/big-pickle" })).not.toThrow(); - }); +const opencode = { name: "opencode" }; +const claude = { name: "claude" }; +const owner = "test-owner"; +const name = "test-repo"; - it("passes for other free opencode models", () => { - for (const slug of ["opencode/mimo-v2-pro-free", "opencode/minimax-m2.5-free"]) { - expect(() => validateAgentApiKey({ ...base, model: slug })).not.toThrow(); - } - }); +describe("validateAgentApiKey — opencode", () => { + it("passes when the resolved model is in the authorized set", () => { + expect(() => + validateAgentApiKey({ + agent: opencode, + model: "anthropic/claude-opus-4-7", + authorized: new Set(["anthropic/claude-opus-4-7"]), + owner, + name, + }) + ).not.toThrow(); }); - describe("keyed model", () => { - it("passes when the required key is present", () => { - process.env.ANTHROPIC_API_KEY = "sk-test"; - expect(() => validateAgentApiKey({ ...base, model: "anthropic/claude-opus" })).not.toThrow(); - }); - - it("throws when the required key is missing", () => { - expect(() => validateAgentApiKey({ ...base, model: "anthropic/claude-opus" })).toThrow( - "no API key found" - ); - }); - - it("passes for opencode keyed model with OPENCODE_API_KEY", () => { - process.env.OPENCODE_API_KEY = "sk-test"; - expect(() => validateAgentApiKey({ ...base, model: "opencode/claude-opus" })).not.toThrow(); - }); - - it("throws for opencode keyed model without OPENCODE_API_KEY", () => { - expect(() => validateAgentApiKey({ ...base, model: "opencode/claude-opus" })).toThrow( - "no API key found" - ); - }); - - it("throws for opencode/gpt-5-nano without OPENCODE_API_KEY (paid Zen alias)", () => { - expect(() => validateAgentApiKey({ ...base, model: "opencode/gpt-5-nano" })).toThrow( - "no API key found" - ); - }); + it("throws when the resolved model is absent from the authorized set", () => { + expect(() => + validateAgentApiKey({ + agent: opencode, + model: "anthropic/claude-opus-4-7", + authorized: new Set(), + owner, + name, + }) + ).toThrow("no API key found"); }); - describe("no model (auto-select)", () => { - it("passes when any known provider key is present", () => { - process.env.OPENAI_API_KEY = "sk-test"; - expect(() => validateAgentApiKey({ ...base, model: undefined })).not.toThrow(); - }); - - it("throws when no provider keys are present", () => { - expect(() => validateAgentApiKey({ ...base, model: undefined })).toThrow("no API key found"); - }); + it("passes the auto-select path when the authorized set is non-empty", () => { + expect(() => + validateAgentApiKey({ + agent: opencode, + model: undefined, + authorized: new Set(["opencode/big-pickle"]), + owner, + name, + }) + ).not.toThrow(); }); - describe("bedrock routing slug", () => { - it("passes with AWS_BEARER_TOKEN_BEDROCK + AWS_REGION + BEDROCK_MODEL_ID", () => { - process.env.AWS_BEARER_TOKEN_BEDROCK = "bedrock-token"; - process.env.AWS_REGION = "us-east-1"; - process.env.BEDROCK_MODEL_ID = "us.anthropic.claude-opus-4-7"; - expect(() => validateAgentApiKey({ ...base, model: "bedrock/byok" })).not.toThrow(); - }); + it("throws the auto-select path when the authorized set is empty", () => { + expect(() => + validateAgentApiKey({ + agent: opencode, + model: undefined, + authorized: new Set(), + owner, + name, + }) + ).toThrow("no API key found"); + }); +}); - it("passes with AWS access keys + region + model id", () => { - process.env.AWS_ACCESS_KEY_ID = "AKIA-test"; - process.env.AWS_SECRET_ACCESS_KEY = "secret-test"; - process.env.AWS_REGION = "us-east-1"; - process.env.BEDROCK_MODEL_ID = "amazon.nova-pro-v1:0"; - expect(() => validateAgentApiKey({ ...base, model: "bedrock/byok" })).not.toThrow(); - }); - - it("throws when BEDROCK_MODEL_ID is missing", () => { - process.env.AWS_BEARER_TOKEN_BEDROCK = "bedrock-token"; - process.env.AWS_REGION = "us-east-1"; - expect(() => validateAgentApiKey({ ...base, model: "bedrock/byok" })).toThrow( - "BEDROCK_MODEL_ID" - ); - }); - - it("throws when AWS_REGION is missing", () => { - process.env.AWS_BEARER_TOKEN_BEDROCK = "bedrock-token"; - process.env.BEDROCK_MODEL_ID = "us.anthropic.claude-opus-4-7"; - expect(() => validateAgentApiKey({ ...base, model: "bedrock/byok" })).toThrow("AWS_REGION"); - }); - - it("throws when no auth is set", () => { - process.env.AWS_REGION = "us-east-1"; - process.env.BEDROCK_MODEL_ID = "us.anthropic.claude-opus-4-7"; - expect(() => validateAgentApiKey({ ...base, model: "bedrock/byok" })).toThrow( - "AWS_BEARER_TOKEN_BEDROCK" - ); - }); - - it("throws when only AWS_ACCESS_KEY_ID is set (missing secret)", () => { - process.env.AWS_ACCESS_KEY_ID = "AKIA-test"; - process.env.AWS_REGION = "us-east-1"; - process.env.BEDROCK_MODEL_ID = "us.anthropic.claude-opus-4-7"; - expect(() => validateAgentApiKey({ ...base, model: "bedrock/byok" })).toThrow( - "AWS_BEARER_TOKEN_BEDROCK" - ); - }); - - // regression: main.ts passes the resolved model into validateAgentApiKey - // (`payload.proxyModel ?? resolvedModel ?? payload.model`), which for - // bedrock is the raw AWS model ID and has no `/`. parseModel would throw. - // see PR #720 e2e run 25821218139 for the original failure mode. - it("accepts a raw Bedrock model ID (post-resolveModel) without throwing", () => { - process.env.AWS_BEARER_TOKEN_BEDROCK = "bedrock-token"; - process.env.AWS_REGION = "us-east-1"; - process.env.BEDROCK_MODEL_ID = "us.anthropic.claude-opus-4-6-v1"; - expect(() => - validateAgentApiKey({ ...base, model: "us.anthropic.claude-opus-4-6-v1" }) - ).not.toThrow(); - }); - - it("throws on raw Bedrock model ID when AWS auth is missing", () => { - process.env.AWS_REGION = "us-east-1"; - process.env.BEDROCK_MODEL_ID = "us.anthropic.claude-opus-4-6-v1"; - expect(() => - validateAgentApiKey({ ...base, model: "us.anthropic.claude-opus-4-6-v1" }) - ).toThrow("AWS_BEARER_TOKEN_BEDROCK"); - }); +describe("validateAgentApiKey — claude (static Anthropic check)", () => { + it("passes when ANTHROPIC_API_KEY is set", () => { + process.env.ANTHROPIC_API_KEY = "sk-test"; + expect(() => + validateAgentApiKey({ + agent: claude, + model: "anthropic/claude-opus-4-7", + authorized: new Set(), + owner, + name, + }) + ).not.toThrow(); }); - describe("vertex routing slug", () => { - it("passes with service-account JSON + project + location + model id", () => { - process.env.VERTEX_SERVICE_ACCOUNT_JSON = "{}"; - process.env.GOOGLE_CLOUD_PROJECT = "test-project"; - process.env.VERTEX_LOCATION = "us-east5"; - process.env.VERTEX_MODEL_ID = "claude-opus-4-1@20250805"; - expect(() => validateAgentApiKey({ ...base, model: "vertex/byok" })).not.toThrow(); - }); + it("passes when CLAUDE_CODE_OAUTH_TOKEN is set", () => { + process.env.CLAUDE_CODE_OAUTH_TOKEN = "oauth-test"; + expect(() => + validateAgentApiKey({ + agent: claude, + model: "anthropic/claude-opus-4-7", + authorized: new Set(), + owner, + name, + }) + ).not.toThrow(); + }); - it("passes when project is derivable from service-account JSON", () => { - process.env.VERTEX_SERVICE_ACCOUNT_JSON = JSON.stringify({ project_id: "test-project" }); - process.env.VERTEX_LOCATION = "us-east5"; - process.env.VERTEX_MODEL_ID = "gemini-2.5-pro"; - expect(() => validateAgentApiKey({ ...base, model: "vertex/byok" })).not.toThrow(); - }); + it("throws when neither Anthropic credential is set", () => { + expect(() => + validateAgentApiKey({ + agent: claude, + model: "anthropic/claude-opus-4-7", + authorized: new Set(), + owner, + name, + }) + ).toThrow("no API key found"); + }); +}); - it("throws when VERTEX_MODEL_ID is missing", () => { - process.env.VERTEX_SERVICE_ACCOUNT_JSON = "{}"; - process.env.GOOGLE_CLOUD_PROJECT = "test-project"; - process.env.VERTEX_LOCATION = "us-east5"; - expect(() => validateAgentApiKey({ ...base, model: "vertex/byok" })).toThrow( - "VERTEX_MODEL_ID" - ); - }); +describe("validateAgentApiKey — Bedrock routing", () => { + const params = { agent: opencode, authorized: new Set(), owner, name }; - it("throws when VERTEX_LOCATION is missing", () => { - process.env.VERTEX_SERVICE_ACCOUNT_JSON = "{}"; - process.env.GOOGLE_CLOUD_PROJECT = "test-project"; - process.env.VERTEX_MODEL_ID = "claude-opus-4-1@20250805"; - expect(() => validateAgentApiKey({ ...base, model: "vertex/byok" })).toThrow( - "VERTEX_LOCATION" - ); - }); + it("passes with AWS_BEARER_TOKEN_BEDROCK + AWS_REGION + BEDROCK_MODEL_ID", () => { + process.env.AWS_BEARER_TOKEN_BEDROCK = "bedrock-token"; + process.env.AWS_REGION = "us-east-1"; + process.env.BEDROCK_MODEL_ID = "us.anthropic.claude-opus-4-7"; + expect(() => validateAgentApiKey({ ...params, model: "bedrock/byok" })).not.toThrow(); + }); - it("throws when GOOGLE_CLOUD_PROJECT is missing and not derivable", () => { - process.env.VERTEX_SERVICE_ACCOUNT_JSON = "{}"; - process.env.VERTEX_LOCATION = "us-east5"; - process.env.VERTEX_MODEL_ID = "claude-opus-4-1@20250805"; - expect(() => validateAgentApiKey({ ...base, model: "vertex/byok" })).toThrow( - "GOOGLE_CLOUD_PROJECT" - ); - }); + it("passes with AWS access keys + region + model id", () => { + process.env.AWS_ACCESS_KEY_ID = "AKIA-test"; + process.env.AWS_SECRET_ACCESS_KEY = "secret-test"; + process.env.AWS_REGION = "us-east-1"; + process.env.BEDROCK_MODEL_ID = "amazon.nova-pro-v1:0"; + expect(() => validateAgentApiKey({ ...params, model: "bedrock/byok" })).not.toThrow(); + }); - it("throws when no auth path is set", () => { - process.env.GOOGLE_CLOUD_PROJECT = "test-project"; - process.env.VERTEX_LOCATION = "us-east5"; - process.env.VERTEX_MODEL_ID = "claude-opus-4-1@20250805"; - expect(() => validateAgentApiKey({ ...base, model: "vertex/byok" })).toThrow( - "VERTEX_SERVICE_ACCOUNT_JSON" - ); - }); + it("throws when BEDROCK_MODEL_ID is missing", () => { + process.env.AWS_BEARER_TOKEN_BEDROCK = "bedrock-token"; + process.env.AWS_REGION = "us-east-1"; + expect(() => validateAgentApiKey({ ...params, model: "bedrock/byok" })).toThrow( + "BEDROCK_MODEL_ID" + ); + }); - it("accepts a raw Vertex model ID (post-resolveModel) without throwing", () => { - process.env.VERTEX_SERVICE_ACCOUNT_JSON = "{}"; - process.env.GOOGLE_CLOUD_PROJECT = "test-project"; - process.env.VERTEX_LOCATION = "us-east5"; - process.env.VERTEX_MODEL_ID = "gemini-2.5-pro"; - expect(() => validateAgentApiKey({ ...base, model: "gemini-2.5-pro" })).not.toThrow(); - }); + // regression: main.ts passes the post-resolveModel value into + // validateAgentApiKey, which for Bedrock is the raw AWS model ID (no `/`). + it("accepts a raw Bedrock model ID without throwing", () => { + process.env.AWS_BEARER_TOKEN_BEDROCK = "bedrock-token"; + process.env.AWS_REGION = "us-east-1"; + process.env.BEDROCK_MODEL_ID = "us.anthropic.claude-opus-4-6-v1"; + expect(() => + validateAgentApiKey({ ...params, model: "us.anthropic.claude-opus-4-6-v1" }) + ).not.toThrow(); + }); - it("throws on raw Vertex model ID when auth is missing", () => { - process.env.GOOGLE_CLOUD_PROJECT = "test-project"; - process.env.VERTEX_LOCATION = "us-east5"; - process.env.VERTEX_MODEL_ID = "gemini-2.5-pro"; - expect(() => validateAgentApiKey({ ...base, model: "gemini-2.5-pro" })).toThrow( - "VERTEX_SERVICE_ACCOUNT_JSON" - ); - }); + it("throws on raw Bedrock model ID when AWS auth is missing", () => { + process.env.AWS_REGION = "us-east-1"; + process.env.BEDROCK_MODEL_ID = "us.anthropic.claude-opus-4-6-v1"; + expect(() => + validateAgentApiKey({ ...params, model: "us.anthropic.claude-opus-4-6-v1" }) + ).toThrow("AWS_BEARER_TOKEN_BEDROCK"); + }); +}); + +describe("validateAgentApiKey — Vertex routing", () => { + const params = { agent: opencode, authorized: new Set(), owner, name }; + + it("passes with service-account JSON + project + location + model id", () => { + process.env.VERTEX_SERVICE_ACCOUNT_JSON = "{}"; + process.env.GOOGLE_CLOUD_PROJECT = "test-project"; + process.env.VERTEX_LOCATION = "us-east5"; + process.env.VERTEX_MODEL_ID = "claude-opus-4-1@20250805"; + expect(() => validateAgentApiKey({ ...params, model: "vertex/byok" })).not.toThrow(); + }); + + it("throws when VERTEX_MODEL_ID is missing", () => { + process.env.VERTEX_SERVICE_ACCOUNT_JSON = "{}"; + process.env.GOOGLE_CLOUD_PROJECT = "test-project"; + process.env.VERTEX_LOCATION = "us-east5"; + expect(() => validateAgentApiKey({ ...params, model: "vertex/byok" })).toThrow( + "VERTEX_MODEL_ID" + ); + }); + + it("accepts a raw Vertex model ID without throwing", () => { + process.env.VERTEX_SERVICE_ACCOUNT_JSON = "{}"; + process.env.GOOGLE_CLOUD_PROJECT = "test-project"; + process.env.VERTEX_LOCATION = "us-east5"; + process.env.VERTEX_MODEL_ID = "gemini-2.5-pro"; + expect(() => validateAgentApiKey({ ...params, model: "gemini-2.5-pro" })).not.toThrow(); + }); + + it("throws on raw Vertex model ID when auth is missing", () => { + process.env.GOOGLE_CLOUD_PROJECT = "test-project"; + process.env.VERTEX_LOCATION = "us-east5"; + process.env.VERTEX_MODEL_ID = "gemini-2.5-pro"; + expect(() => validateAgentApiKey({ ...params, model: "gemini-2.5-pro" })).toThrow( + "VERTEX_SERVICE_ACCOUNT_JSON" + ); }); }); @@ -249,8 +225,7 @@ describe("isApiKeyAuthError", () => { // see #782 — direct-Anthropic 401 shape (revoked / mistyped / rotated // ANTHROPIC_API_KEY) reaches us via Claude CLI as a JSON dump, not as - // any of the canonical "Invalid API key" strings. these matchers ensure - // the formatted CTA fires instead of the raw 401 JSON blob. + // any of the canonical "Invalid API key" strings. it("matches direct-Anthropic 401 shapes", () => { expect( isApiKeyAuthError( diff --git a/utils/apiKeys.ts b/utils/apiKeys.ts index febe59a..cd2da09 100644 --- a/utils/apiKeys.ts +++ b/utils/apiKeys.ts @@ -1,10 +1,4 @@ -import { - BEDROCK_MODEL_ID_ENV, - getModelEnvVars, - providers, - resolveDisplayAlias, - VERTEX_MODEL_ID_ENV, -} from "../models.ts"; +import { BEDROCK_MODEL_ID_ENV, resolveDisplayAlias, VERTEX_MODEL_ID_ENV } from "../models.ts"; import { getApiUrl } from "./apiUrl.ts"; import { GOOGLE_CLOUD_PROJECT_ENV, @@ -13,10 +7,6 @@ import { VERTEX_SERVICE_ACCOUNT_JSON_ENV, } from "./vertex.ts"; -const knownApiKeys: Set = new Set( - Object.values(providers).flatMap((p) => [...p.envVars, ...(p.managedCredentials ?? [])]) -); - /** marker prefix on the throw message for the catch-side reclassification path */ const MISSING_KEY_MARKER = "no API key found"; @@ -72,13 +62,6 @@ function hasEnvVar(name: string): boolean { return typeof value === "string" && value.length > 0; } -/** check if the user has a BYOK key for the given model's provider (does not throw) */ -export function hasProviderKey(model: string): boolean { - const requiredVars = getModelEnvVars(model); - if (requiredVars.length === 0) return true; - return requiredVars.some((v) => hasEnvVar(v)); -} - function validateBedrockSetup(params: { owner: string; name: string }): void { const hasAuth = hasEnvVar("AWS_BEARER_TOKEN_BEDROCK") || @@ -112,18 +95,24 @@ function validateVertexSetup(params: { owner: string; name: string }): void { } } +/** + * Validate that the resolved model can actually be served by the chosen + * agent. For routing slugs (Bedrock / Vertex) the auth shape is multi-var + * (auth + region/location + model-id) and `opencode models` doesn't catch + * gaps in the latter two — keep dedicated setup validators. For the + * opencode path, the authoritative answer comes from OpenCode's own model + * introspection (`authorized` set captured in `openCodeModels.ts`). For + * the claude path, fall back to the static check (`ANTHROPIC_API_KEY` / + * `CLAUDE_CODE_OAUTH_TOKEN`). + */ export function validateAgentApiKey(params: { agent: { name: string }; model: string | undefined; + authorized: Set; owner: string; name: string; }): void { - // if a specific model is configured, only check that model's required env vars if (params.model) { - // routing slugs (e.g. bedrock) get a tailored validation path because - // their auth shape doesn't match the standard "any one envVar present" - // rule (Bedrock needs auth + region + model-id, with auth being either - // a bearer token OR an access-key pair). const alias = resolveDisplayAlias(params.model); if (alias?.routing === "bedrock") { validateBedrockSetup({ owner: params.owner, name: params.name }); @@ -134,13 +123,11 @@ export function validateAgentApiKey(params: { return; } - // upstream `resolveModel` translates routing slugs into raw backend - // model IDs (e.g. `us.anthropic.claude-opus-4-6-v1`), which have no `/` - // and so isn't parseable as `provider/model`. these IDs only reach this - // function via routing aliases, so re-run the matching setup check rather - // than falling through to `getModelEnvVars` (which would throw inside - // parseModel). resolveModel itself already enforced the model-id env var, - // but auth + location/region are still validated here. + // raw backend model IDs (post-resolveModel for routing slugs) have no + // `/`. discriminate by the env-var sentinel, then run the matching + // setup validator — `opencode models` doesn't help here because the + // Bedrock/Vertex provider plugins need region/location/model-id wired + // through env regardless of CLI-side auth. if (!params.model.includes("/")) { if (process.env[VERTEX_MODEL_ID_ENV]?.trim() === params.model) { validateVertexSetup({ owner: params.owner, name: params.name }); @@ -150,19 +137,23 @@ export function validateAgentApiKey(params: { return; } - const requiredVars = getModelEnvVars(params.model); - // free models have no required env vars — skip validation entirely - if (requiredVars.length === 0) return; - if (requiredVars.some((v) => hasEnvVar(v))) return; + if (params.agent.name === "opencode") { + if (params.authorized.has(params.model)) return; + throw new Error(buildMissingApiKeyError({ owner: params.owner, name: params.name })); + } + // claude: single-provider check on the Anthropic auth shapes. + if (hasEnvVar("ANTHROPIC_API_KEY") || hasEnvVar("CLAUDE_CODE_OAUTH_TOKEN")) return; throw new Error(buildMissingApiKeyError({ owner: params.owner, name: params.name })); } - // no model configured — auto-select requires at least one known provider key - const hasAnyKey = [...knownApiKeys].some((k) => hasEnvVar(k)); - if (!hasAnyKey) { + // no model configured (auto-select path). + if (params.agent.name === "opencode") { + if (params.authorized.size > 0) return; throw new Error(buildMissingApiKeyError({ owner: params.owner, name: params.name })); } + if (hasEnvVar("ANTHROPIC_API_KEY") || hasEnvVar("CLAUDE_CODE_OAUTH_TOKEN")) return; + throw new Error(buildMissingApiKeyError({ owner: params.owner, name: params.name })); } /** diff --git a/utils/byokFallback.test.ts b/utils/byokFallback.test.ts index 450c2d0..73e0184 100644 --- a/utils/byokFallback.test.ts +++ b/utils/byokFallback.test.ts @@ -1,4 +1,4 @@ -import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { describe, expect, it } from "vitest"; import { resolveCliModel } from "../models.ts"; import { FREE_FALLBACK_SLUG, selectFallbackModelIfNeeded } from "./byokFallback.ts"; @@ -13,34 +13,13 @@ describe("FREE_FALLBACK_SLUG", () => { }); describe("selectFallbackModelIfNeeded", () => { - const originalEnv = { ...process.env }; - const KEYS = [ - "ANTHROPIC_API_KEY", - "CLAUDE_CODE_OAUTH_TOKEN", - "OPENAI_API_KEY", - "OPENROUTER_API_KEY", - "GEMINI_API_KEY", - "GOOGLE_GENERATIVE_AI_API_KEY", - "XAI_API_KEY", - "DEEPSEEK_API_KEY", - "MOONSHOT_API_KEY", - "OPENCODE_API_KEY", - ] as const; + const empty = new Set(); - beforeEach(() => { - for (const k of KEYS) delete process.env[k]; - }); - afterEach(() => { - for (const k of KEYS) { - if (originalEnv[k] === undefined) delete process.env[k]; - else process.env[k] = originalEnv[k]; - } - }); - - it("falls back when the resolved model needs a key that isn't set", () => { + it("falls back when the resolved model is not in OpenCode's authorized set", () => { const result = selectFallbackModelIfNeeded({ resolvedModel: "anthropic/claude-opus-4-7", proxyModel: undefined, + authorized: empty, }); expect(result).toEqual({ fallback: true, @@ -49,11 +28,11 @@ describe("selectFallbackModelIfNeeded", () => { }); }); - it("does not fall back when the resolved model's key IS set", () => { - process.env.ANTHROPIC_API_KEY = "sk-test"; + it("does not fall back when the resolved model IS authorized", () => { const result = selectFallbackModelIfNeeded({ resolvedModel: "anthropic/claude-opus-4-7", proxyModel: undefined, + authorized: new Set(["anthropic/claude-opus-4-7"]), }); expect(result.fallback).toBe(false); }); @@ -62,6 +41,7 @@ describe("selectFallbackModelIfNeeded", () => { const result = selectFallbackModelIfNeeded({ resolvedModel: undefined, proxyModel: "openrouter/anthropic/claude-opus-4.7", + authorized: empty, }); expect(result.fallback).toBe(false); }); @@ -70,6 +50,7 @@ describe("selectFallbackModelIfNeeded", () => { const result = selectFallbackModelIfNeeded({ resolvedModel: undefined, proxyModel: undefined, + authorized: empty, }); expect(result.fallback).toBe(false); }); @@ -78,26 +59,20 @@ describe("selectFallbackModelIfNeeded", () => { const result = selectFallbackModelIfNeeded({ resolvedModel: FREE_FALLBACK_SLUG, proxyModel: undefined, + authorized: empty, }); expect(result.fallback).toBe(false); }); it("does not fall back for Bedrock routing (raw model ID has no slash)", () => { // resolveModel({slug:"bedrock/byok"}) returns the raw BEDROCK_MODEL_ID - // value (e.g. "us.anthropic.claude-opus-4-7"), which has no `/`. without - // a guard, hasProviderKey → parseModel would throw and crash the action - // before validateBedrockSetup can surface its tailored error. + // value (e.g. "us.anthropic.claude-opus-4-7"), which has no `/`. the + // routing validator (validateBedrockSetup) owns auth + region + model-id + // checking for this path, not the BYOK fallback gate. const result = selectFallbackModelIfNeeded({ resolvedModel: "us.anthropic.claude-opus-4-7", proxyModel: undefined, - }); - expect(result.fallback).toBe(false); - }); - - it("does not fall back for free models that need no key", () => { - const result = selectFallbackModelIfNeeded({ - resolvedModel: "opencode/mimo-v2-pro-free", - proxyModel: undefined, + authorized: empty, }); expect(result.fallback).toBe(false); }); @@ -106,16 +81,8 @@ describe("selectFallbackModelIfNeeded", () => { const result = selectFallbackModelIfNeeded({ resolvedModel: resolveCliModel("opencode/minimax-m2.5-free"), proxyModel: undefined, + authorized: empty, }); 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({ - resolvedModel: "anthropic/claude-opus-4-7", - proxyModel: undefined, - }); - expect(result.fallback).toBe(true); - }); }); diff --git a/utils/byokFallback.ts b/utils/byokFallback.ts index 46f11aa..40cf22a 100644 --- a/utils/byokFallback.ts +++ b/utils/byokFallback.ts @@ -1,9 +1,6 @@ -import { hasProviderKey } from "./apiKeys.ts"; - /** * Slug we fall back to when a BYOK-required model is configured but the - * runner has no provider key in env. Picked because it's free - * (`isFree: true`, `envVars: []` — see `action/models.ts`), stable, and + * runner has no provider key in env. Picked because it's free, stable, and * currently served by OpenCode Zen without a key. * * The slug is intentionally hard-coded and not a config knob — the @@ -16,40 +13,30 @@ export const FREE_FALLBACK_SLUG = "opencode/big-pickle"; export type FallbackDecision = { fallback: false } | { fallback: true; from: string; to: string }; /** - * If the resolved model requires a BYOK key but no provider key is - * available in env, return `fallback: true` with a free OpenCode slug - * so the run can still succeed. Caller is responsible for swapping the - * model state and surfacing the fallback (log line + run summary). + * If the resolved model is NOT in OpenCode's `authorized` set (the + * authoritative "what can OpenCode route right now" snapshot captured + * after dbSecrets + Codex auth.json are in place), swap to a free + * OpenCode slug so the run can still produce value. Caller is responsible + * for surfacing the swap (log line + run summary). * - * Gates on `resolvedModel` directly (not the configured slug) so the - * decision matches both code paths that reach this point: payload-based - * config (`repo.model` from DB) and `PULLFROG_MODEL` env var. Both end - * up in `resolvedModel` after `resolveModel()` runs upstream. - * - * Skip cases: - * - Router / proxy runs (`proxyModel` set): Pullfrog mints the key, - * no BYOK in play — never fall back. - * - No resolved model: keeps the existing auto-select-with-throw - * behavior in `validateAgentApiKey` for the "neither model nor - * key" case (genuine misconfig the user should see). - * - Resolved model is itself the free fallback: avoid suggesting we - * fell back to the model we're already running. - * - Resolved model is a Bedrock raw ID (no `/`): Bedrock has its own - * auth shape (`AWS_BEARER_TOKEN_BEDROCK` + region + model ID), and - * `validateBedrockSetup` already surfaces a tailored error. Skipping - * here also avoids `parseModel`'s slash requirement crashing inside - * `hasProviderKey`. - * - Resolved model has its provider key present: no fallback needed. + * Skip cases (return `fallback: false` without consulting `authorized`): + * - Router / proxy runs (`proxyModel` set): Pullfrog mints the key. + * - No resolved model: auto-select handles it downstream. + * - Resolved model is the free fallback already. + * - Resolved model is a raw Bedrock / Vertex ID (no `/`): the routing + * validators (`validateBedrockSetup` / `validateVertexSetup`) cover + * auth + region/location/model-id; `opencode models` does not. */ export function selectFallbackModelIfNeeded(input: { resolvedModel: string | undefined; proxyModel: string | undefined; + authorized: Set; }): FallbackDecision { if (input.proxyModel) return { fallback: false }; if (!input.resolvedModel) return { fallback: false }; if (input.resolvedModel === FREE_FALLBACK_SLUG) return { fallback: false }; if (!input.resolvedModel.includes("/")) return { fallback: false }; - if (hasProviderKey(input.resolvedModel)) return { fallback: false }; + if (input.authorized.has(input.resolvedModel)) return { fallback: false }; return { fallback: true, from: input.resolvedModel, diff --git a/utils/lifecycle.ts b/utils/lifecycle.ts index 6112333..ed51d47 100644 --- a/utils/lifecycle.ts +++ b/utils/lifecycle.ts @@ -10,6 +10,18 @@ import { export interface ExecuteLifecycleHookParams { event: string; script: string | null; + /** + * when true, after the hook runs (success or failure), discard tracked-file + * mods so the agent doesn't see hook-generated drift (e.g. `pnpm install` + * rewriting a lockfile). untracked files are preserved — hooks that + * intentionally materialize files (e.g. a `.env` from a template) stay + * visible to the agent. skipped (with a warning) if the tree had + * pre-existing tracked changes before the hook ran, so we never clobber + * pre-existing work; pre-existing untracked files are ignored for this + * gate because `git restore --staged --worktree .` doesn't touch them + * anyway. no-op when no script was configured. + */ + normalizeWorkingTreeAfter?: boolean; } /** structured failure info — `output` on the `exit` variant is trimmed @@ -51,51 +63,117 @@ export async function executeLifecycleHook( log.info(`» executing ${params.event} lifecycle hook...`); + // snapshot tracked-file mods BEFORE the hook runs so we can distinguish + // hook-generated drift from pre-existing work. both hook windows should + // start clean in normal operation (setup runs before any working-tree + // writes; checkout_pr refuses to run with a dirty tree), but if that + // invariant breaks we'd rather warn than discard whatever was there. + // pre-existing untracked files don't matter here — `git restore --staged + // --worktree .` never touches untracked files, so they're never at risk. + const preHookTrackedCount = params.normalizeWorkingTreeAfter + ? (await runGitLines(["diff", "--name-only", "HEAD"])).length + : 0; + + // single try/finally so normalization fires on success AND failure paths. + // a hook that fails partway through (e.g. `pnpm install` updates the + // lockfile then explodes on a peer-dep conflict) leaves the same kind of + // drift a successful run does, and the agent will see it next regardless + // of which path we took. failure-mode messaging is unchanged; the only + // delta is that we don't return tracked drift to the agent. + let result: LifecycleHookResult; try { - const result = await spawn({ - cmd: "bash", - args: ["-c", params.script], - env: process.env, - timeout: LIFECYCLE_HOOK_TIMEOUT_MS, - activityTimeout: 0, - onStdout: (chunk) => process.stdout.write(chunk), - onStderr: (chunk) => process.stderr.write(chunk), - }); + try { + const spawnResult = await spawn({ + cmd: "bash", + args: ["-c", params.script], + env: process.env, + timeout: LIFECYCLE_HOOK_TIMEOUT_MS, + activityTimeout: 0, + onStdout: (chunk) => process.stdout.write(chunk), + onStderr: (chunk) => process.stderr.write(chunk), + }); - if (result.exitCode !== 0) { - const output = (result.stderr || result.stdout).trim(); - return { - failure: { kind: "exit", output, exitCode: result.exitCode }, - warning: - `lifecycle hook '${params.event}' failed with exit code ${result.exitCode}. ` + - `output: ${output || "(empty)"}. ` + - `retry the operation if the failure looks flaky (network blips, transient rate limits). ` + - `do NOT retry if the script is broken (missing commands, syntax errors) or the error is persistent.`, - }; + if (spawnResult.exitCode !== 0) { + const output = (spawnResult.stderr || spawnResult.stdout).trim(); + result = { + failure: { kind: "exit", output, exitCode: spawnResult.exitCode }, + warning: + `lifecycle hook '${params.event}' failed with exit code ${spawnResult.exitCode}. ` + + `output: ${output || "(empty)"}. ` + + `retry the operation if the failure looks flaky (network blips, transient rate limits). ` + + `do NOT retry if the script is broken (missing commands, syntax errors) or the error is persistent.`, + }; + } else { + log.info(`» ${params.event} lifecycle hook completed successfully`); + result = {}; + } + } catch (err) { + const isTimeout = + err instanceof SpawnTimeoutError && + (err.code === SPAWN_TIMEOUT_CODE || err.code === SPAWN_ACTIVITY_TIMEOUT_CODE); + if (isTimeout) { + const minutes = Math.round(LIFECYCLE_HOOK_TIMEOUT_MS / 60000); + result = { + failure: { kind: "timeout" }, + warning: + `lifecycle hook '${params.event}' timed out after ${minutes}min. ` + + `do NOT retry — the script is likely hung or doing too much work. ` + + `ask the repo owner to simplify the hook (e.g. move long-running work out of the hook, add caching, or split it).`, + }; + } else { + const msg = err instanceof Error ? err.message : String(err); + result = { + failure: { kind: "spawn", spawnError: msg }, + warning: + `lifecycle hook '${params.event}' failed to spawn: ${msg}. ` + + `this is likely a transient failure — retry the operation.`, + }; + } } - - log.info(`» ${params.event} lifecycle hook completed successfully`); - return {}; - } catch (err) { - const isTimeout = - err instanceof SpawnTimeoutError && - (err.code === SPAWN_TIMEOUT_CODE || err.code === SPAWN_ACTIVITY_TIMEOUT_CODE); - if (isTimeout) { - const minutes = Math.round(LIFECYCLE_HOOK_TIMEOUT_MS / 60000); - return { - failure: { kind: "timeout" }, - warning: - `lifecycle hook '${params.event}' timed out after ${minutes}min. ` + - `do NOT retry — the script is likely hung or doing too much work. ` + - `ask the repo owner to simplify the hook (e.g. move long-running work out of the hook, add caching, or split it).`, - }; + } finally { + if (params.normalizeWorkingTreeAfter) { + await normalizeWorkingTreeAfterHook({ event: params.event, preHookTrackedCount }); } - const msg = err instanceof Error ? err.message : String(err); - return { - failure: { kind: "spawn", spawnError: msg }, - warning: - `lifecycle hook '${params.event}' failed to spawn: ${msg}. ` + - `this is likely a transient failure — retry the operation.`, - }; } + return result; +} + +/** + * discard tracked-file mods left by a lifecycle hook so the agent's next + * `git status` matches the pre-hook state. untracked files (e.g. a `.env` + * the hook materialized from a template) are left alone — the agent decides + * what to do with them. skipped (with a warning) when the tree had + * pre-existing tracked changes before the hook ran, so pre-existing work + * is never clobbered. idempotent: a second call on a clean tree is a no-op + * and stays quiet. + */ +async function normalizeWorkingTreeAfterHook(params: { + event: string; + preHookTrackedCount: number; +}): Promise { + if (params.preHookTrackedCount > 0) { + log.warning( + `» working tree had ${params.preHookTrackedCount} pre-existing tracked changes before ${params.event} hook; ` + + `skipping post-hook normalization to avoid clobbering pre-existing work` + ); + return; + } + const trackedCount = (await runGitLines(["diff", "--name-only", "HEAD"])).length; + if (trackedCount === 0) return; + await runGit(["restore", "--staged", "--worktree", "."]); + log.info(`» discarded ${trackedCount} tracked changes from ${params.event} hook`); +} + +async function runGit(args: string[]): Promise { + const result = await spawn({ cmd: "git", args, env: process.env, activityTimeout: 0 }); + if (result.exitCode !== 0) { + throw new Error( + `git ${args.join(" ")} failed (exit ${result.exitCode}): ${result.stderr.trim() || "(no stderr)"}` + ); + } + return result.stdout; +} + +async function runGitLines(args: string[]): Promise { + return (await runGit(args)).split("\n").filter(Boolean); } diff --git a/utils/openCodeModels.ts b/utils/openCodeModels.ts new file mode 100644 index 0000000..c5cc3a2 --- /dev/null +++ b/utils/openCodeModels.ts @@ -0,0 +1,83 @@ +// OpenCode-as-source-of-truth for BYOK detection. +// +// `opencode models` returns the `provider/model` specifiers that OpenCode +// can actually route given the current env (workflow env block + GH Actions +// secrets) and `auth.json` (Codex / future managed credentials). This is +// authoritative — strictly more accurate than the static +// `provider.envVars + provider.managedCredentials` catalog in `models.ts` +// for the "do we have BYOK auth?" gate. The catalog can (and will) miss +// new auth shapes; OpenCode itself can't. +// +// Two captures per run: +// 1. `captureBaselineModels` — called BEFORE Pullfrog-stored credentials +// (dbSecrets + Codex auth.json) land in the env. The set OpenCode can +// serve from the runner's pre-existing environment alone. +// 2. `captureAuthorizedModels` — called AFTER dbSecrets merge + Codex +// auth.json materialization. The authoritative set for BYOK +// decisions (fallback + validateAgentApiKey). +// +// The set difference (`authorized - baseline`) is the contribution of +// Pullfrog-stored auth to this run — logged once for operator visibility +// and reserved for a future server-side "OSS proxy opt-out" detection. +// +// Memoized at module scope so the two consumers +// (`selectFallbackModelIfNeeded` + `autoSelectModel`) share one shell-out. + +import { execFileSync } from "node:child_process"; +import { log } from "./cli.ts"; + +let baseline: Set | undefined; +let authorized: Set | undefined; + +function readModels(cliPath: string): Set { + try { + const output = execFileSync(cliPath, ["models"], { + encoding: "utf-8", + timeout: 30_000, + env: process.env, + }); + return new Set( + output + .split("\n") + .map((line) => line.trim()) + .filter(Boolean) + ); + } catch (error) { + log.debug( + `» \`opencode models\` failed: ${error instanceof Error ? error.message : String(error)}` + ); + return new Set(); + } +} + +/** Snapshot the set of models OpenCode can serve from the current env, BEFORE + * Pullfrog-stored credentials are merged in. Call once early in `main.ts`. */ +export function captureBaselineModels(cliPath: string): void { + baseline = readModels(cliPath); + log.debug(`» opencode baseline: ${baseline.size} models`); +} + +/** Snapshot the set of models OpenCode can serve AFTER dbSecrets + + * Codex auth.json are in place. Logs the diff against the baseline as + * `» BYOK auth enabled N model(s): …`. */ +export function captureAuthorizedModels(cliPath: string): void { + authorized = readModels(cliPath); + const base = baseline; + if (base) { + const diff = [...authorized].filter((m) => !base.has(m)); + if (diff.length > 0) { + log.info(`» BYOK auth enabled ${diff.length} model(s): ${diff.join(", ")}`); + } + } + log.debug(`» opencode authorized: ${authorized.size} models`); +} + +/** Authorized set captured after Pullfrog-stored auth is applied. Throws if + * called before `captureAuthorizedModels` — the call sites (fallback gate, + * api-key validation, auto-select) all run strictly after capture. */ +export function getAuthorizedModels(): Set { + if (!authorized) { + throw new Error("getAuthorizedModels called before captureAuthorizedModels"); + } + return authorized; +} diff --git a/utils/packageManager.ts b/utils/packageManager.ts new file mode 100644 index 0000000..336ae0c --- /dev/null +++ b/utils/packageManager.ts @@ -0,0 +1,228 @@ +import { existsSync } from "node:fs"; +import { readFile } from "node:fs/promises"; +import { join } from "node:path"; +import semver from "semver"; +import { log } from "./cli.ts"; +import { spawn } from "./subprocess.ts"; + +export type SupportedPackageManager = "npm" | "pnpm" | "yarn" | "bun"; + +const SUPPORTED_NAMES: readonly SupportedPackageManager[] = ["npm", "pnpm", "yarn", "bun"]; + +// corepack ships pnpm + yarn shims out of the box. it does not ship bun or +// npm (npm comes with node). callers fall back to the legacy npm-install-g +// path for managers outside this set. +const COREPACK_MANAGED: readonly SupportedPackageManager[] = ["pnpm", "yarn"]; + +export interface PackageManagerSpec { + name: SupportedPackageManager; + /** + * either a concrete semver (e.g. "11.1.1") or a range (e.g. "^11.0.0"). + * `concrete` distinguishes — corepack only accepts concrete versions. + */ + version: string; + concrete: boolean; + /** which package.json field this came from */ + source: "devEngines" | "packageManager"; +} + +interface PackageJson { + packageManager?: string; + devEngines?: { + packageManager?: { + name?: string; + version?: string; + onFail?: string; + }; + }; +} + +function isSupported(name: string): name is SupportedPackageManager { + return (SUPPORTED_NAMES as readonly string[]).includes(name); +} + +function parsePackageManagerField(value: string): PackageManagerSpec | null { + // npm spec form is "name@version[+integrity]" — corepack adds the integrity + // suffix; we strip it because it's not a semver. + const withoutHash = value.split("+")[0]; + const at = withoutHash.lastIndexOf("@"); + if (at <= 0) return null; + const name = withoutHash.slice(0, at); + const version = withoutHash.slice(at + 1); + if (!isSupported(name)) { + log.warning(`» unknown packageManager in package.json: ${value}`); + return null; + } + return { + name, + version, + concrete: semver.valid(version) !== null, + source: "packageManager", + }; +} + +function parseDevEnginesField( + field: NonNullable["packageManager"]> +): PackageManagerSpec | null { + if (!field.name || !field.version) return null; + if (!isSupported(field.name)) { + log.warning(`» unknown devEngines.packageManager.name in package.json: ${field.name}`); + return null; + } + const version = field.version.trim(); + return { + name: field.name, + version, + concrete: semver.valid(version) !== null, + source: "devEngines", + }; +} + +/** + * resolve the project's intended package manager from package.json. precedence + * matches pnpm 11+: `devEngines.packageManager` wins over `packageManager`. + * when both are present, a concrete `packageManager` that satisfies a + * `devEngines` range is preferred (we can pin it via corepack); otherwise + * we warn on disagreement and stick with `devEngines`. + */ +export async function resolvePackageManagerSpec(cwd: string): Promise { + const pkgPath = join(cwd, "package.json"); + if (!existsSync(pkgPath)) return null; + + let pkg: PackageJson; + try { + pkg = JSON.parse(await readFile(pkgPath, "utf-8")) as PackageJson; + } catch (err) { + log.warning( + `» failed to parse package.json for package manager resolution: ${err instanceof Error ? err.message : String(err)}` + ); + return null; + } + + const devSpec = pkg.devEngines?.packageManager + ? parseDevEnginesField(pkg.devEngines.packageManager) + : null; + const pmSpec = pkg.packageManager?.trim() + ? parsePackageManagerField(pkg.packageManager.trim()) + : null; + + if (!devSpec) return pmSpec; + if (!pmSpec) return devSpec; + + if (devSpec.name !== pmSpec.name) { + log.warning( + `» devEngines.packageManager (${devSpec.name}) disagrees with packageManager (${pmSpec.name}); using devEngines per pnpm 11 precedence` + ); + return devSpec; + } + + // same manager — try to land on a concrete version we can pin via corepack. + if (devSpec.concrete) { + if (pmSpec.concrete && devSpec.version !== pmSpec.version) { + log.warning( + `» devEngines.packageManager (${devSpec.version}) disagrees with packageManager (${pmSpec.version}); using devEngines per pnpm 11 precedence` + ); + } + return devSpec; + } + + if (pmSpec.concrete && semver.satisfies(pmSpec.version, devSpec.version)) { + return pmSpec; + } + + if (pmSpec.concrete) { + log.warning( + `» packageManager (${pmSpec.version}) does not satisfy devEngines range (${devSpec.version}); using devEngines` + ); + } + return devSpec; +} + +interface CorepackResult { + exitCode: number; + stderr: string; +} + +async function runCorepack(args: string[]): Promise { + const result = await spawn({ + cmd: "corepack", + args, + env: { + PATH: process.env.PATH || "", + HOME: process.env.HOME || "", + COREPACK_ENABLE_DOWNLOAD_PROMPT: "0", + }, + onStdout: (chunk) => process.stdout.write(chunk), + onStderr: (chunk) => process.stderr.write(chunk), + }); + return { exitCode: result.exitCode, stderr: result.stderr }; +} + +async function currentVersion(name: SupportedPackageManager): Promise { + const result = await spawn({ + cmd: name, + args: ["--version"], + env: { PATH: process.env.PATH || "" }, + }); + if (result.exitCode !== 0) return null; + return result.stdout.trim(); +} + +/** + * ensure the requested package manager is on PATH at the declared version, + * provisioning via corepack when applicable. returns true if PATH now + * resolves to that version, false if we couldn't pin it (in which case + * the caller should treat PATH as untrusted and may fall back to its + * legacy install path). + * + * never throws: network failure, missing corepack, range-only versions — + * all degrade to "log warning, return false". the existing PATH binary + * still works; we just don't get our version guarantee. + */ +export async function ensurePackageManager(spec: PackageManagerSpec): Promise { + if (spec.name === "npm") return true; + + if (!(COREPACK_MANAGED as readonly string[]).includes(spec.name)) { + return false; + } + + if (!spec.concrete) { + log.warning( + `» ${spec.name} ${spec.source} version is a range (${spec.version}); corepack requires a concrete pin. leaving PATH unchanged.` + ); + return false; + } + + const existing = await currentVersion(spec.name); + if (existing === spec.version) { + log.info(`» ${spec.name}@${spec.version} already active`); + return true; + } + + log.info(`» corepack prepare ${spec.name}@${spec.version} --activate`); + + const enable = await runCorepack(["enable"]); + if (enable.exitCode !== 0) { + log.warning( + `» corepack enable failed (exit ${enable.exitCode}); leaving ${spec.name} from PATH. stderr: ${enable.stderr.trim() || "(empty)"}` + ); + return false; + } + + const prepare = await runCorepack(["prepare", `${spec.name}@${spec.version}`, "--activate"]); + if (prepare.exitCode !== 0) { + log.warning( + `» corepack prepare ${spec.name}@${spec.version} failed (exit ${prepare.exitCode}); leaving ${spec.name} from PATH. stderr: ${prepare.stderr.trim() || "(empty)"}` + ); + return false; + } + + const after = await currentVersion(spec.name); + if (after !== spec.version) { + log.warning( + `» corepack activated ${spec.name}@${spec.version} but PATH still resolves to ${after ?? "(missing)"}; continuing anyway` + ); + } + + return true; +}