fix(ce-compound): guard validate-frontmatter.py on non-Claude platforms (#947)

Author: tmchow

AGENTS.md

@@ -170,15 +170,28 @@ Whether a relative path resolves against the skill directory depends on *who* re
 
 **Read-time file references — resolve against the skill directory:** When skill *content* points the agent at a co-located file to read (e.g., "read `references/schema.yaml`"), use a relative path from the skill root. The skill loader resolves these against the skill's own directory on all major platforms — no variable prefix needed. This is the rule in *File References in Skills* above.
 
-**Runtime script invocations via the Bash tool — resolve against the project CWD:** When skill content tells the agent to *execute* a bundled script through the Bash tool, a bare relative path does **not** work on Claude Code. The Bash tool's working directory is the user's project, not the skill directory, so `bash scripts/my-script.sh` resolves to `<project>/scripts/…`, finds nothing, and the step is silently skipped. This is a recurring bug class — see #764 (`ce-worktree`), #811 (`ce-code-review`), and #898 (`ce-compound`). Invoke the script through the skill directory with the `:-.` fallback instead:
+**Runtime script invocations via the Bash tool — resolve against the project CWD:** When skill content tells the agent to *execute* a bundled script through the Bash tool, a bare relative path does **not** work on Claude Code. The Bash tool's working directory is the user's project, not the skill directory, so `bash scripts/my-script.sh` resolves to `<project>/scripts/…`, finds nothing, and the step is silently skipped. This is a recurring bug class — see #764 (`ce-worktree`), #811 (`ce-code-review`), and #898 (`ce-compound`). Wrap the invocation in a file-existence guard on `${CLAUDE_SKILL_DIR}` so it runs on Claude Code and degrades **visibly** elsewhere:
 
 ```
-bash "${CLAUDE_SKILL_DIR:-.}/scripts/my-script.sh" ARG
+if [ -n "${CLAUDE_SKILL_DIR}" ] && [ -f "${CLAUDE_SKILL_DIR}/scripts/my-script.sh" ]; then
+  bash "${CLAUDE_SKILL_DIR}/scripts/my-script.sh" ARG
+else
+  echo "<this step's bundled script is unavailable on this platform; do X instead>"
+fi
 ```
 
-`${CLAUDE_SKILL_DIR}` is substituted into SKILL.md content by Claude Code, covering both marketplace-cached installs and `claude --plugin-dir` local dev; it resolves to the skill's own directory, so the invocation is correct there. Note `${CLAUDE_SKILL_DIR}` is a SKILL.md *content* substitution, not an environment variable available inside the executed process — a script that needs its own directory should derive it from `BASH_SOURCE` rather than reading `$CLAUDE_SKILL_DIR` (see `ce-update/scripts/`). The `ce-worktree` and `ce-update` skills are the canonical examples of this pattern.
+(The `[ -n "${CLAUDE_SKILL_DIR}" ]` guard keeps an unset variable from probing a root-level `/scripts/…` path.)
 
-**Caveat — this only fully resolves on Claude Code.** On other targets (Codex, Gemini CLI, etc.) `${CLAUDE_SKILL_DIR}` is unset, so `:-.` degrades the command to a project-CWD-relative `./scripts/…`. That keeps the command syntactically valid (no unexpanded `${…}` literal) and is no worse than a bare relative path, but it does **not** resolve to the bundled script: those runtimes also execute the shell from the user's project, while the script lives in their own skill store (e.g. `~/.codex/skills/<plugin>/<skill>/scripts/…`). So runtime invocation of a bundled script is currently *unsolved* on Codex/Gemini — the converter (`src/utils/codex-content.ts`) does not rewrite these paths, and in the default `--to codex` mode skills are not emitted by the converter at all (native plugin install owns them). Use `${CLAUDE_SKILL_DIR:-.}` because it is correct on Claude Code and harmless elsewhere — but do not gate a skill's core behavior on a runtime bundled-script call when portability to those targets matters. Prefer logic the agent can perform inline, or content the agent reads (read-time references resolve against the skill dir on all targets).
+`${CLAUDE_SKILL_DIR}` is substituted into SKILL.md content by Claude Code, covering both marketplace-cached installs and `claude --plugin-dir` local dev; it resolves to the skill's own directory, so the `then` branch runs there. Note `${CLAUDE_SKILL_DIR}` is a SKILL.md *content* substitution, not an environment variable available inside the executed process — a script that needs its own directory should derive it from `BASH_SOURCE` rather than reading `$CLAUDE_SKILL_DIR` (see `ce-update/scripts/`). `ce-compound`'s `validate-frontmatter.py` invocation is the canonical example of this guard pattern.
+
+**Why the guard, and why not the old `${CLAUDE_SKILL_DIR:-.}` shell default (issue #943).** On other targets (Codex, Gemini CLI, etc.) `${CLAUDE_SKILL_DIR}` is unset. The earlier `:-.` form degraded to a project-CWD-relative `./scripts/…` — *syntactically* valid, but resolving to a path that does not exist: the bundled script lives in that runtime's own skill store (e.g. `~/.codex/skills/<plugin>/<skill>/scripts/…`), so the call silently missed. The existence guard makes that case explicit — the `then` branch never fires, and the `else` branch tells the agent what to do instead of running a broken path or claiming success. Two facts make this a real product gap, not a converter bug:
+
+- The converter does not rewrite these paths (`src/utils/codex-content.ts` has no `CLAUDE_SKILL_DIR` case), and in the default `--to codex` mode skills are not emitted by the converter at all.
+- **This plugin also ships as a *native* Codex plugin** (installed via Codex's `/plugins` TUI marketplace). That path never runs the converter — Codex loads the raw `SKILL.md` verbatim. The `ce_platforms` frontmatter is honored only by the converter's `filterSkillsByPlatform`, so it does **not** keep a Claude-only skill out of a native Codex install. The only protection both install paths respect is the SKILL.md content itself.
+
+So: do not gate a skill's *core* behavior on a runtime bundled-script call when portability matters. The existence guard above suits an **optional** guard script (e.g. `ce-compound`'s `validate-frontmatter.py`), where the `else` branch runs an equivalent inline check so the protection still fires off-Claude instead of silently skipping. For a skill whose *entire* behavior is a bundled script, a guard does not make it work off-Claude — prefer logic the agent can perform inline, or content the agent reads (read-time references resolve against the skill dir on all targets).
+
+**Permission caveat (Claude Code).** Claude Code's permission checker evaluates every subcommand of a compound command, and a bare `[ -f … ]` test is not pre-approved — so wrapping a pinned `bash "…sh"` call in an `if … then … fi` guard defeats a narrow `Bash(bash *…sh)` allow-rule and prompts on every run. If a bundled-script call must stay auto-approved via such a pin, keep it a single pinned command rather than guarding it inline.
 
 **When a platform variable is unavoidable:** Use the pre-resolution pattern (`!` backtick syntax) and include explicit fallback instructions in the skill content, so the agent knows what to do if the value is empty, literal, or an error:
 

plugins/compound-engineering/AGENTS.md

@@ -207,8 +207,8 @@ Design rules for blocking question menus (`AskUserQuestion` / `request_user_inpu
 
 ### Script Path References in Skills
 
-- [ ] In bash code blocks, invoke co-located scripts through `${CLAUDE_SKILL_DIR:-.}` (e.g., `bash "${CLAUDE_SKILL_DIR:-.}/scripts/my-script" ARG`) — not a bare relative path, and not `${CLAUDE_PLUGIN_ROOT}`. The runtime Bash tool runs from the user's project CWD, not the skill directory, so a bare `bash scripts/my-script` resolves against `<project>/scripts/` and fails — the recurring bug class behind #764, #811, and #898.
-- [ ] On Claude Code `${CLAUDE_SKILL_DIR}` resolves to the skill directory; on other targets it is unset and the `:-.` fallback only keeps the command syntactically valid — it does **not** resolve to the bundled script there, so runtime bundled-script invocation is currently unsolved on Codex/Gemini. Do not gate a skill's core behavior on a runtime bundled-script call when portability to those targets matters. See "Permission gate on extracted scripts" under *Tool Selection in Agents and Skills* below for the `allowed-tools` pinning that pairs with this.
+- [ ] In bash code blocks, build co-located script paths from `${CLAUDE_SKILL_DIR}` — never a bare relative path and never `${CLAUDE_PLUGIN_ROOT}`. The runtime Bash tool runs from the user's project CWD, not the skill directory, so a bare `bash scripts/my-script` resolves against `<project>/scripts/` and fails — the recurring bug class behind #764, #811, #898, and #943.
+- [ ] `${CLAUDE_SKILL_DIR}` resolves only on Claude Code; on other targets — including the **native Codex plugin install**, which loads `SKILL.md` verbatim and ignores `ce_platforms` — it is unset, so the bundled script is not found there. The bare `${CLAUDE_SKILL_DIR:-.}` shell default silently runs a non-existent `./scripts/…` path off-Claude rather than failing visibly. For an **optional guard script**, prefer an existence guard `if [ -n "${CLAUDE_SKILL_DIR}" ] && [ -f "${CLAUDE_SKILL_DIR}/scripts/x" ]; then ... ; else <inline fallback>; fi` so the protection still runs off-Claude (e.g. `ce-compound`). For a skill whose core behavior *is* a bundled script, do not gate that behavior on a runtime bundled-script call when portability matters — see root `AGENTS.md` > *Platform-Specific Variables in Skills*, issue #943, and "Permission gate on extracted scripts" under *Tool Selection in Agents and Skills* below.
 - [ ] Reference the script with a backtick *read-time* path (e.g., `` `scripts/my-script` ``) so agents can locate it to read it — read-time references do resolve against the skill directory on all platforms; a markdown link is not needed since the bash code block already provides the invocation
 
 ### Cross-Platform Reference Rules

plugins/compound-engineering/skills/ce-compound-refresh/references/per-action-flows.md

@@ -62,7 +62,23 @@ Do not let replacement subagents invent frontmatter fields, enum values, or sect
    - The target path and category (same category as the old learning unless the category itself changed)
    - The relevant contents of the three support files listed above
 2. The subagent writes the new learning using the support files as the source of truth: `references/schema.yaml` for frontmatter fields and enum values, `references/yaml-schema.md` for category mapping and YAML-safety rules for array items, and `assets/resolution-template.md` for section order. It should use dedicated file search and read tools if it needs additional context beyond what was passed.
-3. **Run `python3 "${CLAUDE_SKILL_DIR:-.}/scripts/validate-frontmatter.py" <new-learning-path>`** to catch silent-corruption parser-safety issues that the prose rules miss: malformed `---` delimiter lines, unquoted ` #` in scalar values (silent comment truncation), and unquoted `: ` in scalar values (silent mapping confusion). Exit 0 means the doc is parser-safe; exit 1 means the script's stderr names the offending field(s) and what to fix — quote the value(s), re-write the doc, and re-run until exit 0. Do not declare success while validation fails. The script does not enforce schema rules and does not flag YAML reserved-indicator characters (those produce loud parser errors downstream rather than silent corruption — out of scope). Uses Python 3 stdlib only (no PyYAML or other deps). The validator ships **inside the skill bundle**, not the user's repo, and the runtime Bash tool's CWD is the project root — so a bare `scripts/validate-frontmatter.py` resolves against `<project>/scripts/`, finds nothing, and this guard is silently skipped (the parser-safety protection never runs). `${CLAUDE_SKILL_DIR}` resolves to the skill's own directory on Claude Code (marketplace-cached installs and `--plugin-dir` local dev); the `:-.` fallback keeps the command valid where it is unset.
+3. **Validate parser-safety of the new learning's frontmatter** to catch silent-corruption issues the prose rules miss: malformed `---` delimiter lines, unquoted ` #` in scalar values (silent comment truncation), and unquoted `: ` in scalar values (silent mapping confusion). The bundled validator ships **inside the skill bundle**; on Claude Code `${CLAUDE_SKILL_DIR}` resolves to the skill directory, but the runtime Bash tool's CWD is the user's project, so a project-relative path (without the `${CLAUDE_SKILL_DIR}` prefix) would miss. Run it through an existence guard so platforms that cannot locate the script (e.g. native Codex/Gemini installs, where `${CLAUDE_SKILL_DIR}` is unset) fall back to a manual check instead of silently skipping the protection:
+
+   ```bash
+   if [ -n "${CLAUDE_SKILL_DIR}" ] && [ -f "${CLAUDE_SKILL_DIR}/scripts/validate-frontmatter.py" ]; then
+     python3 "${CLAUDE_SKILL_DIR}/scripts/validate-frontmatter.py" <new-learning-path>
+   else
+     echo "Bundled validate-frontmatter.py not resolvable on this platform; applying the parser-safety checklist manually."
+   fi
+   ```
+
+   - **If the script ran:** exit 0 means parser-safe; exit 1 means stderr names the offending field(s) — quote the value(s), re-write the doc, and re-run until exit 0. Do not declare success while validation fails.
+   - **If the script did not run** (else branch): apply the validator's checks by hand, matching its exact scope — checking more broadly risks edits the validator would not require. Fix any violation by quoting the whole value before continuing:
+     1. The opening and closing frontmatter delimiters are each a line whose content is `---` (trailing whitespace is fine; `----` or `---extra` is not a valid delimiter).
+     2. For each **top-level** mapping entry (`key: value`, no leading indentation) whose value is **not already quoted or structured** (does not start with `"`, `'`, `[`, `{`, `|`, or `>`): the value must contain no unquoted ` #` (space-then-hash — YAML treats it as a comment and silently truncates) and no unquoted `: ` (colon-then-space — strict YAML may read it as a nested mapping). Quote the whole value if either appears.
+     Nested values, array items, and already-quoted values are out of scope here (array-item quoting is handled by the schema/YAML-safety step above). Then note in the completion output that the bundled script validator was unavailable on this platform and the checks were applied manually.
+
+   The validator does not enforce schema rules and does not flag YAML reserved-indicator characters (those produce loud parser errors downstream rather than silent corruption — out of scope). Uses Python 3 stdlib only (no PyYAML or other deps).
 4. After the subagent completes, the orchestrator deletes the old learning file. The new learning's frontmatter may include `supersedes: [old learning filename]` for traceability, but this is optional — the git history and commit message provide the same information.
 
 **When evidence is insufficient:**