* refactor(#712): replace Codex slash-command denylist lookbehind with positive-boundary match The hyphen-style /gsd-<cmd> -> $gsd-<cmd> conversion in convertSlashCommandsToCodexSkillMentions used a negative-lookbehind DENYLIST enumerating characters that must NOT precede a real mention. #637 -> #704 showed this is an unbounded treadmill: each new unanticipated preceding char (/, ., word chars, then }, )) leaked the same path-corruption bug class, and a backtick-wrapped path (`/gsd-core/workflows/update.md`) still leaked through. Replace it with a POSITIVE two-boundary definition of a mention: 1. Left: opens at start-of-string, whitespace, or an inline-prose delimiter (backtick/quote/paren/bracket). 2. Right: the command token is not followed by a path separator `/` (a path continues, a command does not). The (?![a-z0-9/-]) lookahead also blocks regex backtracking to a shorter command. This closes the whole class by construction (no preceding-char denylist to maintain) and fixes the backtick-wrapped-path corruption the #704 test documented as a pre-existing gap, while preserving conversion of legitimate backtick-wrapped mentions (e.g. CONTEXT.md's `/gsd-execute-phase` lists). The colon-style /gsd: replace is intentionally left unguarded (it never appears as a filesystem path segment) and is annotated as such. Tests assert the regex directly (function now exported) across a convert/ don't-convert matrix plus one end-to-end pipeline assertion for the headline backtick-path case. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * chore(#712): add changeset fragment for PR #747 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
7
.changeset/vivid-badgers-zip.md
Normal file
7
.changeset/vivid-badgers-zip.md
Normal file
@@ -0,0 +1,7 @@
|
||||
---
|
||||
type: Changed
|
||||
pr: 747
|
||||
---
|
||||
**Codex slash-command conversion no longer corrupts inline-wrapped `/gsd-…` file paths** — the install-time converter now identifies a real `/gsd-<command>` mention by positive boundaries (opening delimiter + no path continuation) instead of an unbounded preceding-character denylist, closing the path-corruption class (#637 → #704) by construction while still converting legitimate backtick-wrapped mentions.
|
||||
|
||||
<!-- docs-exempt: internal Codex install-time converter; no command, flag, output, or user-doc surface to update -->
|
||||
@@ -2797,17 +2797,22 @@ function convertClaudeAgentToClineAgent(content) {
|
||||
// ── End Cline converters ─────────────────────────────────────────────────────
|
||||
|
||||
function convertSlashCommandsToCodexSkillMentions(content) {
|
||||
// Convert colon-style skill invocations to Codex $ prefix
|
||||
// Colon-style /gsd: never appears as a filesystem path segment, so no boundary guard is needed (unlike the hyphen-style below).
|
||||
let converted = content.replace(/\/gsd:([a-z0-9-]+)/gi, (_, commandName) => {
|
||||
return `$gsd-${String(commandName).toLowerCase()}`;
|
||||
});
|
||||
// Convert hyphen-style command references (workflow output) to Codex $ prefix.
|
||||
// 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(/(?<![a-zA-Z0-9./})])\/gsd-([a-z0-9-]+)/gi, (_, commandName) => {
|
||||
// A real /gsd-<cmd> MENTION is defined positively by two boundaries, so any
|
||||
// in-path occurrence is excluded by construction (no denylist of preceding
|
||||
// chars to maintain — see #712, supersedes the #637/#704 lookbehind treadmill):
|
||||
// 1. Left boundary: opens at start-of-string, whitespace, or an inline-prose
|
||||
// delimiter (backtick/quote/paren/bracket) — e.g. `/gsd-execute-phase`.
|
||||
// 2. Right boundary: the command token is NOT followed by a path separator
|
||||
// `/` (a path continues: `/gsd-core/bin/...`; a command does not). The
|
||||
// `(?![a-z0-9/-])` also blocks regex backtracking to a shorter command.
|
||||
// This converts backtick-wrapped MENTIONS (`/gsd-foo`) while leaving backtick-
|
||||
// wrapped PATHS (`/gsd-core/workflows/update.md`) untouched (#712).
|
||||
converted = converted.replace(/(?<=^|[\s`"'([])\/gsd-([a-z0-9-]+)(?![a-z0-9/-])/gi, (_, commandName) => {
|
||||
return `$gsd-${String(commandName).toLowerCase()}`;
|
||||
});
|
||||
return converted;
|
||||
@@ -10948,6 +10953,7 @@ module.exports = {
|
||||
install,
|
||||
installAllRuntimes,
|
||||
uninstall,
|
||||
convertSlashCommandsToCodexSkillMentions,
|
||||
convertClaudeCommandToCodexSkill,
|
||||
convertClaudeToOpencodeFrontmatter,
|
||||
convertClaudeToKiloFrontmatter,
|
||||
|
||||
@@ -23,7 +23,10 @@ const assert = require('node:assert/strict');
|
||||
const fs = require('node:fs');
|
||||
const path = require('node:path');
|
||||
|
||||
const { convertClaudeCommandToCodexSkill } = require('../bin/install.js');
|
||||
const {
|
||||
convertClaudeCommandToCodexSkill,
|
||||
convertSlashCommandsToCodexSkillMentions,
|
||||
} = require('../bin/install.js');
|
||||
|
||||
// The canonical launcher snippet path that was being corrupted
|
||||
const RUNTIME_ROOT_PATH = '${_GSD_RUNTIME_ROOT}/gsd-core/bin/${_GSD_SHIM_NAME}';
|
||||
@@ -148,11 +151,10 @@ describe('#704 — Codex global install launcher path corruption', () => {
|
||||
// Walk gsd-core/workflows/ and assert that no file produces $gsd-core
|
||||
// inside a shell variable expansion context after Codex conversion.
|
||||
//
|
||||
// NOTE: The regex `/(?<![a-zA-Z0-9./}])\/gsd-/` still converts backtick-
|
||||
// wrapped prose paths like `\`/gsd-core/workflows/update.md\`` (a pre-existing
|
||||
// issue separate from #704 — update.md's backtick is not in the lookbehind
|
||||
// set). That prose-path case is intentionally excluded from this assertion
|
||||
// (tracked separately; the primary #704 bug is the shell-variable expansion).
|
||||
// NOTE: The backtick-wrapped prose-path case (`/gsd-core/workflows/update.md`)
|
||||
// was a pre-existing gap with the #704 lookbehind fix and is now addressed by
|
||||
// the positive-boundary regex introduced in #712. That case is covered by the
|
||||
// "#712" describe block below.
|
||||
//
|
||||
// We probe for the specific shell-context pattern from the issue report:
|
||||
// BAD: ${_GSD_RUNTIME_ROOT}$gsd-core/bin/
|
||||
@@ -235,3 +237,182 @@ describe('#704 — Codex global install launcher path corruption', () => {
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('#712: positive-boundary slash-command conversion', () => {
|
||||
// Tests call convertSlashCommandsToCodexSkillMentions directly so the regex
|
||||
// is exercised in isolation — no frontmatter wrapping, no ADAPTER_CLOSE
|
||||
// stripping, no .claude→.codex rewrite masking the result.
|
||||
|
||||
// ── MUST-NOT-CONVERT (negative) cases ─────────────────────────────────────
|
||||
// These inputs must be returned UNCHANGED — no $gsd-* substitution.
|
||||
|
||||
test('backtick-wrapped path: `/gsd-core/workflows/update.md` is NOT converted (THE new fix)', () => {
|
||||
const input = 'See `/gsd-core/workflows/update.md` for details.';
|
||||
const result = convertSlashCommandsToCodexSkillMentions(input);
|
||||
assert.strictEqual(
|
||||
result,
|
||||
input,
|
||||
`Expected backtick-wrapped path to be unchanged. Got: ${result}`,
|
||||
);
|
||||
});
|
||||
|
||||
test('backtick-wrapped path deeper: `/gsd-pi/bin/foo.cjs` is NOT converted', () => {
|
||||
const input = 'Run `/gsd-pi/bin/foo.cjs` directly.';
|
||||
const result = convertSlashCommandsToCodexSkillMentions(input);
|
||||
assert.strictEqual(
|
||||
result,
|
||||
input,
|
||||
`Expected deep backtick-wrapped path to be unchanged. Got: ${result}`,
|
||||
);
|
||||
});
|
||||
|
||||
test('shell var expansion: ${_GSD_RUNTIME_ROOT}/gsd-core/bin/x is NOT converted (regression guard)', () => {
|
||||
const input = 'PATH="${_GSD_RUNTIME_ROOT}/gsd-core/bin/x"';
|
||||
const result = convertSlashCommandsToCodexSkillMentions(input);
|
||||
assert.ok(
|
||||
!result.includes('$gsd-core'),
|
||||
`Expected no $gsd-core substitution in shell var path. Got: ${result}`,
|
||||
);
|
||||
assert.ok(
|
||||
result.includes('/gsd-core/bin/x'),
|
||||
`Expected original path to be preserved. Got: ${result}`,
|
||||
);
|
||||
});
|
||||
|
||||
test('command substitution: $(expand_home ~/.claude)/gsd-local-patches is NOT converted (regression guard)', () => {
|
||||
const input = 'candidate="$(expand_home ~/.claude)/gsd-local-patches"';
|
||||
const result = convertSlashCommandsToCodexSkillMentions(input);
|
||||
assert.ok(
|
||||
!result.includes(')$gsd-local'),
|
||||
`Expected no )$gsd-local substitution. Got: ${result}`,
|
||||
);
|
||||
assert.ok(
|
||||
result.includes(')/gsd-local-patches'),
|
||||
`Expected original path to be preserved. Got: ${result}`,
|
||||
);
|
||||
});
|
||||
|
||||
test('plain path segment: bin/gsd-tools.cjs is NOT converted', () => {
|
||||
const input = 'node bin/gsd-tools.cjs --help';
|
||||
const result = convertSlashCommandsToCodexSkillMentions(input);
|
||||
assert.strictEqual(
|
||||
result,
|
||||
input,
|
||||
`Expected plain path segment to be unchanged. Got: ${result}`,
|
||||
);
|
||||
});
|
||||
|
||||
test('plain path segment: .claude/gsd-core/agents — /gsd-core portion is NOT slash-command converted', () => {
|
||||
// Tests the regex in isolation: the .claude→.codex path rewrite that happens
|
||||
// inside convertClaudeToCodexMarkdown does NOT run here. We assert directly
|
||||
// that the slash-command regex leaves /gsd-core after the slash intact —
|
||||
// i.e. the `e` in `/gsd-core` is NOT treated as a command boundary.
|
||||
const input = 'Look in .claude/gsd-core/agents for the agent files.';
|
||||
const result = convertSlashCommandsToCodexSkillMentions(input);
|
||||
assert.ok(
|
||||
!result.includes('$gsd-core'),
|
||||
`Expected no $gsd-core substitution in .claude/gsd-core path. Got: ${result}`,
|
||||
);
|
||||
assert.ok(
|
||||
result.includes('/gsd-core/agents'),
|
||||
`Expected /gsd-core/agents to remain as a path segment. Got: ${result}`,
|
||||
);
|
||||
});
|
||||
|
||||
// ── MUST-CONVERT (positive) cases ─────────────────────────────────────────
|
||||
// These inputs contain legitimate /gsd-<cmd> mentions that MUST be converted.
|
||||
|
||||
test('space-preceded prose: Use /gsd-discuss-phase to start. → $gsd-discuss-phase', () => {
|
||||
const input = 'Use /gsd-discuss-phase to start.';
|
||||
const result = convertSlashCommandsToCodexSkillMentions(input);
|
||||
assert.ok(
|
||||
result.includes('$gsd-discuss-phase'),
|
||||
`Expected /gsd-discuss-phase to be converted. Got: ${result}`,
|
||||
);
|
||||
assert.ok(
|
||||
!result.includes('/gsd-discuss-phase'),
|
||||
`Expected original /gsd-discuss-phase to be replaced. Got: ${result}`,
|
||||
);
|
||||
});
|
||||
|
||||
test('backtick-WRAPPED MENTION (single segment): Run `/gsd-execute-phase` now → `$gsd-execute-phase`', () => {
|
||||
// A backtick-wrapped COMMAND (single segment, no path continuation) MUST
|
||||
// still be converted — this guards against a naive whitespace-only fix.
|
||||
const input = 'Run `/gsd-execute-phase` now.';
|
||||
const result = convertSlashCommandsToCodexSkillMentions(input);
|
||||
assert.ok(
|
||||
result.includes('`$gsd-execute-phase`'),
|
||||
`Expected backtick-wrapped command to be converted to \`$gsd-execute-phase\`. Got: ${result}`,
|
||||
);
|
||||
assert.ok(
|
||||
!result.includes('`/gsd-execute-phase`'),
|
||||
`Expected original \`/gsd-execute-phase\` to be replaced. Got: ${result}`,
|
||||
);
|
||||
});
|
||||
|
||||
test('parenthetical/backtick list like CONTEXT.md:59: (`/gsd-plan-phase`, `/gsd-progress`) → converted', () => {
|
||||
const input = 'Available commands: (`/gsd-plan-phase`, `/gsd-progress`) — pick one.';
|
||||
const result = convertSlashCommandsToCodexSkillMentions(input);
|
||||
assert.ok(
|
||||
result.includes('`$gsd-plan-phase`'),
|
||||
`Expected /gsd-plan-phase to be converted. Got: ${result}`,
|
||||
);
|
||||
assert.ok(
|
||||
result.includes('`$gsd-progress`'),
|
||||
`Expected /gsd-progress to be converted. Got: ${result}`,
|
||||
);
|
||||
});
|
||||
|
||||
test('start-of-string: /gsd-manager runs → $gsd-manager runs (exercises the ^ branch of lookbehind)', () => {
|
||||
// This case is IMPOSSIBLE to test through the frontmatter-wrapping pipeline
|
||||
// (the body always has preceding chars). Direct call exercises the ^ branch.
|
||||
const input = '/gsd-manager runs the pipeline.';
|
||||
const result = convertSlashCommandsToCodexSkillMentions(input);
|
||||
assert.ok(
|
||||
result.includes('$gsd-manager'),
|
||||
`Expected /gsd-manager to be converted. Got: ${result}`,
|
||||
);
|
||||
assert.ok(
|
||||
!result.includes('/gsd-manager'),
|
||||
`Expected original /gsd-manager to be replaced. Got: ${result}`,
|
||||
);
|
||||
});
|
||||
|
||||
test('double-quote wrapped: "/gsd-resume" → "$gsd-resume"', () => {
|
||||
const input = 'Call "/gsd-resume" to continue.';
|
||||
const result = convertSlashCommandsToCodexSkillMentions(input);
|
||||
assert.ok(
|
||||
result.includes('"$gsd-resume"'),
|
||||
`Expected "/gsd-resume" to be converted to "$gsd-resume". Got: ${result}`,
|
||||
);
|
||||
assert.ok(
|
||||
!result.includes('"/gsd-resume"'),
|
||||
`Expected original "/gsd-resume" to be replaced. Got: ${result}`,
|
||||
);
|
||||
});
|
||||
|
||||
// ── End-to-end: headline #712 bug through the real install pipeline ────────
|
||||
|
||||
test('end-to-end: backtick-wrapped path `/gsd-core/workflows/update.md` survives full Codex install pipeline', () => {
|
||||
// Uses convertClaudeCommandToCodexSkill (same pattern as #704 tests above)
|
||||
// to prove the real install path does not corrupt prose references to repo paths.
|
||||
const input = [
|
||||
'---',
|
||||
'description: Test',
|
||||
'---',
|
||||
'',
|
||||
'See `/gsd-core/workflows/update.md` for the update workflow.',
|
||||
].join('\n');
|
||||
|
||||
const output = convertClaudeCommandToCodexSkill(input, 'gsd-test-712-e2e');
|
||||
|
||||
assert.ok(
|
||||
!output.includes('$gsd-core'),
|
||||
`Expected no $gsd-core in converted output. Got:\n${output}`,
|
||||
);
|
||||
assert.ok(
|
||||
output.includes('/gsd-core/workflows/update.md'),
|
||||
`Expected backtick-wrapped path to survive conversion. Got:\n${output}`,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user