From 4b50a686e546be1c23fbd9f3cfaec53113ea3795 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Sat, 6 Jun 2026 12:40:06 -0400 Subject: [PATCH] fix(#704): exclude } and ) from Codex path-rewrite lookbehind (no literal $gsd-core in installs) (#710) * fix(#704): exclude } and ) from Codex path-rewrite lookbehind Shell variable expressions like \${VAR}/gsd-core/ and command-substitution paths like \$(cmd)/gsd-local-patches were being rewritten to \$gsd-core and \$gsd-local-patches respectively because the negative lookbehind in convertSlashCommandsToCodexSkillMentions did not include } or ). Add both characters to the lookbehind set: (? * chore: link changeset to PR #710 --------- Co-authored-by: Claude Sonnet 4.6 --- .changeset/rapid-tigers-dance.md | 5 + bin/install.js | 9 +- ...04-codex-launcher-path-corruption.test.cjs | 237 ++++++++++++++++++ 3 files changed, 248 insertions(+), 3 deletions(-) create mode 100644 .changeset/rapid-tigers-dance.md create mode 100644 tests/bug-704-codex-launcher-path-corruption.test.cjs diff --git a/.changeset/rapid-tigers-dance.md b/.changeset/rapid-tigers-dance.md new file mode 100644 index 000000000..d4ee0b42a --- /dev/null +++ b/.changeset/rapid-tigers-dance.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 710 +--- +**Codex install no longer corrupts launcher paths** — shell path segments like `${VAR}/gsd-core/` and `$(cmd)/gsd-local-patches` are no longer rewritten into a literal `$gsd-core` token during Codex markdown conversion (#704). diff --git a/bin/install.js b/bin/install.js index 4dd21d3ae..c143d8533 100755 --- a/bin/install.js +++ b/bin/install.js @@ -2802,9 +2802,12 @@ function convertSlashCommandsToCodexSkillMentions(content) { return `$gsd-${String(commandName).toLowerCase()}`; }); // Convert hyphen-style command references (workflow output) to Codex $ prefix. - // Negative lookbehind excludes file paths like bin/gsd-tools.cjs where - // the slash is preceded by a word char, dot, or another slash. - converted = converted.replace(/(? { + // Negative lookbehind excludes shell path contexts where `/gsd-` is a path + // segment, not a slash-command mention: + // - word chars / dot / slash: `bin/gsd-tools.cjs`, `.claude/gsd-core/` + // - `}`: shell variable expressions `${VAR}/gsd-core/` (#704) + // - `)`: command-substitution paths `$(cmd)/gsd-local-patches` (#704) + converted = converted.replace(/(? { return `$gsd-${String(commandName).toLowerCase()}`; }); return converted; diff --git a/tests/bug-704-codex-launcher-path-corruption.test.cjs b/tests/bug-704-codex-launcher-path-corruption.test.cjs new file mode 100644 index 000000000..c841878c7 --- /dev/null +++ b/tests/bug-704-codex-launcher-path-corruption.test.cjs @@ -0,0 +1,237 @@ +// allow-test-rule: source-text-is-the-product +'use strict'; + +/** + * Regression test for issue #704: + * "v1.3.1 global install ships literal $gsd-core launcher paths in workflows" + * + * ROOT CAUSE: `convertSlashCommandsToCodexSkillMentions` had a regex + * /(? { + test('convertClaudeCommandToCodexSkill does not corrupt ${VAR}/gsd-core/ or $(cmd)/gsd-* paths', () => { + // Minimal fixture with the launcher snippet and command-substitution patterns + // that were being corrupted (#704). + const input = [ + '---', + 'description: Test skill', + '---', + '', + '```bash', + '_GSD_SHIM_NAME="gsd-tools.cjs"', + '_GSD_RUNTIME_ROOT="${RUNTIME_DIR:-$(git rev-parse --show-toplevel 2>/dev/null || pwd)}"', + 'GSD_TOOLS="${_GSD_RUNTIME_ROOT}/gsd-core/bin/${_GSD_SHIM_NAME}"', + 'if [ -f "$GSD_TOOLS" ]; then', + ' gsd_run() { node "$GSD_TOOLS" "$@"; }', + 'elif [ -f "${_GSD_RUNTIME_ROOT}/.claude/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then', + ' GSD_TOOLS="${_GSD_RUNTIME_ROOT}/.claude/gsd-core/bin/${_GSD_SHIM_NAME}"', + ' gsd_run() { node "$GSD_TOOLS" "$@"; }', + 'elif [ -f "$HOME/.claude/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then', + ' GSD_TOOLS="$HOME/.claude/gsd-core/bin/${_GSD_SHIM_NAME}"', + ' gsd_run() { node "$GSD_TOOLS" "$@"; }', + 'fi', + '# Command-substitution path form (reapply-patches pattern)', + 'candidate="$(expand_home "$KILO_CONFIG_DIR")/gsd-local-patches"', + '```', + ].join('\n'); + + const output = convertClaudeCommandToCodexSkill(input, 'gsd-test-704'); + + // Shell-context corruption patterns from issue #704: + // - `}$gsd-*` from shell variable expressions `${VAR}/gsd-*` + // - `)$gsd-*` from command-substitution paths `$(cmd)/gsd-*` + const shellCorruptionPatterns = [ + { pattern: '}' + BAD_TOKEN, description: 'shell-variable }$gsd-core' }, + { pattern: ')$gsd-local', description: 'command-substitution )$gsd-local-patches' }, + ]; + for (const { pattern, description } of shellCorruptionPatterns) { + assert.ok( + !output.includes(pattern), + `Codex skill conversion must not produce "${pattern}" (${description}). ` + + `Offending fragment: ${ + output.includes(pattern) + ? output.substring(output.indexOf(pattern) - 50, output.indexOf(pattern) + 80) + : '(not found)' + }`, + ); + } + + // The correct path forms must be preserved — the canonical launcher path + // (RUNTIME_ROOT_PATH) must survive Codex conversion intact. + assert.ok( + output.includes(RUNTIME_ROOT_PATH), + `Expected canonical launcher path "${RUNTIME_ROOT_PATH}" to appear in the converted output. ` + + `Got:\n${output.substring(0, 500)}`, + ); + assert.ok( + output.includes(')/gsd-local-patches'), + `Expected ")/gsd-local-patches" to appear in the converted output. ` + + `Got:\n${output.substring(0, 500)}`, + ); + }); + + test('convertClaudeCommandToCodexSkill preserves all shell path forms (}, ) closers)', () => { + // All these paths appear after a shell-closing character (} or )) and must + // NOT be converted to $gsd-* by the Codex slash-command converter. + const shellPaths = [ + // Shell variable expression forms (} closer) + { path: '"${_GSD_RUNTIME_ROOT}/gsd-core/bin/${_GSD_SHIM_NAME}"', corruptedForm: '}$gsd-core' }, + { path: '"${_GSD_RUNTIME_ROOT}/.claude/gsd-core/bin/${_GSD_SHIM_NAME}"', corruptedForm: '}$gsd-core' }, + { path: '"$HOME/.claude/gsd-core/bin/${_GSD_SHIM_NAME}"', corruptedForm: '}$gsd-core' }, + // Command-substitution forms () closer) — reapply-patches pattern + { path: 'candidate="$(expand_home "$KILO_CONFIG_DIR")/gsd-local-patches"', corruptedForm: ')$gsd-local' }, + { path: 'candidate="$(dirname "$(expand_home "$OPENCODE_CONFIG")")/gsd-local-patches"', corruptedForm: ')$gsd-local' }, + ]; + + for (const { path: p, corruptedForm } of shellPaths) { + const input = `---\ndescription: Test\n---\n\n\`\`\`bash\n${p}\n\`\`\``; + const output = convertClaudeCommandToCodexSkill(input, 'gsd-test-704-paths'); + assert.ok( + !output.includes(corruptedForm), + `Path "${p}" was corrupted to contain "${corruptedForm}" after Codex conversion.\n` + + `Got:\n${output}`, + ); + } + }); + + test('convertClaudeCommandToCodexSkill still converts legitimate /gsd- slash mentions', () => { + // Slash-command mentions (not preceded by }) should still be converted + const input = [ + '---', + 'description: Test', + '---', + '', + 'Use /gsd-discuss-phase to start a discussion.', + 'Or use /gsd-plan-phase for planning.', + 'Also: /gsd:capture --backlog adds items.', + ].join('\n'); + + const output = convertClaudeCommandToCodexSkill(input, 'gsd-test-704-cmds'); + + assert.ok( + output.includes('$gsd-discuss-phase'), + 'Expected /gsd-discuss-phase to be converted to $gsd-discuss-phase', + ); + assert.ok( + output.includes('$gsd-plan-phase'), + 'Expected /gsd-plan-phase to be converted to $gsd-plan-phase', + ); + assert.ok( + output.includes('$gsd-capture'), + 'Expected /gsd:capture to be converted to $gsd-capture', + ); + }); + + test('actual shipped workflow files: shell-variable launcher paths contain no $gsd-core', () => { + // Walk gsd-core/workflows/ and assert that no file produces $gsd-core + // inside a shell variable expansion context after Codex conversion. + // + // NOTE: The regex `/(? f.endsWith('.md')) + .map((f) => path.join(workflowsDir, f)); + + assert.ok(files.length > 0, 'Expected at least one workflow .md file'); + + // Shell-context corruption patterns from issue #704: + // - `}$gsd-*`: closing brace from `${VAR}/gsd-*` shell variable expressions + // - `)$gsd-*`: closing paren from `$(cmd)/gsd-*` command substitutions + const SHELL_CORRUPTION_RE = /[})](\$gsd-[a-z])/; + + const offending = []; + for (const file of files) { + const content = fs.readFileSync(file, 'utf8'); + const skillName = `gsd-${path.basename(file, '.md')}`; + const converted = convertClaudeCommandToCodexSkill(content, skillName); + const match = converted.match(SHELL_CORRUPTION_RE); + if (match) { + const idx = converted.indexOf(match[0]); + offending.push({ + file: path.relative(workflowsDir, file), + context: converted.substring(Math.max(0, idx - 40), idx + 80), + }); + } + } + + assert.deepStrictEqual( + offending, + [], + `Found shell-context path corruption ([})]$gsd-*) in Codex-converted workflow files (#704):\n` + + offending.map((o) => ` ${o.file}: ...${o.context}...`).join('\n'), + ); + }); + + test('commands/gsd/*.md: shell-variable launcher paths contain no $gsd-core', () => { + // Walk commands/gsd/ and assert that no command file produces the shell-context + // }$gsd-core corruption — since commands also go through + // convertClaudeCommandToCodexSkill when installed globally for Codex. + const commandsDir = path.join(__dirname, '..', 'commands', 'gsd'); + if (!fs.existsSync(commandsDir)) return; + + const files = fs.readdirSync(commandsDir) + .filter((f) => f.endsWith('.md')) + .map((f) => path.join(commandsDir, f)); + + assert.ok(files.length > 0, 'Expected at least one command .md file'); + + const SHELL_CORRUPTION_RE = /[})](\$gsd-[a-z])/; + + const offending = []; + for (const file of files) { + const content = fs.readFileSync(file, 'utf8'); + const skillName = `gsd-${path.basename(file, '.md')}`; + const converted = convertClaudeCommandToCodexSkill(content, skillName); + const match = converted.match(SHELL_CORRUPTION_RE); + if (match) { + const idx = converted.indexOf(match[0]); + offending.push({ + file: path.relative(commandsDir, file), + context: converted.substring(Math.max(0, idx - 40), idx + 80), + }); + } + } + + assert.deepStrictEqual( + offending, + [], + `Found shell-context path corruption ([})]$gsd-*) in Codex-converted command files (#704):\n` + + offending.map((o) => ` ${o.file}: ...${o.context}...`).join('\n'), + ); + }); +});