fix(ce-compound,ce-sessions): handle non-git CWD in pre-resolved git branch#731
Merged
fix(ce-compound,ce-sessions): handle non-git CWD in pre-resolved git branch#731
Conversation
…branch
`git rev-parse --abbrev-ref HEAD 2>/dev/null` exits 128 outside a git repo;
`2>/dev/null` suppresses stderr but the non-zero exit propagates and Claude
Code reports "Shell command failed for pattern", aborting skill load.
Pair the suppression with `|| true` so the line resolves to an empty value
that the existing SKILL.md prose already handles ("if they... are empty,
omit them from the dispatch and let the agent derive them at runtime").
Also fixes the same bug class in ce-work-beta's codex availability probe
(`command -v codex 2>/dev/null` exits 1 on machines without codex).
Adds a `hasUnguardedErrorSuppression` check to `tests/skill-shell-safety.test.ts`
that flags any pre-resolution command whose trailing top-level statement uses
`2>/dev/null` without an `||` fallback. The check ignores non-trailing
suppressions where a later always-succeeding statement (e.g., a final `echo`)
ensures exit 0.
Distinct from #722 / PR #723, which fixed a different line on the same files
(the "Repo name" pipe triggering "multiple operations requiring approval").
Fixes #730
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b9dcff129a
ℹ️ 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".
- Treat top-level `||` as a separator in `hasUnguardedErrorSuppression` so forms like `probe || git rev-parse 2>/dev/null` are flagged. The earlier `||` belongs to the previous statement and does not protect the trailing unguarded `2>/dev/null`. Added regression test for the pattern. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
/ce-compoundand/ce-sessionsfailed to load outside a git repo because!git rev-parse --abbrev-ref HEAD 2>/dev/null`` exits 128 with no fallback.2>/dev/nullonly swallows stderr — the non-zero exit propagates and Claude Code reports "Shell command failed for pattern". Pair with `|| true` so the line resolves to an empty value (the existing prose already handles "are empty, omit them from the dispatch").ce-work-beta(command -v codex 2>/dev/nullexits 1 on machines without codex).hasUnguardedErrorSuppressioncheck totests/skill-shell-safety.test.tsthat flags any pre-resolution command whose trailing top-level statement uses2>/dev/nullwithout an||fallback. The check ignores non-trailing suppressions where a later always-succeeding statement (e.g., a finalecho) ensures exit 0, so it catches real#730-class bugs without false-positiving on theRepo nameline.Test plan
bun test tests/skill-shell-safety.test.ts— 81 pass, the new per-skill check fails on the unfixed lines and passes after the fixbun test— full suite (1045 tests) passesbun run release:validate— cleancd /tmp && bash -c 'git rev-parse --abbrev-ref HEAD 2>/dev/null || true'; echo \$?→ exit 0/ce-compoundand/ce-sessionsload without "Shell command failed for pattern"Fixes #730
🤖 Generated with Claude Code