mirror of https://github.com/garrytan/gstack.git
test(evals): codex runner gains sections option — review variant 88% smaller
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 <noreply@anthropic.com>
This commit is contained in:
parent
9963deac37
commit
5338834654
|
|
@ -16,6 +16,7 @@
|
||||||
import { describe, test, expect, beforeAll, afterAll } from 'bun:test';
|
import { describe, test, expect, beforeAll, afterAll } from 'bun:test';
|
||||||
import { runCodexSkill, parseCodexJSONL, installSkillToTempHome } from './helpers/codex-session-runner';
|
import { runCodexSkill, parseCodexJSONL, installSkillToTempHome } from './helpers/codex-session-runner';
|
||||||
import type { CodexResult } 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 { EvalCollector } from './helpers/eval-store';
|
||||||
import type { EvalTestEntry } from './helpers/eval-store';
|
import type { EvalTestEntry } from './helpers/eval-store';
|
||||||
import { selectTests, detectBaseBranch, getChangedFiles, E2E_TOUCHFILES, GLOBAL_TOUCHFILES } from './helpers/touchfiles';
|
import { selectTests, detectBaseBranch, getChangedFiles, E2E_TOUCHFILES, GLOBAL_TOUCHFILES } from './helpers/touchfiles';
|
||||||
|
|
@ -139,7 +140,11 @@ describeCodex('Codex E2E', () => {
|
||||||
});
|
});
|
||||||
|
|
||||||
testIfSelected('codex-discover-skill', async () => {
|
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 skillDir = path.join(testWorktree, '.agents', 'skills', 'gstack-review');
|
||||||
|
|
||||||
const result = await runCodexSkill({
|
const result = await runCodexSkill({
|
||||||
|
|
@ -172,7 +177,10 @@ describeCodex('Codex E2E', () => {
|
||||||
// code review, and produce structured review output with findings/issues.
|
// 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.
|
// Accepts Codex timeout (exit 124/137) as non-failure since that's a CLI perf issue.
|
||||||
testIfSelected('codex-review-findings', async () => {
|
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 skillDir = path.join(testWorktree, '.agents', 'skills', 'gstack-review');
|
||||||
|
|
||||||
const result = await runCodexSkill({
|
const result = await runCodexSkill({
|
||||||
|
|
@ -181,6 +189,7 @@ describeCodex('Codex E2E', () => {
|
||||||
timeoutMs: 540_000,
|
timeoutMs: 540_000,
|
||||||
cwd: testWorktree,
|
cwd: testWorktree,
|
||||||
skillName: 'gstack-review',
|
skillName: 'gstack-review',
|
||||||
|
sections: CODEX_REVIEW_E2E_SECTIONS,
|
||||||
});
|
});
|
||||||
|
|
||||||
logCodexCost('codex-review-findings', result);
|
logCodexCost('codex-review-findings', result);
|
||||||
|
|
|
||||||
|
|
@ -16,6 +16,7 @@ import * as fs from 'fs';
|
||||||
import * as path from 'path';
|
import * as path from 'path';
|
||||||
import * as os from 'os';
|
import * as os from 'os';
|
||||||
import { hermeticChildEnv } from './hermetic-env';
|
import { hermeticChildEnv } from './hermetic-env';
|
||||||
|
import { extractSkillSections } from './skill-fixture';
|
||||||
|
|
||||||
// --- Interfaces ---
|
// --- Interfaces ---
|
||||||
|
|
||||||
|
|
@ -103,19 +104,32 @@ export function parseCodexJSONL(lines: string[]): ParsedCodexJSONL {
|
||||||
* Creates ~/.codex/skills/{skillName}/SKILL.md in the temp HOME and copies
|
* 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.
|
* 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 `## <section>` 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.
|
* Returns the temp HOME path. Caller is responsible for cleanup.
|
||||||
*/
|
*/
|
||||||
export function installSkillToTempHome(
|
export function installSkillToTempHome(
|
||||||
skillDir: string,
|
skillDir: string,
|
||||||
skillName: string,
|
skillName: string,
|
||||||
tempHome?: string,
|
tempHome?: string,
|
||||||
|
sections?: string[],
|
||||||
): string {
|
): string {
|
||||||
const home = tempHome || fs.mkdtempSync(path.join(os.tmpdir(), 'codex-e2e-'));
|
const home = tempHome || fs.mkdtempSync(path.join(os.tmpdir(), 'codex-e2e-'));
|
||||||
const destDir = path.join(home, '.codex', 'skills', skillName);
|
const destDir = path.join(home, '.codex', 'skills', skillName);
|
||||||
fs.mkdirSync(destDir, { recursive: true });
|
fs.mkdirSync(destDir, { recursive: true });
|
||||||
|
|
||||||
const srcSkill = path.join(skillDir, 'SKILL.md');
|
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'));
|
fs.copyFileSync(srcSkill, path.join(destDir, 'SKILL.md'));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -144,6 +158,7 @@ export async function runCodexSkill(opts: {
|
||||||
cwd?: string; // Working directory
|
cwd?: string; // Working directory
|
||||||
skillName?: string; // Skill name for installation (default: dirname)
|
skillName?: string; // Skill name for installation (default: dirname)
|
||||||
sandbox?: string; // Sandbox mode (default: 'read-only')
|
sandbox?: string; // Sandbox mode (default: 'read-only')
|
||||||
|
sections?: string[]; // Install only these `## <section>` blocks (extract, don't copy)
|
||||||
}): Promise<CodexResult> {
|
}): Promise<CodexResult> {
|
||||||
const {
|
const {
|
||||||
skillDir,
|
skillDir,
|
||||||
|
|
@ -152,6 +167,7 @@ export async function runCodexSkill(opts: {
|
||||||
cwd,
|
cwd,
|
||||||
skillName,
|
skillName,
|
||||||
sandbox = 'read-only',
|
sandbox = 'read-only',
|
||||||
|
sections,
|
||||||
} = opts;
|
} = opts;
|
||||||
|
|
||||||
const startTime = Date.now();
|
const startTime = Date.now();
|
||||||
|
|
@ -178,7 +194,7 @@ export async function runCodexSkill(opts: {
|
||||||
const realHome = os.homedir();
|
const realHome = os.homedir();
|
||||||
|
|
||||||
try {
|
try {
|
||||||
installSkillToTempHome(skillDir, name, tempHome);
|
installSkillToTempHome(skillDir, name, tempHome, sections);
|
||||||
|
|
||||||
// Symlink real Codex auth config so codex can authenticate from temp HOME.
|
// 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
|
// Codex stores auth in ~/.codex/ — we need the config but not the skills
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue