From 53388346546fd9fd5bc2dce0a1b97f42fd918db4 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Sat, 15 Aug 2026 08:50:03 -0700 Subject: [PATCH] =?UTF-8?q?test(evals):=20codex=20runner=20gains=20section?= =?UTF-8?q?s=20option=20=E2=80=94=20review=20variant=2088%=20smaller?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit runCodexSkill/installSkillToTempHome accept sections?: string[] routed through extractSkillSections; codex-review-findings wired (1465->181 lines). codex-discover-skill deliberately keeps the FULL copy — its stderr assertions validate that the real generated artifact loads. Co-Authored-By: Claude Fable 5 --- test/codex-e2e.test.ts | 13 +++++++++++-- test/helpers/codex-session-runner.ts | 20 ++++++++++++++++++-- 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/test/codex-e2e.test.ts b/test/codex-e2e.test.ts index e2f33b110..0b9c3349a 100644 --- a/test/codex-e2e.test.ts +++ b/test/codex-e2e.test.ts @@ -16,6 +16,7 @@ import { describe, test, expect, beforeAll, afterAll } from 'bun:test'; import { runCodexSkill, parseCodexJSONL, installSkillToTempHome } from './helpers/codex-session-runner'; import type { CodexResult } from './helpers/codex-session-runner'; +import { CODEX_REVIEW_E2E_SECTIONS } from './helpers/skill-fixture'; import { EvalCollector } from './helpers/eval-store'; import type { EvalTestEntry } from './helpers/eval-store'; import { selectTests, detectBaseBranch, getChangedFiles, E2E_TOUCHFILES, GLOBAL_TOUCHFILES } from './helpers/touchfiles'; @@ -139,7 +140,11 @@ describeCodex('Codex E2E', () => { }); testIfSelected('codex-discover-skill', async () => { - // Install gstack-review skill to a temp HOME and ask Codex to list skills + // Install gstack-review skill to a temp HOME and ask Codex to list skills. + // Deliberately installs the FULL generated SKILL.md (no `sections`): this + // test's purpose is to prove the real artifact loads under Codex — the + // stderr assertions below ('invalid' / 'Skipped loading') would be + // meaningless against an extracted fixture. const skillDir = path.join(testWorktree, '.agents', 'skills', 'gstack-review'); const result = await runCodexSkill({ @@ -172,7 +177,10 @@ describeCodex('Codex E2E', () => { // code review, and produce structured review output with findings/issues. // Accepts Codex timeout (exit 124/137) as non-failure since that's a CLI perf issue. testIfSelected('codex-review-findings', async () => { - // Install gstack-review skill and ask Codex to review the worktree + // Install gstack-review and ask Codex to review the worktree. The skill + // fixture is EXTRACTED to the core review-workflow sections — the full + // Codex host variant is ~1460 lines and this test only exercises the + // diff-review flow (CLAUDE.md: "E2E test fixtures: extract, don't copy"). const skillDir = path.join(testWorktree, '.agents', 'skills', 'gstack-review'); const result = await runCodexSkill({ @@ -181,6 +189,7 @@ describeCodex('Codex E2E', () => { timeoutMs: 540_000, cwd: testWorktree, skillName: 'gstack-review', + sections: CODEX_REVIEW_E2E_SECTIONS, }); logCodexCost('codex-review-findings', result); diff --git a/test/helpers/codex-session-runner.ts b/test/helpers/codex-session-runner.ts index 6aa2ba74f..ca66704e0 100644 --- a/test/helpers/codex-session-runner.ts +++ b/test/helpers/codex-session-runner.ts @@ -16,6 +16,7 @@ import * as fs from 'fs'; import * as path from 'path'; import * as os from 'os'; import { hermeticChildEnv } from './hermetic-env'; +import { extractSkillSections } from './skill-fixture'; // --- Interfaces --- @@ -103,19 +104,32 @@ export function parseCodexJSONL(lines: string[]): ParsedCodexJSONL { * Creates ~/.codex/skills/{skillName}/SKILL.md in the temp HOME and copies * agents/openai.yaml when present so Codex sees the same metadata as a real install. * + * When `sections` is provided, the installed SKILL.md is an EXTRACTION + * (frontmatter + the named `##
` blocks via + * test/helpers/skill-fixture.ts) instead of the full 1000-1900-line file — + * CLAUDE.md: "E2E test fixtures: extract, don't copy". Omit `sections` only + * when the test's purpose is to validate the real generated artifact itself + * (e.g., codex-discover-skill asserts the full SKILL.md loads without + * "invalid" / "Skipped loading" stderr from Codex). + * * Returns the temp HOME path. Caller is responsible for cleanup. */ export function installSkillToTempHome( skillDir: string, skillName: string, tempHome?: string, + sections?: string[], ): string { const home = tempHome || fs.mkdtempSync(path.join(os.tmpdir(), 'codex-e2e-')); const destDir = path.join(home, '.codex', 'skills', skillName); fs.mkdirSync(destDir, { recursive: true }); const srcSkill = path.join(skillDir, 'SKILL.md'); - if (fs.existsSync(srcSkill)) { + if (sections && sections.length > 0) { + // extractSkillSections throws loudly on a missing file or renamed + // section — a fixture is never silently written empty. + fs.writeFileSync(path.join(destDir, 'SKILL.md'), extractSkillSections(skillDir, sections)); + } else if (fs.existsSync(srcSkill)) { fs.copyFileSync(srcSkill, path.join(destDir, 'SKILL.md')); } @@ -144,6 +158,7 @@ export async function runCodexSkill(opts: { cwd?: string; // Working directory skillName?: string; // Skill name for installation (default: dirname) sandbox?: string; // Sandbox mode (default: 'read-only') + sections?: string[]; // Install only these `##
` blocks (extract, don't copy) }): Promise { const { skillDir, @@ -152,6 +167,7 @@ export async function runCodexSkill(opts: { cwd, skillName, sandbox = 'read-only', + sections, } = opts; const startTime = Date.now(); @@ -178,7 +194,7 @@ export async function runCodexSkill(opts: { const realHome = os.homedir(); try { - installSkillToTempHome(skillDir, name, tempHome); + installSkillToTempHome(skillDir, name, tempHome, sections); // Symlink real Codex auth config so codex can authenticate from temp HOME. // Codex stores auth in ~/.codex/ — we need the config but not the skills