From bd11416d80c31449e5d66517bd4323b1ae4c29a3 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Thu, 13 Aug 2026 10:34:25 -0700 Subject: [PATCH] fix(evals): absorb codex/gemini CLI drift; external-service tests go periodic-tier MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - codex exec gains --skip-git-repo-check: newer CLIs refuse exec in an untrusted non-git dir (our temp skill dirs) — empirically verified. - gemini: --skip-trust was removed in gemini-cli 0.34 (argv parse error); dropped from the session runner and the benchmark adapter. A present- but-unusable CLI (deprecated individual code-assist auth path) now classifies as SKIP, not a false adapter failure; the benchmark live smoke skips on auth/rate_limit error codes (environmental) while still failing on timeout/unknown (the drift classes it exists to catch). - codex-e2e, gemini-e2e, and benchmark-providers gain the canonical whole-file EVALS_TIER === 'periodic' guard per CLAUDE.md tiering rule 3 (external service -> periodic) — the sharded gate runner now excludes all three (gate: 45 -> 42 shards). Co-Authored-By: Claude Fable 5 --- test/codex-e2e.test.ts | 13 ++++++-- test/gemini-e2e.test.ts | 28 ++++++++++++++-- test/helpers/codex-session-runner.ts | 8 +++-- test/helpers/gemini-session-runner.ts | 38 ++++++++++++++++++---- test/helpers/providers/gemini.ts | 15 ++++----- test/skill-e2e-benchmark-providers.test.ts | 19 ++++++++++- 6 files changed, 100 insertions(+), 21 deletions(-) diff --git a/test/codex-e2e.test.ts b/test/codex-e2e.test.ts index 2f2817f90..e2f33b110 100644 --- a/test/codex-e2e.test.ts +++ b/test/codex-e2e.test.ts @@ -37,15 +37,24 @@ const CODEX_AVAILABLE = (() => { const evalsEnabled = !!process.env.EVALS; -// Skip all tests if codex is not available or EVALS is not set. +// External-service tests are periodic-tier (CLAUDE.md tiering rule 3): +// "Requires external service (Codex, Gemini)? -> periodic". The positive +// form below is the canonical whole-file guard shape — the sharded runner's +// classifyPaidTestFile greps for it to exclude this file from gate. +const tierOk = process.env.EVALS_TIER === 'periodic'; + +// Skip all tests if codex is not available, EVALS is not set, or we're in +// the gate tier. // Note: Codex uses its own auth from ~/.codex/ config — no OPENAI_API_KEY env var needed. -const SKIP = !CODEX_AVAILABLE || !evalsEnabled; +const SKIP = !CODEX_AVAILABLE || !evalsEnabled || !tierOk; const describeCodex = SKIP ? describe.skip : describe; // Log why we're skipping (helpful for debugging CI) if (!evalsEnabled) { // Silent — same as Claude E2E tests, EVALS=1 required +} else if (!tierOk) { + process.stderr.write('\nCodex E2E: SKIPPED — external-service test, periodic tier only (EVALS_TIER === \'periodic\')\n'); } else if (!CODEX_AVAILABLE) { process.stderr.write('\nCodex E2E: SKIPPED — codex binary not found (install: npm i -g @openai/codex)\n'); } diff --git a/test/gemini-e2e.test.ts b/test/gemini-e2e.test.ts index 307665ee6..afbb07f83 100644 --- a/test/gemini-e2e.test.ts +++ b/test/gemini-e2e.test.ts @@ -33,18 +33,42 @@ const GEMINI_AVAILABLE = (() => { } catch { return false; } })(); +// A binary on PATH is not enough: the CLI can be present but UNUSABLE — the +// individual code-assist auth path was deprecated upstream ("migrate to the +// Antigravity suite"), which fails every run before any model call, and flag +// churn (--skip-trust removed in 0.34) errors at argv parse. Probe with a +// bare --help: a CLI that can't even print usage is unusable, and a working +// one is cheap to confirm. Deeper auth failures are classified per-run. +const GEMINI_USABLE = GEMINI_AVAILABLE && (() => { + try { + const result = Bun.spawnSync(['gemini', '--help'], { timeout: 15_000 }); + return result.exitCode === 0; + } catch { return false; } +})(); + const evalsEnabled = !!process.env.EVALS; -// Skip all tests if gemini is not available or EVALS is not set. -const SKIP = !GEMINI_AVAILABLE || !evalsEnabled; +// External-service tests are periodic-tier (CLAUDE.md tiering rule 3): +// "Requires external service (Codex, Gemini)? -> periodic". The positive +// form below is the canonical whole-file guard shape — the sharded runner's +// classifyPaidTestFile greps for it to exclude this file from gate. +const tierOk = process.env.EVALS_TIER === 'periodic'; + +// Skip all tests if gemini is not available/usable, EVALS is not set, or +// we're in the gate tier. +const SKIP = !GEMINI_USABLE || !evalsEnabled || !tierOk; const describeGemini = SKIP ? describe.skip : describe; // Log why we're skipping (helpful for debugging CI) if (!evalsEnabled) { // Silent — same as Claude E2E tests, EVALS=1 required +} else if (!tierOk) { + process.stderr.write('\nGemini E2E: SKIPPED — external-service test, periodic tier only (EVALS_TIER === \'periodic\')\n'); } else if (!GEMINI_AVAILABLE) { process.stderr.write('\nGemini E2E: SKIPPED — gemini binary not found (install: npm i -g @google/gemini-cli)\n'); +} else if (!GEMINI_USABLE) { + process.stderr.write('\nGemini E2E: SKIPPED — gemini CLI present but unusable (auth path deprecated upstream or CLI broken; try updating @google/gemini-cli)\n'); } // --- Diff-based test selection --- diff --git a/test/helpers/codex-session-runner.ts b/test/helpers/codex-session-runner.ts index 404aa6bb2..6aa2ba74f 100644 --- a/test/helpers/codex-session-runner.ts +++ b/test/helpers/codex-session-runner.ts @@ -199,8 +199,12 @@ export async function runCodexSkill(opts: { } } - // Build codex exec command - const args = ['exec', prompt, '--json', '-s', sandbox]; + // Build codex exec command. + // --skip-git-repo-check: newer codex CLIs refuse exec in an untrusted + // non-git directory ("Not inside a trusted directory and + // --skip-git-repo-check was not specified") — our temp skill dirs are + // exactly that. Empirically verified against codex on this machine. + const args = ['exec', prompt, '--json', '-s', sandbox, '--skip-git-repo-check']; // Spawn codex with temp HOME so it discovers our installed skill. // Hermetic scrub (test/helpers/hermetic-env.ts) with codex's auth surface diff --git a/test/helpers/gemini-session-runner.ts b/test/helpers/gemini-session-runner.ts index 3b58c79ca..2b080265d 100644 --- a/test/helpers/gemini-session-runner.ts +++ b/test/helpers/gemini-session-runner.ts @@ -8,8 +8,8 @@ * Key differences from Codex session-runner: * - Uses `gemini -p` instead of `codex exec` * - Output is NDJSON with event types: init, message, tool_use, tool_result, result - * - Uses `--output-format stream-json --yolo --skip-trust` instead of `--json -s read-only` - * (`--skip-trust` required for headless/untrusted cwds; see gemini trusted-folders docs) + * - Uses `--output-format stream-json --yolo` instead of `--json -s read-only` + * (`--skip-trust` was removed in gemini-cli 0.34; folder trust is settings-driven now) * - No temp HOME needed — Gemini discovers skills from `.agents/skills/` in cwd * - Message events are streamed with `delta: true` — must concatenate */ @@ -121,10 +121,11 @@ export async function runGeminiSkill(opts: { }; } - // Build gemini command - // --skip-trust: headless/CI and temp cwds aren't in ~/.gemini/trustedFolders.json; - // without it gemini exits FatalUntrustedWorkspaceError before any model call. - const args = ['-p', prompt, '--output-format', 'stream-json', '--yolo', '--skip-trust']; + // Build gemini command. + // --skip-trust was REMOVED in gemini-cli 0.34 ("Unknown arguments: + // skip-trust"); folder trust moved to settings and no longer needs a flag + // for headless runs. --yolo still auto-approves tool actions. + const args = ['-p', prompt, '--output-format', 'stream-json', '--yolo']; // Spawn gemini — uses real HOME for auth (~/.gemini; HOME is allowlisted), // cwd for skill discovery. Hermetic scrub with gemini's auth surface @@ -198,6 +199,31 @@ export async function runGeminiSkill(opts: { process.stderr.write(` [gemini stderr] ${stderr.trim().slice(0, 200)}\n`); } + // Environment-unusable classification: these are Google-side conditions no + // test assertion can act on — the deprecated individual code-assist auth + // path ("migrate to the Antigravity suite") and argv drift on older/newer + // CLIs. Return the same SKIP shape as binary-not-found so callers report + // SKIPPED instead of a false FAIL. + const unusableMarkers = [ + 'no longer supported for Gemini Code Assist', + 'antigravity', + 'Unknown arguments: skip-trust', + ]; + if (exitCode !== 0 && parsed.tokens === 0) { + const marker = unusableMarkers.find((m) => stderr.toLowerCase().includes(m.toLowerCase())); + if (marker) { + return { + output: `SKIP: gemini CLI unusable (${marker})`, + toolCalls: [], + tokens: 0, + exitCode: -1, + durationMs, + sessionId: null, + rawLines: collectedLines, + }; + } + } + return { output: parsed.output, toolCalls: parsed.toolCalls, diff --git a/test/helpers/providers/gemini.ts b/test/helpers/providers/gemini.ts index b53d725d1..e5d967f85 100644 --- a/test/helpers/providers/gemini.ts +++ b/test/helpers/providers/gemini.ts @@ -103,10 +103,10 @@ export function resultFromGeminiStream( * Headless flags always passed: * --output-format stream-json — NDJSON events (message/tool_use/result) * --yolo — auto-approve tools (non-interactive) - * --skip-trust — trust cwd for this session; required when - * workdir is a temp/untrusted folder (benchmarks - * use mkdtemp). Without it headless gemini exits - * before calling the model. + * + * --skip-trust is gone: gemini-cli 0.34 removed the flag ("Unknown arguments: + * skip-trust") — folder trust is settings-driven now and headless runs no + * longer need a flag for temp workdirs. */ export class GeminiAdapter implements ProviderAdapter { readonly name = 'gemini'; @@ -136,10 +136,9 @@ export class GeminiAdapter implements ProviderAdapter { async run(opts: RunOpts): Promise { const start = Date.now(); // Default to --yolo (non-interactive) and stream-json output so we can parse - // tokens + tool calls. --skip-trust is required for headless/temp workdirs - // (gemini CLI otherwise exits: "not running in a trusted directory"). - // Callers can override via extraArgs. - const args = ['-p', opts.prompt, '--output-format', 'stream-json', '--yolo', '--skip-trust']; + // tokens + tool calls. Callers can override via extraArgs. (--skip-trust was + // removed in gemini-cli 0.34; passing it errors at argv parse.) + const args = ['-p', opts.prompt, '--output-format', 'stream-json', '--yolo']; if (opts.model) args.push('--model', opts.model); if (opts.extraArgs) args.push(...opts.extraArgs); diff --git a/test/skill-e2e-benchmark-providers.test.ts b/test/skill-e2e-benchmark-providers.test.ts index 98c2f3e03..10395dea5 100644 --- a/test/skill-e2e-benchmark-providers.test.ts +++ b/test/skill-e2e-benchmark-providers.test.ts @@ -30,7 +30,15 @@ import * as os from 'os'; // --- Prerequisites / gating --- const evalsEnabled = !!process.env.EVALS; -const describeIfEvals = evalsEnabled ? describe : describe.skip; +// External-service tests are periodic-tier (CLAUDE.md tiering rule 3) — +// the header above says so, but without a whole-file guard the sharded gate +// runner still selects this file into gate. The positive form below is the +// canonical guard shape classifyPaidTestFile greps for. +const tierOk = process.env.EVALS_TIER === 'periodic'; +const describeIfEvals = evalsEnabled && tierOk ? describe : describe.skip; +if (evalsEnabled && !tierOk) { + process.stderr.write('\nbenchmark-providers: SKIPPED — external-service test, periodic tier only\n'); +} const PROMPT = 'Reply with exactly this text and nothing else: ok'; @@ -127,6 +135,15 @@ describeIfEvals('multi-provider benchmark adapters (live)', () => { } const result = await gemini.run({ prompt: PROMPT, workdir, timeoutMs: 120_000 }); if (result.error) { + // auth / rate_limit are ENVIRONMENT conditions the test can't act on + // (e.g. Google deprecated the individual code-assist auth path — the + // adapter classifies "no longer supported" as auth). A live smoke + // reports them as a skip, not a false adapter failure. timeout/unknown + // still fail: those are the drift classes this test exists to catch. + if (result.error.code === 'auth' || result.error.code === 'rate_limit') { + process.stderr.write(`\ngemini live smoke: SKIPPED — ${result.error.code}: ${result.error.reason.slice(0, 160)}\n`); + return; + } throw new Error(`gemini errored: ${result.error.code} — ${result.error.reason}`); } // Adapter must never report empty-success (#2159). After content/stats