codex: map workflows to ce-* and enforce LEARNINGS compound contract

[JayPanashe]

Summary

• map Codex workflow command skills from workflows:* to ce-* names (ce-plan, ce-work, ce-review, ce-compound)
• keep prompt names as workflows-* while wiring prompt bodies to the mapped ce-* skills
• add Codex compound transform that enforces LEARNINGS.md single-artifact contract language
• remove residual docs/solutions/... references from converted compound content for Codex target
• add regression coverage for workflow mapping, prompt linkage, and compound contract rewrite

Validation

• PATH="$HOME/.bun/bin:$PATH" bun test
• result: 189 pass, 0 fail

Notes

• base upstream synced from origin/main commit 63e76cf67f08aac6f0dccf275ce328e47deae541
• feature branch head: 152f758d4809a99ad799cbb30feebd42e30ceca9

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 152f758d48

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Remove stale category/path guidance from compound rewrite

transformCompoundArtifactContract only swaps a handful of docs/solutions strings, so the converted workflows:compound skill still contains conflicting instructions from the original template (for example, category validation and path/filename-oriented guidance from plugins/compound-engineering/commands/workflows/compound.md). In Codex output this mixes “single LEARNINGS.md in project root” with residual category/path workflow steps, which can steer the agent back toward directory-structured artifacts instead of the intended single-file contract.

Useful? React with 👍 / 👎.

[kieranklaassen]

Code Review: PR #207 — codex: map workflows to ce-* and enforce LEARNINGS compound contract

Reviewed by: Multi-agent review ensemble (TypeScript reviewer, Security sentinel, Simplicity reviewer, Architecture strategist, Pattern recognition)
Tests: 131 pass, 0 fail ✅

---

Overall Assessment

Feature A (workflow name mapping) is clean, well-tested, and follows established codebase conventions. No issues.

Feature B (compound artifact contract transform) has architectural and maintainability concerns that should be addressed before merge.

---

🔴 P1 — Must Fix Before Merge

1. transformCompoundArtifactContract is fragile — silent failure on source content changes

File: src/converters/claude-to-codex.ts:152-198

The function performs 13 sequential string.replace() calls with exact literal strings copied from the compound command body. If anyone edits workflows:compound even slightly (rewording, whitespace, punctuation), these replacements silently become no-ops — String.prototype.replace returns the original string on no match without warning.

The test validates the current wording but won't catch silent regressions if the source changes independently.

Suggestion: Add a guard assertion at the end of the function:

if (/docs\/solutions\//.test(result)) {
  throw new Error(
    "transformCompoundArtifactContract: residual docs/solutions/ references found. " +
    "The source compound.md may have changed — update the replacement patterns."
  )
}

This turns silent data corruption into a loud, actionable failure.

2. Variable shadowing of commandName

File: src/converters/claude-to-codex.ts:104,122

The outer function parameter commandName (line 104) is shadowed by the regex callback parameter also named commandName (line 122):

function transformContentForCodex(body: string, commandName?: string): string {
  // ...
  result = result.replace(slashCommandPattern, (match, commandName: string) => {
    // ← shadows outer commandName

Not a bug today (scoping works by accident), but a maintenance trap.

Fix: Rename the inner parameter to matchedCmd or capturedCommand.

---

🟡 P2 — Should Fix

3. Command-specific logic leaks into generic transform function

File: src/converters/claude-to-codex.ts:104,145

All six converters follow the pattern transformContentFor{Target}(body: string) — a pure function with no knowledge of which command is being processed. This PR breaks that convention by making transformContentForCodex command-name-aware with if (commandName === "workflows-compound").

The commandName parameter was threaded through 5 call sites (transformContentForCodex, transformTaskCalls alias, convertCommandSkill, renderPrompt) solely for one conditional check.

Suggestion: Extract the compound-specific logic to convertCommandSkill where command identity is already known, restoring the generic signature:

function convertCommandSkill(command: ClaudeCommand, usedNames: Set<string>): CodexGeneratedSkill {
  const normalizedCommandName = normalizeName(command.name)
  // ...
  let transformedBody = transformContentForCodex(command.body.trim()) // no commandName param
  if (normalizedCommandName === "workflows-compound") {
    transformedBody = transformCompoundArtifactContract(transformedBody)
  }
  sections.push(transformedBody)
  // ...
}

Same for renderPrompt. This removes the commandName parameter from transformContentForCodex entirely.

4. Redundant replacements in transformCompoundArtifactContract

File: src/converters/claude-to-codex.ts:152-198

Analysis of the 13 replacements reveals:

• 3 are fully redundant — handled by the catch-all regexes at the bottom
• 2 of 3 catch-all regexes are redundant — docs\/solutions\/[^\s\)]*/galready covers both the baredocs/solutions/` case and the backtick-wrapped case

Removing the 3 redundant specific replacements and consolidating to 1 catch-all regex reduces the function from 13 to 8 replacements with identical output.

5. .replace() only matches first occurrence for literal strings

File: src/converters/claude-to-codex.ts:154-188

String.prototype.replace(string, string) only replaces the first occurrence. If the source body has duplicate matching lines, subsequent occurrences survive unchanged. The catch-all regexes (with /g flag) mitigate this for docs/solutions/ path references, but other literal replacements (like the mkdir and SINGLE final file lines) would only match once.

Suggestion: Use replaceAll() instead of replace() for the literal string replacements, or document that first-occurrence-only is intentional.

---

🔵 P3 — Nice to Have

6. WORKFLOW_SKILL_NAME_MAP type could be narrower

const WORKFLOW_SKILL_NAME_MAP: Record<string, string> = { ... }

Could use as const satisfies Record<string, string> for better autocomplete and intent clarity.

7. Test coverage gap — unmapped commands

The "maps workflow command skills to ce-* names" test verifies all four mapped names but has no assertion that a non-workflow command passes through unchanged. An explicit assertion would strengthen the test.

8. Replacement ordering dependency is undocumented

Specific replacements must run before catch-alls. This is correct but fragile and undocumented. A comment would help future maintainers.

---

✅ What Looks Good

• WORKFLOW_SKILL_NAME_MAP approach — Clean, declarative, follows codebase conventions for Record<string, string> maps
• Prompt names preserved as workflows-* while skills get ce-* — Thoughtful design that maintains user-facing consistency
• Test coverage — 112 lines of new tests covering both features, with positive and negative assertions
• CLI test updates — Correctly updated to match new skill directory names
• Security — No exploitable vulnerabilities. Regex patterns are linear-time (no ReDoS). The normalizeName sanitizer prevents command name manipulation.

---

Summary

Severity	Count
🔴 P1	2
🟡 P2	3
🔵 P3	3

The name mapping feature is solid. The compound contract transform needs: (1) a guard assertion against silent failures, (2) the variable shadowing fixed, and (3) the command-specific logic extracted from the generic transform function. With those changes, this is ready to merge.