mirror of https://github.com/garrytan/gstack.git
fix: Codex E2E test now validates all skills load without warnings
- Install ALL skills to temp HOME (not just one) to catch missing SKILL.md - Pre-flight asserts every .agents/ dir has both SKILL.md and openai.yaml - Assert no "invalid SKILL.md" or "Skipped loading" in stderr - Add existingHome option to runCodexSkill for pre-populated temp HOMEs - Increase discover test timeout to 120s (all-skills load takes longer)
This commit is contained in:
parent
308f5fc5ad
commit
95d9116003
|
|
@ -120,15 +120,36 @@ afterAll(async () => {
|
||||||
describeCodex('Codex E2E', () => {
|
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
|
// ── Pre-flight: verify .agents/ has both SKILL.md and openai.yaml ──
|
||||||
const skillDir = path.join(ROOT, '.agents', 'skills', 'gstack-review');
|
const agentsDir = path.join(ROOT, '.agents', 'skills');
|
||||||
|
const skillDirs = fs.existsSync(agentsDir)
|
||||||
|
? fs.readdirSync(agentsDir, { withFileTypes: true }).filter(d => d.isDirectory()).map(d => d.name)
|
||||||
|
: [];
|
||||||
|
expect(skillDirs.length).toBeGreaterThan(0); // .agents/ must be generated
|
||||||
|
|
||||||
|
const missingSkillMd: string[] = [];
|
||||||
|
const missingYaml: string[] = [];
|
||||||
|
for (const dir of skillDirs) {
|
||||||
|
if (!fs.existsSync(path.join(agentsDir, dir, 'SKILL.md'))) missingSkillMd.push(dir);
|
||||||
|
if (!fs.existsSync(path.join(agentsDir, dir, 'agents', 'openai.yaml'))) missingYaml.push(dir);
|
||||||
|
}
|
||||||
|
expect(missingSkillMd).toEqual([]); // Every skill dir must have SKILL.md
|
||||||
|
expect(missingYaml).toEqual([]); // Every skill dir must have agents/openai.yaml
|
||||||
|
|
||||||
|
// ── Install ALL skills to temp HOME (like a real install) ──
|
||||||
|
const tempHome = fs.mkdtempSync(path.join(os.tmpdir(), 'codex-e2e-all-'));
|
||||||
|
try {
|
||||||
|
for (const dir of skillDirs) {
|
||||||
|
installSkillToTempHome(path.join(agentsDir, dir), dir, tempHome);
|
||||||
|
}
|
||||||
|
|
||||||
const result = await runCodexSkill({
|
const result = await runCodexSkill({
|
||||||
skillDir,
|
skillDir: path.join(agentsDir, 'gstack-review'),
|
||||||
prompt: 'List any skills or instructions you have available. Just list the names.',
|
prompt: 'List all skills or instructions you have available. List every skill name, one per line.',
|
||||||
timeoutMs: 60_000,
|
timeoutMs: 120_000,
|
||||||
cwd: ROOT,
|
cwd: ROOT,
|
||||||
skillName: 'gstack-review',
|
skillName: 'gstack-review',
|
||||||
|
existingHome: tempHome,
|
||||||
});
|
});
|
||||||
|
|
||||||
logCodexCost('codex-discover-skill', result);
|
logCodexCost('codex-discover-skill', result);
|
||||||
|
|
@ -139,14 +160,21 @@ describeCodex('Codex E2E', () => {
|
||||||
|
|
||||||
expect(result.exitCode).toBe(0);
|
expect(result.exitCode).toBe(0);
|
||||||
expect(result.output.length).toBeGreaterThan(0);
|
expect(result.output.length).toBeGreaterThan(0);
|
||||||
// Skill loading errors mean our generated SKILL.md files are broken
|
|
||||||
expect(result.stderr).not.toContain('invalid');
|
// No skill loading warnings or errors in stderr
|
||||||
|
// (Codex CLI may emit its own PATH warnings in temp HOME — ignore those)
|
||||||
|
expect(result.stderr).not.toContain('invalid SKILL.md');
|
||||||
expect(result.stderr).not.toContain('Skipped loading');
|
expect(result.stderr).not.toContain('Skipped loading');
|
||||||
// The output should reference the skill name in some form
|
expect(result.stderr).not.toContain('invalid skill');
|
||||||
|
|
||||||
|
// The output should reference skills by name
|
||||||
const outputLower = result.output.toLowerCase();
|
const outputLower = result.output.toLowerCase();
|
||||||
expect(
|
expect(
|
||||||
outputLower.includes('review') || outputLower.includes('gstack') || outputLower.includes('skill'),
|
outputLower.includes('review') || outputLower.includes('gstack') || outputLower.includes('skill'),
|
||||||
).toBe(true);
|
).toBe(true);
|
||||||
|
} finally {
|
||||||
|
fs.rmSync(tempHome, { recursive: true, force: true });
|
||||||
|
}
|
||||||
}, 120_000);
|
}, 120_000);
|
||||||
|
|
||||||
// Validates that Codex can invoke the gstack-review skill, run a diff-based
|
// Validates that Codex can invoke the gstack-review skill, run a diff-based
|
||||||
|
|
|
||||||
|
|
@ -143,6 +143,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')
|
||||||
|
existingHome?: string; // Pre-populated temp HOME (skips installSkillToTempHome)
|
||||||
}): Promise<CodexResult> {
|
}): Promise<CodexResult> {
|
||||||
const {
|
const {
|
||||||
skillDir,
|
skillDir,
|
||||||
|
|
@ -151,6 +152,7 @@ export async function runCodexSkill(opts: {
|
||||||
cwd,
|
cwd,
|
||||||
skillName,
|
skillName,
|
||||||
sandbox = 'read-only',
|
sandbox = 'read-only',
|
||||||
|
existingHome,
|
||||||
} = opts;
|
} = opts;
|
||||||
|
|
||||||
const startTime = Date.now();
|
const startTime = Date.now();
|
||||||
|
|
@ -172,12 +174,15 @@ export async function runCodexSkill(opts: {
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
// Set up temp HOME with skill installed
|
// Set up temp HOME with skill installed (or use pre-populated one)
|
||||||
const tempHome = fs.mkdtempSync(path.join(os.tmpdir(), 'codex-e2e-'));
|
const ownedHome = !existingHome;
|
||||||
|
const tempHome = existingHome || fs.mkdtempSync(path.join(os.tmpdir(), 'codex-e2e-'));
|
||||||
const realHome = os.homedir();
|
const realHome = os.homedir();
|
||||||
|
|
||||||
try {
|
try {
|
||||||
installSkillToTempHome(skillDir, name, tempHome);
|
if (ownedHome) {
|
||||||
|
installSkillToTempHome(skillDir, name, tempHome);
|
||||||
|
}
|
||||||
|
|
||||||
// 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
|
||||||
|
|
@ -287,7 +292,9 @@ export async function runCodexSkill(opts: {
|
||||||
stderr,
|
stderr,
|
||||||
};
|
};
|
||||||
} finally {
|
} finally {
|
||||||
// Clean up temp HOME
|
// Clean up temp HOME (only if we created it)
|
||||||
try { fs.rmSync(tempHome, { recursive: true, force: true }); } catch { /* non-fatal */ }
|
if (ownedHome) {
|
||||||
|
try { fs.rmSync(tempHome, { recursive: true, force: true }); } catch { /* non-fatal */ }
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue