fix(skills): replace shell antipatterns blocked by permission check

Two safety-check rejections in `!` backtick pre-resolutions were breaking skill-load in Claude Code: - `[A] && B || C` shape ("ambiguous syntax with command separators", issue #710): ce-setup, ce-update, ce-work-beta/codex-delegation-workflow. - `$()` containing a double-quoted string ("Unhandled node type: string", issue #709): ce-compound, ce-sessions, ce-update, ce-work-beta. Replaces each with a safe shape: raw env-var emit, pure-pipe sed, `${var%suffix}` parameter expansion, or an extracted script under the skill's `scripts/`. ce-update gets three scripts (upstream-version, currently-loaded-version, marketplace-name) since it's Claude-only and has the most complex pre-resolution logic. Adds regression tests in tests/skill-shell-safety.test.ts that flag both antipatterns at PR time, and updates AGENTS.md to enumerate the rejected shapes alongside the existing `case`/`esac` rule. Closes #709, #710. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-06-19 15:41:46 +02:00 · 2026-04-27 21:05:03 -07:00
parent 41e7f72ab6
commit 8605e0f96c
12 changed files with 251 additions and 22 deletions
@@ -225,7 +225,12 @@ Why: shell-heavy exploration causes avoidable permission prompts in sub-agent wo
 - [ ] Never instruct agents to use `find`, `ls`, `cat`, `head`, `tail`, `grep`, `rg`, `wc`, or `tree` through a shell for routine file discovery, content search, or file reading
 - [ ] Describe tools by capability class with platform hints — e.g., "Use the native file-search/glob tool (e.g., Glob in Claude Code)" — not by Claude Code-specific tool names alone
 - [ ] When shell is the only option (e.g., `ast-grep`, `bundle show`, git commands), instruct one simple command at a time — no action chaining (`cmd1 && cmd2`, `cmd1 ; cmd2`) and no error suppression (`2>/dev/null`, `|| true`). Two narrow exceptions: boolean conditions within if/while guards (`[ -n "$X" ] || [ -n "$Y" ]`) are fine — that is normal conditional logic, not action chaining. **Value-producing preparatory commands** (`VAR=$(cmd1) && cmd2 "$VAR"`) are also fine when `cmd2` strictly consumes `cmd1`'s output and splitting would require manually threading the value through model context across bash calls (e.g., `BODY_FILE=$(mktemp -u) && cat > "$BODY_FILE" <<EOF ... EOF`). Simple pipes (e.g., `| jq .field`) and output redirection (e.g., `> file`) are acceptable when they don't obscure failures
- [ ] **Pre-resolution exception:** `!` backtick pre-resolution commands run at skill load time, not at agent runtime. They may use chaining (`&&`, `||`), error suppression (`2>/dev/null`), and fallback sentinels (e.g., `|| echo '__NO_CONFIG__'`) to produce a clean, parseable value for the model. This is the preferred pattern for environment probes (CLI availability, config file reads) that would otherwise require runtime shell calls with chaining. Example: `` !`command -v codex >/dev/null 2>&1 && echo "AVAILABLE" || echo "NOT_FOUND"` ``
+- [ ] **Pre-resolution exception:** `!` backtick pre-resolution commands run at skill load time, not at agent runtime. They may use chaining (`&&`, `||`), error suppression (`2>/dev/null`), and fallback sentinels (e.g., `|| echo '__NO_CONFIG__'`) to produce a clean, parseable value for the model. This is the preferred pattern for environment probes (CLI availability, config file reads) that would otherwise require runtime shell calls with chaining. Three shapes are rejected by Claude Code's safety check and must be avoided in `!` backticks:
+  - **`case ... esac`** is rejected as `Contains case_statement`. Use `&&` chaining or pipe-to-sed, or extract to a script.
+  - **`[A] && B || C`** (mixing `&&` and `||` at the same lexical depth) is rejected as `ambiguous syntax with command separators` (issue #710). Wrap the `&&` chain in a subshell so only `||` remains at top level — `(A && B) || C` — or emit the raw value and let the agent's prose decide. Example of the safe shape: `` !`(top=$(...); [ -n "$top" ] && cat "$top/file") || echo '__SENTINEL__'` ``
+  - **`$(...)` containing a double-quoted string** (e.g., `basename "$(dirname "$common")"`) is rejected as `Unhandled node type: string` (issue #709). Replace nested `$()` with parameter expansion (`${common%/.git}`), pipe to sed (`| sed -E 's|/\.git/?$||; s|.*/||'`), or extract to a script invoked as `` !`bash "${CLAUDE_SKILL_DIR}/scripts/<name>.sh"` ``.
+
+  When the logic is non-trivial, prefer extracting to a script under the skill's `scripts/` directory; the safety check then sees only `bash <quoted-path>`, which sidesteps both current and future safety-check tightenings. Tests in `tests/skill-shell-safety.test.ts` enforce all three patterns.
 - [ ] Do not encode shell recipes for routine exploration when native tools can do the job; encode intent and preferred tool classes instead
 - [ ] For shell-only workflows (e.g., `gh`, `git`, `bundle show`, project CLIs), explicit command examples are acceptable when they are simple, task-scoped, and not chained together

@@ -245,9 +250,9 @@ Plugin config lives at `.compound-engineering/config.local.yaml` in the repo roo

 2. **Worktrees:** Gitignored files are per-worktree. A config file created in the main checkout does not exist in worktrees. When reading config, fall back to the main repo root if the file is missing in the current worktree:
   ```
-   !`(top=$(git rev-parse --show-toplevel 2>/dev/null); [ -n "$top" ] && cat "$top/.compound-engineering/config.local.yaml" 2>/dev/null) || (common=$(git rev-parse --path-format=absolute --git-common-dir 2>/dev/null); [ -n "$common" ] && cat "$(dirname "$common")/.compound-engineering/config.local.yaml" 2>/dev/null) || echo '__NO_CONFIG__'`
+   !`(top=$(git rev-parse --show-toplevel 2>/dev/null); [ -n "$top" ] && cat "$top/.compound-engineering/config.local.yaml" 2>/dev/null) || (common=$(git rev-parse --path-format=absolute --git-common-dir 2>/dev/null); main="${common%/.git}"; [ -n "$main" ] && cat "$main/.compound-engineering/config.local.yaml" 2>/dev/null) || echo '__NO_CONFIG__'`
   ```
-   The first subshell tries the current worktree root. The second derives the main repo root from `git-common-dir` as a fallback. The `[ -n "$top" ]` and `[ -n "$common" ]` guards matter: outside a git repo, both `git rev-parse` invocations emit empty, and an unguarded `cat "$(dirname "")/.compound-engineering/config.local.yaml"` would resolve to a CWD-relative `./...config.local.yaml` and could succeed against a stray file in the user's working directory. Guarded, both branches simply fail and the `__NO_CONFIG__` sentinel takes over. In a regular (non-worktree) checkout, both repo paths are identical.
+   The first subshell tries the current worktree root. The second derives the main repo root from `git-common-dir` (stripping the trailing `/.git` with `${common%/.git}`) as a fallback. Use parameter expansion rather than `dirname` because Claude Code's safety check rejects nested `$(dirname "$common")` inside double-quoted strings as "Unhandled node type: string" (see issue #709). The `[ -n "$top" ]` and `[ -n "$main" ]` guards matter: outside a git repo, both `git rev-parse` invocations emit empty, and an unguarded `cat "/.compound-engineering/config.local.yaml"` would attempt to read from `/`. Guarded, both branches simply fail and the `__NO_CONFIG__` sentinel takes over. In a regular (non-worktree) checkout, both repo paths are identical.

 If neither path has the file, fall through to defaults — never fail or block on missing config.

@@ -22,7 +22,7 @@ Captures problem solutions while context is fresh, creating structured documenta

 ## Pre-resolved context

-**Repo name (pre-resolved):** !`common=$(git rev-parse --path-format=absolute --git-common-dir 2>/dev/null) && [ -n "$common" ] && basename "$(dirname "$common")"`
+**Repo name (pre-resolved):** !`git rev-parse --path-format=absolute --git-common-dir 2>/dev/null | sed -E 's|/\.git/?$||; s|.*/||'`

 **Git branch (pre-resolved):** !`git rev-parse --abbrev-ref HEAD 2>/dev/null`

@@ -16,7 +16,7 @@ Search your session history.

 ## Pre-resolved context

-**Repo name (pre-resolved):** !`common=$(git rev-parse --path-format=absolute --git-common-dir 2>/dev/null) && [ -n "$common" ] && basename "$(dirname "$common")"`
+**Repo name (pre-resolved):** !`git rev-parse --path-format=absolute --git-common-dir 2>/dev/null | sed -E 's|/\.git/?$||; s|.*/||'`

 **Git branch (pre-resolved):** !`git rev-parse --abbrev-ref HEAD 2>/dev/null`

@@ -42,9 +42,9 @@ Display the script's output to the user.

 ### Step 3: Evaluate Results

-**Platform detection (pre-resolved):** !`[ -n "${CLAUDE_PLUGIN_ROOT}" ] && echo "CLAUDE_CODE" || echo "OTHER"`
+**Plugin root (pre-resolved):** !`echo "${CLAUDE_PLUGIN_ROOT}"`

-If the line above resolved to `CLAUDE_CODE`, this is a Claude Code session and `/ce-update` is available. Otherwise, omit any `/ce-update` references from output.
+If the line above resolved to a non-empty path, this is a Claude Code session and `/ce-update` is available. If it is empty or shows the literal string `${CLAUDE_PLUGIN_ROOT}`, omit any `/ce-update` references from output.

 After the diagnostic report, check whether:

@@ -66,7 +66,7 @@ If everything is installed, no repo-local cleanup is needed, and `.compound-engi
    Run /ce-setup anytime to re-check.
 ```

-If this is a Claude Code session, append to the message: "Run /ce-update to grab the latest plugin version."
+If this is a Claude Code session (the **Plugin root** above resolved to a non-empty path), append to the message: "Run /ce-update to grab the latest plugin version."

 Stop here.

@@ -39,13 +39,13 @@ between releases).
 !`echo "${CLAUDE_SKILL_DIR}"`

 **Latest upstream version:**
-!`version=$(gh api repos/EveryInc/compound-engineering-plugin/contents/plugins/compound-engineering/.claude-plugin/plugin.json --jq '.content | @base64d | fromjson | .version' 2>/dev/null) && [ -n "$version" ] && echo "$version" || echo '__CE_UPDATE_VERSION_FAILED__'`
+!`bash "${CLAUDE_SKILL_DIR}/scripts/upstream-version.sh"`

 **Currently loaded version:**
-!`echo "${CLAUDE_SKILL_DIR}" | grep -q "/plugins/cache/.*/compound-engineering/.*/skills/ce-update$" && basename "$(dirname "$(dirname "${CLAUDE_SKILL_DIR}")")" || echo '__CE_UPDATE_NOT_MARKETPLACE__'`
+!`bash "${CLAUDE_SKILL_DIR}/scripts/currently-loaded-version.sh"`

 **Marketplace name:**
-!`echo "${CLAUDE_SKILL_DIR}" | grep -q "/plugins/cache/.*/compound-engineering/.*/skills/ce-update$" && basename "$(dirname "$(dirname "$(dirname "$(dirname "${CLAUDE_SKILL_DIR}")")")")" || echo '__CE_UPDATE_NOT_MARKETPLACE__'`
+!`bash "${CLAUDE_SKILL_DIR}/scripts/marketplace-name.sh"`

 ## Decision logic

@@ -0,0 +1,23 @@
+#!/usr/bin/env bash
+# Print the version segment of CLAUDE_SKILL_DIR when it matches the marketplace
+# cache layout `~/.claude/plugins/cache/<marketplace>/compound-engineering/<version>/skills/ce-update`,
+# or the literal sentinel `__CE_UPDATE_NOT_MARKETPLACE__` otherwise.
+
+set -u
+
+skill_dir="${CLAUDE_SKILL_DIR:-}"
+
+if [ -z "$skill_dir" ]; then
+  echo '__CE_UPDATE_NOT_MARKETPLACE__'
+  exit 0
+fi
+
+# Match `.../plugins/cache/*/compound-engineering/<version>/skills/ce-update[/]?`
+# Capture group 1 is the version segment.
+version=$(printf '%s\n' "$skill_dir" | sed -nE 's|.*/plugins/cache/[^/]+/compound-engineering/([^/]+)/skills/ce-update/?$|\1|p')
+
+if [ -n "$version" ]; then
+  echo "$version"
+else
+  echo '__CE_UPDATE_NOT_MARKETPLACE__'
+fi
@@ -0,0 +1,22 @@
+#!/usr/bin/env bash
+# Print the marketplace-name segment of CLAUDE_SKILL_DIR when it matches the
+# marketplace cache layout `~/.claude/plugins/cache/<marketplace>/compound-engineering/<version>/skills/ce-update`,
+# or the literal sentinel `__CE_UPDATE_NOT_MARKETPLACE__` otherwise.
+
+set -u
+
+skill_dir="${CLAUDE_SKILL_DIR:-}"
+
+if [ -z "$skill_dir" ]; then
+  echo '__CE_UPDATE_NOT_MARKETPLACE__'
+  exit 0
+fi
+
+# Capture group 1 is the marketplace segment.
+marketplace=$(printf '%s\n' "$skill_dir" | sed -nE 's|.*/plugins/cache/([^/]+)/compound-engineering/[^/]+/skills/ce-update/?$|\1|p')
+
+if [ -n "$marketplace" ]; then
+  echo "$marketplace"
+else
+  echo '__CE_UPDATE_NOT_MARKETPLACE__'
+fi
@@ -0,0 +1,17 @@
+#!/usr/bin/env bash
+# Print the upstream `version` field from plugins/compound-engineering/.claude-plugin/plugin.json
+# on main, or the literal sentinel `__CE_UPDATE_VERSION_FAILED__` if the lookup fails.
+#
+# Compared to release tags, this reads the current main HEAD because the marketplace
+# installs plugin contents from main HEAD; comparing against tags false-positives
+# whenever main is ahead of the last tag.
+
+set -u
+
+version=$(gh api repos/EveryInc/compound-engineering-plugin/contents/plugins/compound-engineering/.claude-plugin/plugin.json --jq '.content | @base64d | fromjson | .version' 2>/dev/null)
+
+if [ -n "$version" ]; then
+  echo "$version"
+else
+  echo '__CE_UPDATE_VERSION_FAILED__'
+fi
@@ -43,7 +43,7 @@ After extracting tokens from arguments, resolve the delegation state using this
 3. **Hard default** -- `false` (delegation off)

 **Config (pre-resolved):**
-!`(top=$(git rev-parse --show-toplevel 2>/dev/null); [ -n "$top" ] && cat "$top/.compound-engineering/config.local.yaml" 2>/dev/null) || (common=$(git rev-parse --path-format=absolute --git-common-dir 2>/dev/null); [ -n "$common" ] && cat "$(dirname "$common")/.compound-engineering/config.local.yaml" 2>/dev/null) || echo '__NO_CONFIG__'`
+!`(top=$(git rev-parse --show-toplevel 2>/dev/null); [ -n "$top" ] && cat "$top/.compound-engineering/config.local.yaml" 2>/dev/null) || (common=$(git rev-parse --path-format=absolute --git-common-dir 2>/dev/null); main="${common%/.git}"; [ -n "$main" ] && cat "$main/.compound-engineering/config.local.yaml" 2>/dev/null) || echo '__NO_CONFIG__'`

 If the block above contains YAML key-value pairs, extract values for the keys listed below.
 If it shows `__NO_CONFIG__`, the file does not exist — all settings fall through to defaults.
@@ -55,11 +55,11 @@ If `inside_sandbox` is true, delegation would recurse or fail.

 **2. Availability Check**

-**Codex availability (pre-resolved):**
-!`command -v codex >/dev/null 2>&1 && echo "CODEX_AVAILABLE" || echo "CODEX_NOT_FOUND"`
+**Codex CLI path (pre-resolved):**
+!`command -v codex 2>/dev/null`

-If the line above shows `CODEX_AVAILABLE`, proceed to the next check.
-If it shows `CODEX_NOT_FOUND`, the Codex CLI is not installed. Emit "Codex CLI not found (install via `npm install -g @openai/codex` or `brew install codex`) -- using standard mode." and set `delegation_active` to false.
+If the line above shows a non-empty path (e.g., `/opt/homebrew/bin/codex`), the Codex CLI is available — proceed to the next check.
+If it is empty, the Codex CLI is not installed. Emit "Codex CLI not found (install via `npm install -g @openai/codex` or `brew install codex`) -- using standard mode." and set `delegation_active` to false.
 If it shows an unresolved command string, run `command -v codex` using a shell tool. If the command prints a path, proceed. If it fails or prints nothing, emit the same message and set `delegation_active` to false.

 **3. Consent Flow**
@@ -4,17 +4,24 @@ import { describe, expect, test } from "bun:test"

 /**
 * Skill `!` backtick pre-resolution commands run through Claude Code's shell
- * permission checker at skill-load time. The checker rejects bash `case`
- * statements outright (`Error: ... Contains case_statement`), which fails the
- * skill before its body ever runs. AGENTS.md guidance for the `!`
- * pre-resolution exception allows `&&`, `||`, `2>/dev/null`, and fallback
- * sentinels — but `case ... esac` is not on that allowlist.
+ * permission checker at skill-load time. The checker rejects several patterns
+ * outright, failing the skill before its body ever runs. AGENTS.md guidance
+ * for the `!` pre-resolution exception allows `&&`, `||`, `2>/dev/null`, and
+ * fallback sentinels — but several specific shapes are not on that allowlist.
 *
 * Past incidents:
 *   - PR #699 introduced a `case "$common" in /*) ... ;; *) ... ;; esac` block
 *     into ce-compound and ce-sessions to derive a worktree-stable repo name.
 *     The cleaner replacement is
 *     `basename "$(dirname "$(git rev-parse --path-format=absolute --git-common-dir 2>/dev/null)")"`.
+ *   - PR #701 replaced the `case` blocks with `[A] && B || C` chains, which
+ *     trip a different rejection: "ambiguous syntax with command separators".
+ *     Issue #710. Fix: wrap the `&&` chain in a subshell, or split into
+ *     scripts so the safety check sees only `bash <path>`.
+ *   - The `basename "$(dirname "$common")"` shape (a double-quoted string
+ *     containing `$()` containing another double-quoted string) trips
+ *     "Unhandled node type: string". Issue #709. Fix: replace nested `$()`
+ *     with parameter expansion, pipe to sed, or extract to a script.
 */

 const PLUGIN_SKILLS_GLOB = ["plugins/compound-engineering/skills", "plugins/coding-tutor/skills"]
@@ -54,6 +61,83 @@ function findPreResolutionCommands(body: string): { lineNumber: number; command:
  return found
 }

+/**
+ * Returns true when both `&&` and `||` appear at the same lexical depth (not
+ * inside `( ... )` subshells or `$( ... )` command substitutions, and not
+ * inside quoted strings). This is the `[A] && B || C` shell antipattern that
+ * Claude Code's safety check rejects as "ambiguous syntax".
+ */
+function hasTopLevelMixedAndOr(cmd: string): boolean {
+  let depth = 0
+  let inSingleQuote = false
+  let inDoubleQuote = false
+  let andAtDepth0 = false
+  let orAtDepth0 = false
+
+  for (let i = 0; i < cmd.length; i++) {
+    const c = cmd[i]
+    const next = cmd[i + 1]
+
+    if (!inDoubleQuote && c === "'") { inSingleQuote = !inSingleQuote; continue }
+    if (!inSingleQuote && c === '"') { inDoubleQuote = !inDoubleQuote; continue }
+    if (inSingleQuote || inDoubleQuote) continue
+
+    if (c === '$' && next === '(') { depth++; i++; continue }
+    if (c === '(') { depth++; continue }
+    if (c === ')') { depth--; continue }
+
+    if (depth === 0) {
+      if (c === '&' && next === '&') { andAtDepth0 = true; i++; continue }
+      if (c === '|' && next === '|') { orAtDepth0 = true; i++; continue }
+    }
+  }
+
+  return andAtDepth0 && orAtDepth0
+}
+
+/**
+ * Returns the contents of every top-level `$(...)` in the command, with
+ * matched parens preserved correctly even when nested. Used to detect the
+ * "Unhandled node type: string" pattern (a `$(...)` whose contents contain
+ * a double-quoted string).
+ */
+function findCommandSubstitutionContents(cmd: string): string[] {
+  const results: string[] = []
+  let i = 0
+  let inSingleQuote = false
+  while (i < cmd.length) {
+    const c = cmd[i]
+    if (c === "'" && !inSingleQuote) { inSingleQuote = true; i++; continue }
+    if (c === "'" && inSingleQuote) { inSingleQuote = false; i++; continue }
+    if (inSingleQuote) { i++; continue }
+    if (c === '$' && cmd[i + 1] === '(') {
+      let depth = 1
+      let j = i + 2
+      const start = j
+      while (j < cmd.length && depth > 0) {
+        if (cmd[j] === '$' && cmd[j + 1] === '(') { depth++; j += 2; continue }
+        if (cmd[j] === '(') { depth++; j++; continue }
+        if (cmd[j] === ')') { depth--; j++; continue }
+        j++
+      }
+      results.push(cmd.slice(start, Math.max(start, j - 1)))
+      i = j
+      continue
+    }
+    i++
+  }
+  return results
+}
+
+/**
+ * Returns true when any `$(...)` in the command contains a double-quoted
+ * string — the shape that trips Claude Code's "Unhandled node type: string"
+ * rejection (e.g., `basename "$(dirname "$common")"`).
+ */
+function hasNestedQuotedStringInCommandSubst(cmd: string): boolean {
+  return findCommandSubstitutionContents(cmd).some(s => s.includes('"'))
+}
+
 describe("findPreResolutionCommands", () => {
  test("captures single-line `!` blocks with correct line numbers", () => {
    const sample = "intro\n!`echo hi` mid !`echo bye`\nend"
@@ -72,6 +156,50 @@ describe("findPreResolutionCommands", () => {
  })
 })

+describe("hasTopLevelMixedAndOr", () => {
+  test("flags the `[A] && B || C` antipattern", () => {
+    expect(hasTopLevelMixedAndOr('[ -n "$x" ] && echo yes || echo no')).toBe(true)
+  })
+
+  test("does not flag `&&`-only chains", () => {
+    expect(hasTopLevelMixedAndOr('a=$(cmd) && [ -n "$a" ] && echo "$a"')).toBe(false)
+  })
+
+  test("does not flag `||`-only chains", () => {
+    expect(hasTopLevelMixedAndOr("cmd 2>/dev/null || echo fallback")).toBe(false)
+  })
+
+  test("does not flag `&&` inside subshells with `||` only at top level", () => {
+    expect(hasTopLevelMixedAndOr('(a && b) || (c && d) || echo fallback')).toBe(false)
+  })
+
+  test("does not flag operators inside quoted strings", () => {
+    expect(hasTopLevelMixedAndOr('echo "a && b || c"')).toBe(false)
+  })
+})
+
+describe("hasNestedQuotedStringInCommandSubst", () => {
+  test("flags `basename \"$(dirname \"$common\")\"`", () => {
+    expect(hasNestedQuotedStringInCommandSubst('basename "$(dirname "$common")"')).toBe(true)
+  })
+
+  test("flags deeply nested `$(dirname \"$(dirname \"$x\")\")`", () => {
+    expect(hasNestedQuotedStringInCommandSubst('basename "$(dirname "$(dirname "$x")")"')).toBe(true)
+  })
+
+  test("does not flag `$(...)` whose contents only contain single-quoted strings", () => {
+    expect(hasNestedQuotedStringInCommandSubst("a=$(gh api endpoint --jq '.field')")).toBe(false)
+  })
+
+  test("does not flag `$(...)` with no quoted strings inside", () => {
+    expect(hasNestedQuotedStringInCommandSubst('a=$(git rev-parse HEAD 2>/dev/null)')).toBe(false)
+  })
+
+  test("does not flag double-quoted strings outside any `$(...)`", () => {
+    expect(hasNestedQuotedStringInCommandSubst('echo "${VAR}/path"')).toBe(false)
+  })
+})
+
 describe("skill `!` pre-resolution commands avoid Claude Code denylist", () => {
  const files = listSkillFiles()

@@ -93,5 +221,31 @@ describe("skill `!` pre-resolution commands avoid Claude Code denylist", () => {
        `Claude Code rejects \`case ... esac\` in \`!\` pre-resolution commands. Use \`if\`/\`then\`/\`else\` or \`&&\`/\`||\` chaining, or \`git rev-parse --path-format=absolute --git-common-dir\` for worktree-stable repo names.\nOffending commands:\n${formatted}`,
      ).toEqual([])
    })
+
+    test(`${rel} pre-resolution commands do not mix \`&&\` and \`||\` at top level (issue #710)`, () => {
+      const offenders = preResolutionCommands.filter(({ command }) =>
+        hasTopLevelMixedAndOr(command),
+      )
+      const formatted = offenders
+        .map(({ lineNumber, command }) => `  line ${lineNumber}: ${command}`)
+        .join("\n")
+      expect(
+        offenders,
+        `Claude Code rejects the \`[A] && B || C\` antipattern as "ambiguous syntax with command separators". Wrap the \`&&\` chain in a subshell so only \`||\` remains at top level — \`(A && B) || C\` — or extract to a script.\nOffending commands:\n${formatted}`,
+      ).toEqual([])
+    })
+
+    test(`${rel} pre-resolution commands do not nest double-quoted strings inside \`$(...)\` (issue #709)`, () => {
+      const offenders = preResolutionCommands.filter(({ command }) =>
+        hasNestedQuotedStringInCommandSubst(command),
+      )
+      const formatted = offenders
+        .map(({ lineNumber, command }) => `  line ${lineNumber}: ${command}`)
+        .join("\n")
+      expect(
+        offenders,
+        `Claude Code rejects \`$(...)\` containing a double-quoted string as "Unhandled node type: string" (e.g., \`basename "$(dirname "$common")"\`). Replace nested \`$()\` with parameter expansion (\`\${var%/suffix}\`), pipe to sed, or extract to a script invoked as \`bash "\${CLAUDE_SKILL_DIR}/scripts/<name>.sh"\`.\nOffending commands:\n${formatted}`,
+      ).toEqual([])
+    })
  }
 })
@@ -155,8 +155,16 @@ esac
    writeFileSync(ghPath, ghScript)
    chmodSync(ghPath, 0o755)

+    // The pre-resolution command invokes a script under
+    // ${CLAUDE_SKILL_DIR}/scripts/. Point CLAUDE_SKILL_DIR at the real skill
+    // directory so the script resolves; this exercises the full script path
+    // (substitution, script lookup, and gh invocation) end-to-end.
    return execFileSync("bash", ["-c", command], {
-      env: { ...process.env, PATH: `${mockDir}:${process.env.PATH ?? ""}` },
+      env: {
+        ...process.env,
+        PATH: `${mockDir}:${process.env.PATH ?? ""}`,
+        CLAUDE_SKILL_DIR: path.dirname(SKILL_PATH),
+      },
      encoding: "utf8",
    }).trim()
  } finally {