From f3c06f59df2eaf8b888206312fed2a74cfa6eda6 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Tue, 16 Jun 2026 21:55:49 -0400 Subject: [PATCH] fix(#1326): stop emitting Codex agents/openai.yaml sidecars; clean up stale ones (#1360) * fix(#1326): stop emitting Codex agents/openai.yaml sidecars; clean up stale ones Codex installs wrote an agents/openai.yaml sidecar under every managed gsd-* skill dir. Recent Codex builds index both SKILL.md and the sidecar, so each GSD skill appeared twice in autocomplete (canonical gsd-* name + humanized display_name). - Replace writeCodexSkillMetadataFiles / generateCodexSkillMetadataYaml with cleanupCodexSkillMetadataSidecars: Codex-only (if isCodex), removes stale managed gsd-*/agents/openai.yaml and prunes the now-empty agents/ dir. - Preserve user-owned dirs (gsd-dev-preferences), non-empty agents/ dirs, and non-gsd dirs; lstat-guard against symlinked agents/ so a delete can never escape the skills tree; fail-open per directory. - Codex relies on SKILL.md alone for /skills discovery. - Update USER-GUIDE/FEATURES docs and rewrite the #774 emission tests into cleanup tests. Scope: the sidecar duplicate only. The separate multi-root (~/.agents/skills shared-skills) duplicate facet is a distinct concern, not addressed here. Co-Authored-By: Claude Opus 4.8 * chore(#1326): add changeset for Codex sidecar cleanup Co-Authored-By: Claude Opus 4.8 --------- Co-authored-by: Claude Opus 4.8 --- .changeset/1326-codex-sidecar-cleanup.md | 6 + bin/install.js | 138 ++++-------- docs/FEATURES.md | 2 +- docs/USER-GUIDE.md | 8 +- tests/codex-config.test.cjs | 273 ++++++++--------------- 5 files changed, 148 insertions(+), 279 deletions(-) create mode 100644 .changeset/1326-codex-sidecar-cleanup.md diff --git a/.changeset/1326-codex-sidecar-cleanup.md b/.changeset/1326-codex-sidecar-cleanup.md new file mode 100644 index 000000000..6627f0a8e --- /dev/null +++ b/.changeset/1326-codex-sidecar-cleanup.md @@ -0,0 +1,6 @@ +--- +type: Fixed +pr: 1360 +--- + +**Codex skills no longer show up twice in autocomplete** — GSD's Codex install wrote an `agents/openai.yaml` sidecar under every managed `gsd-*` skill directory, and recent Codex builds index both `SKILL.md` and the sidecar, so each skill appeared twice (once as `gsd-foo`, once as a humanized `foo` display name). The installer now stops emitting these sidecars and removes stale ones left by prior installs (pruning the empty `agents/` directory), while preserving user-owned skill directories. Codex discovers GSD skills via `SKILL.md` alone. (#1326) diff --git a/bin/install.js b/bin/install.js index 9c71521ee..f084958ef 100755 --- a/bin/install.js +++ b/bin/install.js @@ -3198,110 +3198,55 @@ function generateCodexAgentToml(agentName, agentContent, modelOverrides = null, } /** - * Generate the agents/openai.yaml TUI chip metadata content for a Codex skill. + * Remove stale agents/openai.yaml sidecar files from GSD-managed Codex skill dirs. * - * This file is written alongside SKILL.md as /agents/openai.yaml. - * Codex loads it as a SkillMetadataFile (codex-rs/core-skills/src/loader.rs), - * making the skill discoverable in the /skills TUI popup with a display name - * and short description. If the file is absent, Codex silently skips it (fails open). + * Prior to #1326, GSD's Codex install path wrote an agents/openai.yaml file + * alongside each gsd-* SKILL.md. Recent Codex builds index BOTH SKILL.md and + * the sidecar, causing each GSD skill to appear twice in autocomplete. This + * function removes those stale sidecars and — if the agents/ subdirectory is + * now empty — prunes it too. * - * Schema (interface section): - * display_name: short human-readable skill name (strip gsd- prefix) - * short_description: 1-2 sentence description for TUI chip, ≤180 chars - * - * @param {string} skillName - Full skill name e.g. "gsd-plan-phase" - * @param {string} shortDescription - Description text (already truncated by caller) - * @returns {string} YAML content for agents/openai.yaml - */ -function generateCodexSkillMetadataYaml(skillName, shortDescription) { - // Display name: strip "gsd-" prefix and convert hyphens to spaces for readability. - const displayName = skillName.replace(/^gsd-/, '').replace(/-/g, ' '); - // yamlQuote (= JSON.stringify) handles all YAML-unsafe chars: backslashes, - // quotes, newlines, control characters, and Unicode escapes. - return [ - 'interface:', - ` display_name: ${yamlQuote(displayName)}`, - ` short_description: ${yamlQuote(shortDescription)}`, - '', - ].join('\n'); -} - -/** - * Write agents/openai.yaml TUI chip metadata for each gsd-* skill directory. - * - * Called after layout-driven skill install for Codex. Iterates every gsd-* - * skill directory in skillsDir, reads the SKILL.md frontmatter to extract the - * short-description already emitted by convertClaudeCommandToCodexSkill, then - * writes /agents/openai.yaml using generateCodexSkillMetadataYaml. - * - * Fails open: individual skill directories that cannot be processed are silently - * skipped so a single malformed SKILL.md cannot block the whole install. - * - * User-owned skill directories (e.g. gsd-dev-preferences) are explicitly - * skipped so existing user-authored agents/openai.yaml files are never - * overwritten. These dirs are listed in the same USER_OWNED_SKILL_DIRS - * constant used by installOpencodeFamilySkills. - * - * The YAML-quoted description value is unescaped before embedding so that - * YAML escape sequences (e.g. \" in a double-quoted scalar) become the - * literal characters they represent rather than being double-escaped in the - * output. + * Behaviour: + * - Returns immediately if skillsDir does not exist (fails open). + * - Only touches directories whose names start with "gsd-". + * - Skips user-owned dirs (gsd-dev-preferences) — their agents/ content is + * never modified, mirroring the same USER_OWNED_SKILL_DIRS guard used by + * installOpencodeFamilySkills. + * - For each managed gsd-* dir, if agents/openai.yaml exists, deletes it. + * - If agents/ is now empty, removes the directory; if it still contains + * other files (e.g. user-added content), leaves it in place. + * - Non-gsd-* dirs and their agents/ content are never touched. + * - Individual failures are caught and swallowed so a single bad dir cannot + * block the install (fail-open, matching the original design). * * @param {string} skillsDir - Path to the skills/ directory (e.g. ~/.codex/skills) */ -function writeCodexSkillMetadataFiles(skillsDir) { +function cleanupCodexSkillMetadataSidecars(skillsDir) { if (!fs.existsSync(skillsDir)) return; // Mirror the user-owned list from installOpencodeFamilySkills (#2973). // We MUST skip these dirs — their contents are user-generated and must - // never be overwritten by GSD's install path. + // never be modified by GSD's install path. const _userOwnedSkillDirs = new Set(['gsd-dev-preferences']); for (const entry of fs.readdirSync(skillsDir, { withFileTypes: true })) { if (!entry.isDirectory() || !entry.name.startsWith('gsd-')) continue; if (_userOwnedSkillDirs.has(entry.name)) continue; // preserve user content - const skillDir = path.join(skillsDir, entry.name); - const skillMdPath = path.join(skillDir, 'SKILL.md'); + const agentsSubdir = path.join(skillsDir, entry.name, 'agents'); + const sidecarPath = path.join(agentsSubdir, 'openai.yaml'); try { - const content = fs.readFileSync(skillMdPath, 'utf8'); - const { frontmatter } = extractFrontmatterAndBody(content); - // Prefer the short-description field emitted by convertClaudeCommandToCodexSkill; - // fall back to description, then a synthetic label from the skill name. - let shortDesc = ''; - if (frontmatter) { - // SKILL.md uses YAML frontmatter with a nested metadata.short-description key. - // extractFrontmatterField handles only top-level keys; parse the metadata block - // by looking for " short-description:" directly. - const metaMatch = frontmatter.match(/^[ \t]*metadata\s*:\s*\n((?:[ \t]+.*\n?)*)/m); - if (metaMatch) { - const metaBlock = metaMatch[1]; - const sdMatch = metaBlock.match(/^[ \t]+short-description\s*:\s*(.+)$/m); - if (sdMatch) { - // Unescape YAML double-quoted scalar escapes before embedding. - // convertClaudeCommandToCodexSkill always emits a double-quoted - // value (via yamlQuote) so only double-quote unescaping is needed. - let raw = sdMatch[1].trim(); - if (raw.startsWith('"') && raw.endsWith('"')) { - // Strip outer double-quotes and decode \" → " and \\ → \ - raw = raw.slice(1, -1).replace(/\\"/g, '"').replace(/\\\\/g, '\\'); - } else { - // Single-quoted or unquoted: strip surrounding quotes/whitespace - raw = raw.replace(/^["']|["']$/g, ''); - } - shortDesc = raw; - } - } - if (!shortDesc) { - shortDesc = extractFrontmatterField(frontmatter, 'description') || ''; - } + // Symlink guard: if agents/ is a symlink pointing outside the skills tree, + // deleting through it could escape the tree. Skip this dir entirely. + let agentsStat; + try { agentsStat = fs.lstatSync(agentsSubdir); } catch (_e) { continue; } + if (agentsStat.isSymbolicLink()) continue; + if (fs.existsSync(sidecarPath)) { + fs.rmSync(sidecarPath); } - if (!shortDesc) { - shortDesc = `Run GSD workflow ${entry.name}.`; + // Prune the agents/ dir only if it is now empty (leave it if other files remain). + if (fs.existsSync(agentsSubdir) && fs.readdirSync(agentsSubdir).length === 0) { + fs.rmdirSync(agentsSubdir); } - const yamlContent = generateCodexSkillMetadataYaml(entry.name, shortDesc); - const agentsSubdir = path.join(skillDir, 'agents'); - fs.mkdirSync(agentsSubdir, { recursive: true }); - fs.writeFileSync(path.join(agentsSubdir, 'openai.yaml'), yamlContent); } catch (_err) { - // Fail open — missing or unreadable SKILL.md must not block the install. + // Fail open — a single bad dir must not block the install. } } } @@ -9841,14 +9786,14 @@ function install(isGlobal, runtime = 'claude', options = {}) { const scope = isGlobal ? 'global' : 'local'; installRuntimeArtifacts(runtime, targetDir, scope, _resolvedProfile); - // #774 — Codex only: write agents/openai.yaml TUI chip metadata alongside each - // installed skill so the /skills popup shows name + description for each gsd-* skill. - // The SkillMetadataFile is loaded by codex-rs/core-skills/src/loader.rs from - // /agents/openai.yaml; absence is silently tolerated (fails open). - // We parse the SKILL.md frontmatter to extract short-description already emitted - // by convertClaudeCommandToCodexSkill and use it as the TUI chip description. + // #1326 — Codex only: remove stale agents/openai.yaml sidecars from managed + // gsd-* skill dirs. Prior installs wrote these files so Codex would show a + // display name and description in the /skills TUI popup. Recent Codex builds + // index BOTH SKILL.md and the sidecar, causing each GSD skill to appear twice + // in autocomplete. Cleaning them up fixes the duplication; SKILL.md alone is + // sufficient for Codex discovery. User-owned dirs are never touched. if (isCodex) { - writeCodexSkillMetadataFiles(path.join(targetDir, 'skills')); + cleanupCodexSkillMetadataSidecars(path.join(targetDir, 'skills')); } // Hermes only: write DESCRIPTION.md for the gsd/ category after layout install @@ -12130,8 +12075,7 @@ module.exports = { convertClaudeToGeminiAgent, convertClaudeAgentToCodexAgent, generateCodexAgentToml, - generateCodexSkillMetadataYaml, - writeCodexSkillMetadataFiles, + cleanupCodexSkillMetadataSidecars, generateCodexConfigBlock, stripGsdFromCodexConfig, migrateCodexHooksMapFormat, diff --git a/docs/FEATURES.md b/docs/FEATURES.md index 808a67b68..d6520f9c6 100644 --- a/docs/FEATURES.md +++ b/docs/FEATURES.md @@ -1053,7 +1053,7 @@ fix(03-01): correct auth token expiry - **Copilot:** `sessionStart` **Runtime-specific enrichments (1.4.0):** -- Codex emits `service_tier: flex` for light-tier agents and an `agents/openai.yaml` chip so GSD skills appear in the Codex `/skills` picker +- Codex emits `service_tier: flex` for light-tier agents; GSD skills appear in the Codex `/skills` picker via `SKILL.md` (no `agents/openai.yaml` sidecar is emitted — doing so caused duplicate autocomplete entries, #1326) - Gemini commands use native `{{args}}` interpolation **Native packaging:** diff --git a/docs/USER-GUIDE.md b/docs/USER-GUIDE.md index b8ac37ef9..f165340c5 100644 --- a/docs/USER-GUIDE.md +++ b/docs/USER-GUIDE.md @@ -731,13 +731,13 @@ To assign different models on a non-Claude runtime: #### Codex skill picker and agent scheduling (#774) -GSD enriches each Codex install with two additional artifacts: - -- **Skill TUI chip** — each installed `gsd-*` skill directory contains an `agents/openai.yaml` file that populates the Codex `/skills` picker with a human-readable display name and a short description, so you can browse and invoke GSD skills from the Codex TUI without typing the full skill name. +GSD enriches each Codex install with an additional artifact: - **Flex-tier scheduling** — light-tier agents (haiku-equivalent) emit `service_tier = "flex"` and `model_verbosity = "low"` in their agent TOML. The Codex scheduler routes these agents to the flex tier (lower cost, background processing) and suppresses verbose token output. -Both enrichments are written automatically at install time and require no manual configuration. Requires Codex CLI ≥ 0.130.0. +GSD skills appear in the Codex `/skills` picker via their `SKILL.md` file, which Codex discovers automatically. No `agents/openai.yaml` sidecar is emitted — doing so caused duplicate autocomplete entries (#1326). + +This enrichment is written automatically at install time and requires no manual configuration. Requires Codex CLI ≥ 0.130.0. #### Switching from Claude to Codex with one config change (#2517) diff --git a/tests/codex-config.test.cjs b/tests/codex-config.test.cjs index d0ceeffe4..eb6f1908f 100644 --- a/tests/codex-config.test.cjs +++ b/tests/codex-config.test.cjs @@ -20,7 +20,6 @@ const path = require('path'); const os = require('os'); const { execFileSync } = require('child_process'); const { cleanup } = require('./helpers.cjs'); -const jsYaml = require('js-yaml'); // #2153 follow-up: ensure hooks/dist/ exists before any install integration // test runs. The Codex install path copies hook files from hooks/dist/, which @@ -45,8 +44,7 @@ const { convertClaudeAgentToCodexAgent, convertClaudeCommandToCodexSkill, generateCodexAgentToml, - generateCodexSkillMetadataYaml, - writeCodexSkillMetadataFiles, + cleanupCodexSkillMetadataSidecars, generateCodexConfigBlock, stripGsdFromCodexConfig, migrateCodexHooksMapFormat, @@ -2599,214 +2597,135 @@ describe('Codex uninstall symmetry for hook-enabled configs', () => { }); }); -// ─── #774: generateCodexSkillMetadataYaml ──────────────────────────────────────── +// ─── #1326: cleanupCodexSkillMetadataSidecars (replaces #774 writeCodexSkillMetadataFiles) ── -describe('generateCodexSkillMetadataYaml', () => { - test('emits valid parseable YAML with interface section (#774)', () => { - const yaml = generateCodexSkillMetadataYaml('gsd-plan-phase', 'Plan and structure the next development phase.'); - // Must start with interface: and be parseable YAML - assert.ok(yaml.startsWith('interface:'), 'must start with interface: key'); - const parsed = jsYaml.load(yaml); - assert.ok(parsed && typeof parsed === 'object', 'must be parseable YAML object'); - assert.ok(parsed.interface, 'must have interface key'); - assert.ok('display_name' in parsed.interface, 'must include display_name'); - assert.ok('short_description' in parsed.interface, 'must include short_description'); - }); - - test('strips gsd- prefix from display_name and converts hyphens to spaces (#774)', () => { - const yaml = generateCodexSkillMetadataYaml('gsd-plan-phase', 'Plan and structure the next development phase.'); - const parsed = jsYaml.load(yaml); - assert.strictEqual(parsed.interface.display_name, 'plan phase', - 'display_name must be "plan phase" (gsd- stripped, hyphens→spaces)'); - }); - - test('embeds the short_description text as decoded string (#774)', () => { - const desc = 'Run GSD workflow gsd-test.'; - const yaml = generateCodexSkillMetadataYaml('gsd-test', desc); - const parsed = jsYaml.load(yaml); - assert.strictEqual(parsed.interface.short_description, desc, - 'short_description must round-trip through YAML correctly'); - }); - - test('escapes double-quotes in description so parsed value is correct (#774)', () => { - const desc = 'Run "special" workflow.'; - const yaml = generateCodexSkillMetadataYaml('gsd-test', desc); - const parsed = jsYaml.load(yaml); - assert.strictEqual(parsed.interface.short_description, desc, - 'double-quotes in description must round-trip correctly through YAML'); - }); - - test('escapes backslashes in description so parsed value is correct (#774)', () => { - const desc = 'Run C:\\path\\to workflow.'; - const yaml = generateCodexSkillMetadataYaml('gsd-test', desc); - const parsed = jsYaml.load(yaml); - assert.strictEqual(parsed.interface.short_description, desc, - 'backslashes in description must round-trip correctly through YAML'); - }); - - test('output ends with a newline (#774)', () => { - const yaml = generateCodexSkillMetadataYaml('gsd-test', 'Test skill.'); - assert.ok(yaml.endsWith('\n'), 'output must end with a newline'); - }); - - test('works for skill without gsd- prefix (#774)', () => { - const yaml = generateCodexSkillMetadataYaml('my-skill', 'A custom skill.'); - const parsed = jsYaml.load(yaml); - assert.strictEqual(parsed.interface.display_name, 'my skill', - 'display_name must convert hyphens to spaces even without gsd- prefix'); - }); -}); - -// ─── #774: writeCodexSkillMetadataFiles ───────────────────────────────────────── - -describe('writeCodexSkillMetadataFiles', () => { +describe('cleanupCodexSkillMetadataSidecars (#1326)', () => { let tmpDir; beforeEach(() => { - tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-test-skill-meta-')); + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-test-sidecar-cleanup-')); }); afterEach(() => { cleanup(tmpDir); }); - test('writes agents/openai.yaml for each gsd-* skill directory (#774)', () => { - // Set up a synthetic skills directory with two gsd-* skill dirs - const skills = [ - { name: 'gsd-plan-phase', desc: 'Plan and structure the next development phase.' }, - { name: 'gsd-execute-phase', desc: 'Execute the current phase.' }, - ]; - for (const { name, desc } of skills) { - const skillDir = path.join(tmpDir, name); - fs.mkdirSync(skillDir, { recursive: true }); - const skillMd = `---\nname: ${name}\ndescription: "${desc}"\nmetadata:\n short-description: "${desc}"\n---\n\nBody text.\n`; - fs.writeFileSync(path.join(skillDir, 'SKILL.md'), skillMd); - } + test('Codex install does not emit managed agents/openai.yaml sidecars and removes stale ones (#1326)', () => { + // gsd-foo: managed skill with stale sidecar → sidecar removed, empty agents/ pruned + const fooAgents = path.join(tmpDir, 'gsd-foo', 'agents'); + fs.mkdirSync(fooAgents, { recursive: true }); + fs.writeFileSync(path.join(tmpDir, 'gsd-foo', 'SKILL.md'), '---\nname: gsd-foo\n---\nBody.\n'); + fs.writeFileSync(path.join(fooAgents, 'openai.yaml'), 'interface:\n display_name: "foo"\n'); - writeCodexSkillMetadataFiles(tmpDir); + // gsd-dev-preferences: user-owned → sidecar PRESERVED + const prefAgents = path.join(tmpDir, 'gsd-dev-preferences', 'agents'); + fs.mkdirSync(prefAgents, { recursive: true }); + fs.writeFileSync(path.join(tmpDir, 'gsd-dev-preferences', 'SKILL.md'), '---\nname: gsd-dev-preferences\n---\nBody.\n'); + const userYaml = 'interface:\n display_name: "my prefs"\n short_description: "User-authored"\n'; + fs.writeFileSync(path.join(prefAgents, 'openai.yaml'), userYaml); - for (const { name, desc } of skills) { - const yamlPath = path.join(tmpDir, name, 'agents', 'openai.yaml'); - assert.ok(fs.existsSync(yamlPath), `agents/openai.yaml must exist for ${name}`); - const content = fs.readFileSync(yamlPath, 'utf8'); - // Verify it's parseable YAML with the correct structure - const parsed = jsYaml.load(content); - assert.ok(parsed && parsed.interface, `${name}/agents/openai.yaml must parse to object with interface:`); - assert.ok('display_name' in parsed.interface, `${name}/agents/openai.yaml must have display_name`); - assert.strictEqual(parsed.interface.short_description, desc, - `${name}/agents/openai.yaml short_description must match source description`); - } + // gsd-bar: managed skill with sidecar + another file in agents/ → sidecar removed, agents/ kept (has other.txt) + const barAgents = path.join(tmpDir, 'gsd-bar', 'agents'); + fs.mkdirSync(barAgents, { recursive: true }); + fs.writeFileSync(path.join(tmpDir, 'gsd-bar', 'SKILL.md'), '---\nname: gsd-bar\n---\nBody.\n'); + fs.writeFileSync(path.join(barAgents, 'openai.yaml'), 'interface:\n display_name: "bar"\n'); + fs.writeFileSync(path.join(barAgents, 'other.txt'), 'some other content\n'); + + // helper: non-gsd dir with openai.yaml → UNTOUCHED + const helperAgents = path.join(tmpDir, 'helper', 'agents'); + fs.mkdirSync(helperAgents, { recursive: true }); + fs.writeFileSync(path.join(helperAgents, 'openai.yaml'), 'interface:\n display_name: "helper"\n'); + + cleanupCodexSkillMetadataSidecars(tmpDir); + + // gsd-foo: sidecar removed and empty agents/ pruned + assert.ok(!fs.existsSync(path.join(fooAgents, 'openai.yaml')), + 'gsd-foo/agents/openai.yaml must be removed (managed stale sidecar)'); + assert.ok(!fs.existsSync(fooAgents), + 'gsd-foo/agents/ must be pruned when empty after sidecar removal'); + + // gsd-dev-preferences: user-owned, sidecar preserved + assert.ok(fs.existsSync(path.join(prefAgents, 'openai.yaml')), + 'gsd-dev-preferences/agents/openai.yaml must be preserved (user-owned)'); + assert.strictEqual(fs.readFileSync(path.join(prefAgents, 'openai.yaml'), 'utf8'), userYaml, + 'gsd-dev-preferences/agents/openai.yaml content must be unchanged'); + + // gsd-bar: sidecar removed but agents/ kept (still has other.txt) + assert.ok(!fs.existsSync(path.join(barAgents, 'openai.yaml')), + 'gsd-bar/agents/openai.yaml must be removed'); + assert.ok(fs.existsSync(barAgents), + 'gsd-bar/agents/ must NOT be pruned (still contains other.txt)'); + assert.ok(fs.existsSync(path.join(barAgents, 'other.txt')), + 'gsd-bar/agents/other.txt must be preserved'); + + // helper: non-gsd dir untouched + assert.ok(fs.existsSync(path.join(helperAgents, 'openai.yaml')), + 'helper/agents/openai.yaml must be untouched (non-gsd dir)'); }); - test('ignores non-gsd directories (#774)', () => { - // Non-gsd-* dir should not get agents/openai.yaml - const nonGsdDir = path.join(tmpDir, 'custom-skill'); - fs.mkdirSync(nonGsdDir, { recursive: true }); - fs.writeFileSync(path.join(nonGsdDir, 'SKILL.md'), '---\nname: custom\n---\nBody.\n'); - - writeCodexSkillMetadataFiles(tmpDir); - - assert.ok(!fs.existsSync(path.join(nonGsdDir, 'agents', 'openai.yaml')), - 'non-gsd-* dirs must not get agents/openai.yaml'); - }); - - test('does not overwrite user-owned gsd-dev-preferences/agents/openai.yaml (#774)', () => { - // gsd-dev-preferences is user-owned and must never be modified by GSD install - const userOwnedDir = path.join(tmpDir, 'gsd-dev-preferences'); - const agentsSubdir = path.join(userOwnedDir, 'agents'); - fs.mkdirSync(agentsSubdir, { recursive: true }); - fs.writeFileSync(path.join(userOwnedDir, 'SKILL.md'), '---\nname: gsd-dev-preferences\ndescription: "User pref"\n---\nBody.\n'); - const userYaml = 'interface:\n display_name: "my preferences"\n short_description: "User-authored"\n'; - fs.writeFileSync(path.join(agentsSubdir, 'openai.yaml'), userYaml); - - writeCodexSkillMetadataFiles(tmpDir); - - // User-authored file must remain unchanged - const after = fs.readFileSync(path.join(agentsSubdir, 'openai.yaml'), 'utf8'); - assert.strictEqual(after, userYaml, 'user-owned gsd-dev-preferences/agents/openai.yaml must not be overwritten'); - }); - - test('does not create agents/openai.yaml for gsd-dev-preferences if absent (#774)', () => { - // Even if gsd-dev-preferences exists without an openai.yaml, we must not create one - const userOwnedDir = path.join(tmpDir, 'gsd-dev-preferences'); - fs.mkdirSync(userOwnedDir, { recursive: true }); - fs.writeFileSync(path.join(userOwnedDir, 'SKILL.md'), '---\nname: gsd-dev-preferences\n---\nBody.\n'); - - writeCodexSkillMetadataFiles(tmpDir); - - assert.ok(!fs.existsSync(path.join(userOwnedDir, 'agents', 'openai.yaml')), - 'gsd-dev-preferences must not get agents/openai.yaml even if it was absent'); - }); - - test('is a no-op when skillsDir does not exist (#774)', () => { - // Should not throw when the directory doesn't exist + test('is a no-op when skillsDir does not exist (#1326)', () => { assert.doesNotThrow(() => { - writeCodexSkillMetadataFiles(path.join(tmpDir, 'nonexistent')); + cleanupCodexSkillMetadataSidecars(path.join(tmpDir, 'nonexistent')); }, 'must not throw when skillsDir does not exist'); }); - test('skips skill dirs with missing SKILL.md without throwing (#774)', () => { - // Create a gsd-* dir with no SKILL.md — should fail open - const emptySkillDir = path.join(tmpDir, 'gsd-empty'); - fs.mkdirSync(emptySkillDir, { recursive: true }); + test('is a no-op for managed gsd-* dirs with no agents/openai.yaml (#1326)', () => { + // No sidecar present — should not throw, should not create anything + const skillDir = path.join(tmpDir, 'gsd-baz'); + fs.mkdirSync(skillDir, { recursive: true }); + fs.writeFileSync(path.join(skillDir, 'SKILL.md'), '---\nname: gsd-baz\n---\nBody.\n'); assert.doesNotThrow(() => { - writeCodexSkillMetadataFiles(tmpDir); - }, 'must not throw when SKILL.md is missing'); + cleanupCodexSkillMetadataSidecars(tmpDir); + }, 'must not throw when no sidecar exists'); + assert.ok(!fs.existsSync(path.join(skillDir, 'agents')), + 'must not create agents/ dir when no sidecar was present'); }); - test('display_name in agents/openai.yaml has gsd- prefix stripped (#774)', () => { - const skillDir = path.join(tmpDir, 'gsd-plan-phase'); - fs.mkdirSync(skillDir, { recursive: true }); - const skillMd = `---\nname: gsd-plan-phase\ndescription: "Plan the phase."\nmetadata:\n short-description: "Plan the phase."\n---\n\nBody.\n`; - fs.writeFileSync(path.join(skillDir, 'SKILL.md'), skillMd); + test('does not delete through a symlinked agents/ directory (#1326)', { skip: process.platform === 'win32' }, () => { + // Setup: a skills dir with gsd-foo/ whose agents/ is a SYMLINK to an external dir. + // The cleanup must not delete files through the symlink. + const externalDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-test-symlink-ext-')); + try { + // Place openai.yaml and a sentinel in the external dir. + fs.writeFileSync(path.join(externalDir, 'openai.yaml'), 'interface:\n display_name: "external"\n'); + fs.writeFileSync(path.join(externalDir, 'keep.txt'), 'sentinel\n'); - writeCodexSkillMetadataFiles(tmpDir); + // Create gsd-foo/ in the skills dir and make agents/ a symlink to externalDir. + const skillDir = path.join(tmpDir, 'gsd-foo'); + fs.mkdirSync(skillDir, { recursive: true }); + const agentsLink = path.join(skillDir, 'agents'); + fs.symlinkSync(externalDir, agentsLink, 'dir'); - const yamlContent = fs.readFileSync(path.join(tmpDir, 'gsd-plan-phase', 'agents', 'openai.yaml'), 'utf8'); - const parsed = jsYaml.load(yamlContent); - assert.strictEqual(parsed.interface.display_name, 'plan phase', - 'display_name must have gsd- stripped and hyphens→spaces'); + cleanupCodexSkillMetadataSidecars(tmpDir); + + // Nothing in the external dir must have been deleted. + assert.ok(fs.existsSync(path.join(externalDir, 'openai.yaml')), + 'external/openai.yaml must still exist — cleanup must not delete through a symlinked agents/ dir'); + assert.ok(fs.existsSync(path.join(externalDir, 'keep.txt')), + 'external/keep.txt must still exist — cleanup must not delete through a symlinked agents/ dir'); + // The symlink itself must still be present. + assert.ok(fs.existsSync(agentsLink), + 'gsd-foo/agents symlink must still exist'); + } finally { + cleanup(externalDir); + } }); - test('correctly unescapes YAML-quoted short-description from SKILL.md (#774)', () => { - // convertClaudeCommandToCodexSkill emits double-quoted YAML for descriptions. - // Verify that escape sequences like \" in the YAML source round-trip to literal " in output. - const skillDir = path.join(tmpDir, 'gsd-test-esc'); - fs.mkdirSync(skillDir, { recursive: true }); - // SKILL.md has a YAML-escaped double-quote in short-description - const skillMd = '---\nname: gsd-test-esc\ndescription: "Normal"\nmetadata:\n short-description: "Run \\"special\\" workflow."\n---\n\nBody.\n'; - fs.writeFileSync(path.join(skillDir, 'SKILL.md'), skillMd); - - writeCodexSkillMetadataFiles(tmpDir); - - const yamlContent = fs.readFileSync(path.join(tmpDir, 'gsd-test-esc', 'agents', 'openai.yaml'), 'utf8'); - const parsed = jsYaml.load(yamlContent); - assert.strictEqual(parsed.interface.short_description, 'Run "special" workflow.', - 'YAML-escaped quotes in SKILL.md must round-trip to literal quotes in agents/openai.yaml'); - }); - - test('Codex install emits agents/openai.yaml for each skill (#774)', () => { - // Integration test: run a full Codex install and verify agents/openai.yaml is written. - // Use runCodexInstall so CODEX_HOME is saved and restored correctly even if it was - // already set in the environment before this test ran. + test('Codex install does not create agents/openai.yaml sidecars for any managed skill (#1326)', () => { + // Integration test: full Codex install must NOT produce any managed gsd-*/agents/openai.yaml const codexHome = path.join(tmpDir, 'codex-home'); fs.mkdirSync(codexHome, { recursive: true }); runCodexInstall(codexHome); const skillsDir = path.join(codexHome, 'skills'); - // Assert that the install actually created a skills directory assert.ok(fs.existsSync(skillsDir), 'Codex install must create a skills/ directory'); const gsdSkillDirs = fs.readdirSync(skillsDir, { withFileTypes: true }) - .filter(e => e.isDirectory() && e.name.startsWith('gsd-')); - // At least some skills should be present - assert.ok(gsdSkillDirs.length > 0, 'install must create at least one gsd-* skill directory'); - // Each skill directory must have agents/openai.yaml with valid YAML + .filter(e => e.isDirectory() && e.name.startsWith('gsd-') && e.name !== 'gsd-dev-preferences'); + assert.ok(gsdSkillDirs.length > 0, 'install must create at least one managed gsd-* skill directory'); for (const skillEntry of gsdSkillDirs) { const yamlPath = path.join(skillsDir, skillEntry.name, 'agents', 'openai.yaml'); - assert.ok(fs.existsSync(yamlPath), `${skillEntry.name}/agents/openai.yaml must exist after install`); - const content = fs.readFileSync(yamlPath, 'utf8'); - const parsed = jsYaml.load(content); - assert.ok(parsed && parsed.interface, `${skillEntry.name}/agents/openai.yaml must parse to object with interface:`); + assert.ok(!fs.existsSync(yamlPath), + `${skillEntry.name}/agents/openai.yaml must NOT exist after install (#1326 sidecar dedup)`); } }); });