fix(ce-compound,ce-sessions): handle non-git CWD in pre-resolved git branch (#731)

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: tmchow

plugins/compound-engineering/skills/ce-compound/SKILL.md

@@ -24,7 +24,7 @@ Captures problem solutions while context is fresh, creating structured documenta
 
 **Repo name (pre-resolved):** !`common=$(git rev-parse --path-format=absolute --git-common-dir 2>/dev/null); repo="${common%/.git}"; echo "${repo##*/}"`
 
-**Git branch (pre-resolved):** !`git rev-parse --abbrev-ref HEAD 2>/dev/null`
+**Git branch (pre-resolved):** !`git rev-parse --abbrev-ref HEAD 2>/dev/null || true`
 
 If the lines above resolved to plain values (a folder name like `my-repo` and a branch name like `feat/my-branch`), pass them into the Session Historian dispatch in Phase 1 so the agent does not waste a turn deriving them. If they still contain backtick command strings or are empty, omit them from the dispatch and let the agent derive them at runtime.
 

plugins/compound-engineering/skills/ce-sessions/SKILL.md

@@ -18,7 +18,7 @@ Search your session history.
 
 **Repo name (pre-resolved):** !`common=$(git rev-parse --path-format=absolute --git-common-dir 2>/dev/null); repo="${common%/.git}"; echo "${repo##*/}"`
 
-**Git branch (pre-resolved):** !`git rev-parse --abbrev-ref HEAD 2>/dev/null`
+**Git branch (pre-resolved):** !`git rev-parse --abbrev-ref HEAD 2>/dev/null || true`
 
 If the lines above resolved to plain values (a folder name like `my-repo` and a branch name like `feat/my-branch`), they are ready to pass to the agent. If they still contain backtick command strings or are empty, they did not resolve — omit them from the dispatch and let the agent derive them at runtime.
 

plugins/compound-engineering/skills/ce-work-beta/references/codex-delegation-workflow.md

@@ -56,7 +56,7 @@ If `inside_sandbox` is true, delegation would recurse or fail.
 **2. Availability Check**
 
 **Codex CLI path (pre-resolved):**
-!`command -v codex 2>/dev/null`
+!`command -v codex 2>/dev/null || true`
 
 If the line above shows an absolute path (starts with `/`, e.g., `/opt/homebrew/bin/codex`), the Codex CLI is available — proceed to the next check.
 Otherwise — empty, an unresolved command string like `command -v codex 2>/dev/null` left in place by a non-Claude harness that doesn't process `!` pre-resolution, or any other non-path value — run `command -v codex` via the shell/Bash tool to verify at runtime. If that prints an absolute path, the Codex CLI is available; proceed. If it fails or prints nothing, 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.

tests/skill-shell-safety.test.ts

@@ -25,6 +25,11 @@ import { describe, expect, test } from "bun:test"
  *   - ce-compound and ce-sessions used `git rev-parse ... | sed -E '...'` which
  *     trips the permission checker as "multiple operations". Fix: replace the
  *     pipe with bash parameter expansion (e.g. strip suffix, strip prefix).
+ *   - ce-compound and ce-sessions used `git rev-parse --abbrev-ref HEAD 2>/dev/null`
+ *     with no fallback. Outside a git repo, `git rev-parse` exits 128;
+ *     `2>/dev/null` suppresses stderr but the non-zero exit propagates and
+ *     Claude Code reports "Shell command failed for pattern" (issue #730). Fix:
+ *     pair `2>/dev/null` with `|| true` or `|| echo '__SENTINEL__'`.
  */
 
 const PLUGIN_SKILLS_GLOB = ["plugins/compound-engineering/skills", "plugins/coding-tutor/skills"]
@@ -141,6 +146,45 @@ function hasNestedQuotedStringInCommandSubst(cmd: string): boolean {
   return findCommandSubstitutionContents(cmd).some(s => s.includes('"'))
 }
 
+/**
+ * Returns true when the command's *trailing* top-level statement suppresses
+ * stderr with `2>/dev/null` but has no `||` fallback. The trailing statement
+ * determines the command's overall exit code (after `;` or `&&` chains). When
+ * that exit code is non-zero, Claude Code reports "Shell command failed for
+ * pattern" and aborts skill load before its body runs (issue #730). Established
+ * defensive pattern: pair `2>/dev/null` with `|| true` or `|| echo '__SENTINEL__'`.
+ *
+ * `2>/dev/null` inside an earlier `;`-chained statement is fine if the trailing
+ * statement (e.g., a final `echo`) succeeds — that case is not flagged.
+ */
+function hasUnguardedErrorSuppression(cmd: string): boolean {
+  let depth = 0
+  let inSingleQuote = false
+  let inDoubleQuote = false
+  let lastSeparatorEnd = 0
+
+  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 === ';') { lastSeparatorEnd = i + 1; continue }
+      if (c === '&' && next === '&') { lastSeparatorEnd = i + 2; i++; continue }
+      if (c === '|' && next === '|') { lastSeparatorEnd = i + 2; i++; continue }
+    }
+  }
+
+  return cmd.slice(lastSeparatorEnd).includes("2>/dev/null")
+}
+
 /**
  * Returns true when the command contains a top-level pipe (`|` that is not
  * `||`). Claude Code's permission checker treats piped commands as separate
@@ -232,6 +276,48 @@ describe("hasNestedQuotedStringInCommandSubst", () => {
   })
 })
 
+describe("hasUnguardedErrorSuppression", () => {
+  test("flags single-statement `2>/dev/null` with no fallback", () => {
+    expect(hasUnguardedErrorSuppression("git rev-parse --abbrev-ref HEAD 2>/dev/null")).toBe(true)
+  })
+
+  test("flags `command -v <name> 2>/dev/null` with no fallback", () => {
+    expect(hasUnguardedErrorSuppression("command -v codex 2>/dev/null")).toBe(true)
+  })
+
+  test("does not flag `2>/dev/null || true`", () => {
+    expect(hasUnguardedErrorSuppression("git rev-parse --abbrev-ref HEAD 2>/dev/null || true")).toBe(false)
+  })
+
+  test("does not flag `2>/dev/null || echo '__SENTINEL__'`", () => {
+    expect(hasUnguardedErrorSuppression("git rev-parse --abbrev-ref origin/HEAD 2>/dev/null || echo '__DEFAULT_BRANCH_UNRESOLVED__'")).toBe(false)
+  })
+
+  test("does not flag commands without `2>/dev/null`", () => {
+    expect(hasUnguardedErrorSuppression("git status")).toBe(false)
+  })
+
+  test("does not flag `2>/dev/null` inside a guarded subshell", () => {
+    expect(hasUnguardedErrorSuppression("(top=$(git rev-parse --show-toplevel 2>/dev/null); cat \"$top/file\") || echo '__NO_CONFIG__'")).toBe(false)
+  })
+
+  test("does not flag `2>/dev/null` in non-trailing statement followed by always-succeeding tail", () => {
+    expect(hasUnguardedErrorSuppression('common=$(git rev-parse --git-common-dir 2>/dev/null); repo="${common%/.git}"; echo "${repo##*/}"')).toBe(false)
+  })
+
+  test("flags `2>/dev/null` on the trailing statement of a `;` chain", () => {
+    expect(hasUnguardedErrorSuppression("setup; cmd 2>/dev/null")).toBe(true)
+  })
+
+  test("flags `2>/dev/null` on the trailing statement of an `&&` chain", () => {
+    expect(hasUnguardedErrorSuppression("setup && cmd 2>/dev/null")).toBe(true)
+  })
+
+  test("flags `2>/dev/null` on the trailing statement of an `||` chain (a `||` belonging to an earlier statement does not protect a later unguarded one)", () => {
+    expect(hasUnguardedErrorSuppression("probe || git rev-parse --abbrev-ref HEAD 2>/dev/null")).toBe(true)
+  })
+})
+
 describe("hasTopLevelPipe", () => {
   test("flags a simple pipe", () => {
     expect(hasTopLevelPipe("git rev-parse --git-common-dir 2>/dev/null | sed -E 's|x||'")).toBe(true)
@@ -302,6 +388,19 @@ describe("skill `!` pre-resolution commands avoid Claude Code denylist", () => {
       ).toEqual([])
     })
 
+    test(`${rel} pre-resolution commands that suppress stderr with \`2>/dev/null\` also include a \`||\` fallback (issue #730)`, () => {
+      const offenders = preResolutionCommands.filter(({ command }) =>
+        hasUnguardedErrorSuppression(command),
+      )
+      const formatted = offenders
+        .map(({ lineNumber, command }) => `  line ${lineNumber}: ${command}`)
+        .join("\n")
+      expect(
+        offenders,
+        `\`2>/dev/null\` only suppresses stderr — the non-zero exit code still propagates. Outside the success path (e.g., \`git rev-parse\` outside a git repo), Claude Code reports "Shell command failed for pattern" and aborts skill load. Pair \`2>/dev/null\` with \`|| true\` (when an empty result is fine) or \`|| echo '__SENTINEL__'\` (when the agent should distinguish "did not resolve" from "resolved to empty").\nOffending commands:\n${formatted}`,
+      ).toEqual([])
+    })
+
     test(`${rel} pre-resolution commands do not use top-level pipes (triggers permission check for multiple operations)`, () => {
       const offenders = preResolutionCommands.filter(({ command }) =>
         hasTopLevelPipe(command),