Fix Codex skill version metadata (#703)

Move Codex and .agents skill versions under metadata while keeping all version readers compatible with legacy top-level frontmatter.\n\nAI assistance: prepared with Codex under maintainer direction.

tests/context.test.mjs

@@ -1465,11 +1465,11 @@ describe('context.mjs update check', () => {
 
   const cachePath = () => path.join(scratch, 'update-check.json');
 
-  function setup(cacheObj, { disable = false, host } = {}) {
+  function setup(cacheObj, { disable = false, host, skillFrontmatter } = {}) {
     const skillScript = stageContextBundle(path.join(scratch, 'skill', 'scripts'));
     fs.writeFileSync(
       path.join(scratch, 'skill', 'SKILL.md'),
-      `---\nname: impeccable\nversion: ${LOCAL_VERSION}\n---\n\nbody\n`,
+      `---\n${skillFrontmatter || `name: impeccable\nversion: ${LOCAL_VERSION}`}\n---\n\nbody\n`,
     );
     fs.writeFileSync(cachePath(), JSON.stringify(cacheObj));
     const project = path.join(scratch, 'project');
@@ -1522,6 +1522,22 @@ describe('context.mjs update check', () => {
     assert.match(res.stdout, /^# PRODUCT\.md/);
   });
 
+  it('reads metadata.version and prefers it over the legacy top-level key', () => {
+    const metadataOnly = run(
+      { lastCheck: Date.now(), latestVersion: '2.0.0' },
+      { skillFrontmatter: `name: impeccable\nmetadata:\n  version: ${LOCAL_VERSION}` },
+    );
+    assert.equal(metadataOnly.status, 0);
+    assert.match(metadataOnly.stdout, /installed v1\.0\.0, latest v2\.0\.0/);
+
+    const both = run(
+      { lastCheck: Date.now(), latestVersion: '2.0.0' },
+      { skillFrontmatter: `name: impeccable\nversion: 9.0.0\nmetadata:\n  version: ${LOCAL_VERSION}` },
+    );
+    assert.equal(both.status, 0);
+    assert.match(both.stdout, /installed v1\.0\.0, latest v2\.0\.0/);
+  });
+
   // The directive used to say "ask once" and "if they agree, run it" while also
   // saying to continue without waiting. Nothing gated the run on an answer that
   // could not arrive, so the command read as the next step. It now forbids

tests/lib/transformers/providers.test.js

@@ -68,8 +68,14 @@ for (const [key, config] of Object.entries(PROVIDERS)) {
     test('should emit skillsVersion in generated skill frontmatter', () => {
       const skills = [{ name: 'test', description: 'Test', body: 'Body' }];
       transform(skills, TEST_DIR, { skillsVersion: '1.2.3-test' });
-      const parsed = parseFrontmatter(fs.readFileSync(skillPath(config, 'test'), 'utf-8'));
-      expect(parsed.frontmatter.version).toBe('1.2.3-test');
+      const content = fs.readFileSync(skillPath(config, 'test'), 'utf-8');
+      const parsed = parseFrontmatter(content);
+      if (key === 'codex' || key === 'agents') {
+        expect(parsed.frontmatter.version).toBeUndefined();
+        expect(content).toContain('metadata:\n  version: 1.2.3-test');
+      } else {
+        expect(parsed.frontmatter.version).toBe('1.2.3-test');
+      }
     });
 
     // Field-specific tests based on provider config

tests/lib/utils.test.js

@@ -162,6 +162,15 @@ describe('generateYamlFrontmatter', () => {
     expect(result).toContain('user-invocable: true');
   });
 
+  test('should generate nested metadata', () => {
+    const result = generateYamlFrontmatter({
+      name: 'test',
+      metadata: { version: '1.2.3' },
+    });
+
+    expect(result).toContain('metadata:\n  version: 1.2.3');
+  });
+
   test('should roundtrip: generate and parse back', () => {
     const original = {
       name: 'roundtrip-test',

tests/skills-cli.test.js

@@ -24,6 +24,7 @@ import {
   downloadFile,
   expectedHookDests,
   formatInstallDetectionLines,
+  getSkillsVersion,
   mergeHookManifests,
   migrateUnprefixImpeccable,
   resolveInstallTargets,
@@ -169,6 +170,32 @@ function createPrefixedInstall(root, { prefix = 'i-', providers = ['.claude'], f
   }
 }
 
+describe('skills version discovery', () => {
+  test('prefers metadata.version while accepting legacy top-level version', () => {
+    const tmp = mkdtempSync(join(tmpdir(), 'imp-test-skill-version-'));
+    const skillDir = join(tmp, '.agents', 'skills', 'impeccable');
+    mkdirSync(skillDir, { recursive: true });
+    writeFileSync(join(skillDir, 'SKILL.md'), [
+      '---',
+      'name: impeccable',
+      'version: 0.9.0',
+      'metadata:',
+      '  version: 1.2.3',
+      '---',
+      '',
+      'Body.',
+    ].join('\n'));
+
+    try {
+      expect(getSkillsVersion(tmp, 'project')).toBe('1.2.3');
+      writeFileSync(join(skillDir, 'SKILL.md'), '---\nname: impeccable\nversion: 0.9.0\n---\nBody.\n');
+      expect(getSkillsVersion(tmp, 'project')).toBe('0.9.0');
+    } finally {
+      rmSync(tmp, { recursive: true, force: true });
+    }
+  });
+});
+
 // ─── Already-installed detection ─────────────────────────────────────────────
 
 // Remote e2e blocks (real bundle downloads from impeccable.style) run only

tests/validate-plugin-versions.test.js

@@ -157,6 +157,16 @@ describe('readSkillFrontmatterVersion', () => {
     expect(readSkillFrontmatterVersion('---\nversion: "3.7.1"\n---\n')).toBe('3.7.1');
   });
 
+  test('prefers metadata.version while accepting the legacy top-level key', () => {
+    const content = '---\nname: impeccable\nversion: 3.0.0\nmetadata:\n  version: 3.7.1\n---\n';
+    expect(readSkillFrontmatterVersion(content)).toBe('3.7.1');
+  });
+
+  test('reads metadata.version after nested metadata fields', () => {
+    const content = '---\nname: impeccable\nmetadata:\n  interface:\n    display_name: Impeccable\n  version: "3.7.1"\n---\n';
+    expect(readSkillFrontmatterVersion(content)).toBe('3.7.1');
+  });
+
   test('returns null when there is no frontmatter block', () => {
     expect(readSkillFrontmatterVersion('no frontmatter here')).toBeNull();
   });