RomanNum3ral
/
gstack
mirror of https://github.com/garrytan/gstack.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

fix(skills): use command -v instead of which for codex detection (#1197) 

`which` is not on PATH in every shell — some Windows shells, BusyBox-
only containers, and minimal CI images all fail when skills probe
codex availability via `which codex`. `command -v` is a POSIX builtin
and always available where the skill is running.

Touched:
- codex/SKILL.md.tmpl: CODEX_BIN=$(command -v codex || echo "")
- scripts/resolvers/review.ts and scripts/resolvers/design.ts:
  3 + 3 sites each rewritten to `command -v codex >/dev/null 2>&1`
- Regenerated all 10 affected SKILL.md files (codex, review, ship,
  design-consultation, design-review, office-hours, plan-ceo-review,
  plan-design-review, plan-devex-review, plan-eng-review)
- test/skill-validation.test.ts: updated pin + defensive regression
  test that fails if `which codex` returns to codex/SKILL.md
- test/skill-e2e-plan.test.ts: updated summary regex

Contributed by @mvanhorn via #1197.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Garry Tan 2026-05-18 21:16:32 -07:00
parent 95968b3eb4
commit 75872b9541
No known key found for this signature in database
GPG Key ID: C1F69E85C74EFE1D
15 changed files with 27 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
codex/SKILL.md
View File

@ -814,7 +814,7 @@ assumptions, catches things you might miss. Present its output faithfully, not s
## Step 0.4: Check codex binary ## Step 0.4: Check codex binary
```bash ```bash
CODEX_BIN=$(which codex 2>/dev/null || echo "") CODEX_BIN=$(command -v codex || echo "")
[ -z "$CODEX_BIN" ] && echo "NOT_FOUND" || echo "FOUND: $CODEX_BIN" [ -z "$CODEX_BIN" ] && echo "NOT_FOUND" || echo "FOUND: $CODEX_BIN"
``` ```

2
codex/SKILL.md.tmpl
View File

@ -42,7 +42,7 @@ assumptions, catches things you might miss. Present its output faithfully, not s
## Step 0.4: Check codex binary ## Step 0.4: Check codex binary
```bash ```bash
CODEX_BIN=$(which codex 2>/dev/null || echo "") CODEX_BIN=$(command -v codex || echo "")
[ -z "$CODEX_BIN" ] && echo "NOT_FOUND" || echo "FOUND: $CODEX_BIN" [ -z "$CODEX_BIN" ] && echo "NOT_FOUND" || echo "FOUND: $CODEX_BIN"
``` ```

2
design-consultation/SKILL.md
View File

@ -1090,7 +1090,7 @@ If user chooses B, skip this step and continue.
**Check Codex availability:** **Check Codex availability:**
```bash ```bash
which codex 2>/dev/null && echo "CODEX_AVAILABLE" || echo "CODEX_NOT_AVAILABLE" command -v codex >/dev/null 2>&1 && echo "CODEX_AVAILABLE" || echo "CODEX_NOT_AVAILABLE"
``` ```
**If Codex is available**, launch both voices simultaneously: **If Codex is available**, launch both voices simultaneously:

2
design-review/SKILL.md
View File

@ -1687,7 +1687,7 @@ Record baseline design score and AI slop score at end of Phase 6.
**Check Codex availability:** **Check Codex availability:**
```bash ```bash
which codex 2>/dev/null && echo "CODEX_AVAILABLE" || echo "CODEX_NOT_AVAILABLE" command -v codex >/dev/null 2>&1 && echo "CODEX_AVAILABLE" || echo "CODEX_NOT_AVAILABLE"
``` ```
**If Codex is available**, launch both voices simultaneously: **If Codex is available**, launch both voices simultaneously:

4
office-hours/SKILL.md
View File

@ -1219,7 +1219,7 @@ Use AskUserQuestion to confirm. If the user disagrees with a premise, revise und
**Binary check first:** **Binary check first:**
```bash ```bash
which codex 2>/dev/null && echo "CODEX_AVAILABLE" || echo "CODEX_NOT_AVAILABLE" command -v codex >/dev/null 2>&1 && echo "CODEX_AVAILABLE" || echo "CODEX_NOT_AVAILABLE"
``` ```
Use AskUserQuestion (regardless of codex availability): Use AskUserQuestion (regardless of codex availability):
@ -1491,7 +1491,7 @@ The screenshot file at `/tmp/gstack-sketch.png` can be referenced by downstream
After the wireframe is approved, offer outside design perspectives: After the wireframe is approved, offer outside design perspectives:
```bash ```bash
which codex 2>/dev/null && echo "CODEX_AVAILABLE" || echo "CODEX_NOT_AVAILABLE" command -v codex >/dev/null 2>&1 && echo "CODEX_AVAILABLE" || echo "CODEX_NOT_AVAILABLE"
``` ```
If Codex is available, use AskUserQuestion: If Codex is available, use AskUserQuestion:

2
plan-ceo-review/SKILL.md
View File

@ -1613,7 +1613,7 @@ thorough review.
**Check tool availability:** **Check tool availability:**
```bash ```bash
which codex 2>/dev/null && echo "CODEX_AVAILABLE" || echo "CODEX_NOT_AVAILABLE" command -v codex >/dev/null 2>&1 && echo "CODEX_AVAILABLE" || echo "CODEX_NOT_AVAILABLE"
``` ```
Use AskUserQuestion: Use AskUserQuestion:

2
plan-design-review/SKILL.md
View File

@ -1241,7 +1241,7 @@ If user chooses B, skip this step and continue.
**Check Codex availability:** **Check Codex availability:**
```bash ```bash
which codex 2>/dev/null && echo "CODEX_AVAILABLE" || echo "CODEX_NOT_AVAILABLE" command -v codex >/dev/null 2>&1 && echo "CODEX_AVAILABLE" || echo "CODEX_NOT_AVAILABLE"
``` ```
**If Codex is available**, launch both voices simultaneously: **If Codex is available**, launch both voices simultaneously:

2
plan-devex-review/SKILL.md
View File

@ -1585,7 +1585,7 @@ thorough review.
**Check tool availability:** **Check tool availability:**
```bash ```bash
which codex 2>/dev/null && echo "CODEX_AVAILABLE" || echo "CODEX_NOT_AVAILABLE" command -v codex >/dev/null 2>&1 && echo "CODEX_AVAILABLE" || echo "CODEX_NOT_AVAILABLE"
``` ```
Use AskUserQuestion: Use AskUserQuestion:

2
plan-eng-review/SKILL.md
View File

@ -1214,7 +1214,7 @@ thorough review.
**Check tool availability:** **Check tool availability:**
```bash ```bash
which codex 2>/dev/null && echo "CODEX_AVAILABLE" || echo "CODEX_NOT_AVAILABLE" command -v codex >/dev/null 2>&1 && echo "CODEX_AVAILABLE" || echo "CODEX_NOT_AVAILABLE"
``` ```
Use AskUserQuestion: Use AskUserQuestion:

2
review/SKILL.md
View File

@ -1578,7 +1578,7 @@ DIFF_BASE=$(git merge-base origin/<base> HEAD)
DIFF_INS=$(git diff "$DIFF_BASE" --stat | tail -1 | grep -oE '[0-9]+ insertion' | grep -oE '[0-9]+' || echo "0") DIFF_INS=$(git diff "$DIFF_BASE" --stat | tail -1 | grep -oE '[0-9]+ insertion' | grep -oE '[0-9]+' || echo "0")
DIFF_DEL=$(git diff "$DIFF_BASE" --stat | tail -1 | grep -oE '[0-9]+ deletion' | grep -oE '[0-9]+' || echo "0") DIFF_DEL=$(git diff "$DIFF_BASE" --stat | tail -1 | grep -oE '[0-9]+ deletion' | grep -oE '[0-9]+' || echo "0")
DIFF_TOTAL=$((DIFF_INS + DIFF_DEL)) DIFF_TOTAL=$((DIFF_INS + DIFF_DEL))
which codex 2>/dev/null && echo "CODEX_AVAILABLE" || echo "CODEX_NOT_AVAILABLE" command -v codex >/dev/null 2>&1 && echo "CODEX_AVAILABLE" || echo "CODEX_NOT_AVAILABLE"
# Legacy opt-out — only gates Codex passes, Claude always runs # Legacy opt-out — only gates Codex passes, Claude always runs
OLD_CFG=$(~/.claude/skills/gstack/bin/gstack-config get codex_reviews 2>/dev/null || true) OLD_CFG=$(~/.claude/skills/gstack/bin/gstack-config get codex_reviews 2>/dev/null || true)
echo "DIFF_SIZE: $DIFF_TOTAL" echo "DIFF_SIZE: $DIFF_TOTAL"

6
scripts/resolvers/design.ts
View File

@ -10,7 +10,7 @@ export function generateDesignReviewLite(ctx: TemplateContext): string {
7. **Codex design voice** (optional, automatic if available): 7. **Codex design voice** (optional, automatic if available):
\`\`\`bash \`\`\`bash
which codex 2>/dev/null && echo "CODEX_AVAILABLE" || echo "CODEX_NOT_AVAILABLE" command -v codex >/dev/null 2>&1 && echo "CODEX_AVAILABLE" || echo "CODEX_NOT_AVAILABLE"
\`\`\` \`\`\`
If Codex is available, run a lightweight design check on the diff: If Codex is available, run a lightweight design check on the diff:
@ -512,7 +512,7 @@ The screenshot file at \`/tmp/gstack-sketch.png\` can be referenced by downstrea
After the wireframe is approved, offer outside design perspectives: After the wireframe is approved, offer outside design perspectives:
\`\`\`bash \`\`\`bash
which codex 2>/dev/null && echo "CODEX_AVAILABLE" || echo "CODEX_NOT_AVAILABLE" command -v codex >/dev/null 2>&1 && echo "CODEX_AVAILABLE" || echo "CODEX_NOT_AVAILABLE"
\`\`\` \`\`\`
If Codex is available, use AskUserQuestion: If Codex is available, use AskUserQuestion:
@ -688,7 +688,7 @@ ${optInSection}
**Check Codex availability:** **Check Codex availability:**
\`\`\`bash \`\`\`bash
which codex 2>/dev/null && echo "CODEX_AVAILABLE" || echo "CODEX_NOT_AVAILABLE" command -v codex >/dev/null 2>&1 && echo "CODEX_AVAILABLE" || echo "CODEX_NOT_AVAILABLE"
\`\`\` \`\`\`
**If Codex is available**, launch both voices simultaneously: **If Codex is available**, launch both voices simultaneously:

6
scripts/resolvers/review.ts
View File

@ -311,7 +311,7 @@ export function generateCodexSecondOpinion(ctx: TemplateContext): string {
**Binary check first:** **Binary check first:**
\`\`\`bash \`\`\`bash
which codex 2>/dev/null && echo "CODEX_AVAILABLE" || echo "CODEX_NOT_AVAILABLE" command -v codex >/dev/null 2>&1 && echo "CODEX_AVAILABLE" || echo "CODEX_NOT_AVAILABLE"
\`\`\` \`\`\`
Use AskUserQuestion (regardless of codex availability): Use AskUserQuestion (regardless of codex availability):
@ -471,7 +471,7 @@ DIFF_BASE=$(git merge-base origin/<base> HEAD)
DIFF_INS=$(git diff "$DIFF_BASE" --stat | tail -1 | grep -oE '[0-9]+ insertion' | grep -oE '[0-9]+' || echo "0") DIFF_INS=$(git diff "$DIFF_BASE" --stat | tail -1 | grep -oE '[0-9]+ insertion' | grep -oE '[0-9]+' || echo "0")
DIFF_DEL=$(git diff "$DIFF_BASE" --stat | tail -1 | grep -oE '[0-9]+ deletion' | grep -oE '[0-9]+' || echo "0") DIFF_DEL=$(git diff "$DIFF_BASE" --stat | tail -1 | grep -oE '[0-9]+ deletion' | grep -oE '[0-9]+' || echo "0")
DIFF_TOTAL=$((DIFF_INS + DIFF_DEL)) DIFF_TOTAL=$((DIFF_INS + DIFF_DEL))
which codex 2>/dev/null && echo "CODEX_AVAILABLE" || echo "CODEX_NOT_AVAILABLE" command -v codex >/dev/null 2>&1 && echo "CODEX_AVAILABLE" || echo "CODEX_NOT_AVAILABLE"
# Legacy opt-out — only gates Codex passes, Claude always runs # Legacy opt-out — only gates Codex passes, Claude always runs
OLD_CFG=$(~/.claude/skills/gstack/bin/gstack-config get codex_reviews 2>/dev/null || true) OLD_CFG=$(~/.claude/skills/gstack/bin/gstack-config get codex_reviews 2>/dev/null || true)
echo "DIFF_SIZE: $DIFF_TOTAL" echo "DIFF_SIZE: $DIFF_TOTAL"
@ -600,7 +600,7 @@ thorough review.
**Check tool availability:** **Check tool availability:**
\`\`\`bash \`\`\`bash
which codex 2>/dev/null && echo "CODEX_AVAILABLE" || echo "CODEX_NOT_AVAILABLE" command -v codex >/dev/null 2>&1 && echo "CODEX_AVAILABLE" || echo "CODEX_NOT_AVAILABLE"
\`\`\` \`\`\`
Use AskUserQuestion: Use AskUserQuestion:

4
ship/SKILL.md
View File

@ -1962,7 +1962,7 @@ Substitute: TIMESTAMP = ISO 8601 datetime, STATUS = "clean" if 0 findings or "is
7. **Codex design voice** (optional, automatic if available): 7. **Codex design voice** (optional, automatic if available):
```bash ```bash
which codex 2>/dev/null && echo "CODEX_AVAILABLE" || echo "CODEX_NOT_AVAILABLE" command -v codex >/dev/null 2>&1 && echo "CODEX_AVAILABLE" || echo "CODEX_NOT_AVAILABLE"
``` ```
If Codex is available, run a lightweight design check on the diff: If Codex is available, run a lightweight design check on the diff:
@ -2317,7 +2317,7 @@ DIFF_BASE=$(git merge-base origin/<base> HEAD)
DIFF_INS=$(git diff "$DIFF_BASE" --stat | tail -1 | grep -oE '[0-9]+ insertion' | grep -oE '[0-9]+' || echo "0") DIFF_INS=$(git diff "$DIFF_BASE" --stat | tail -1 | grep -oE '[0-9]+ insertion' | grep -oE '[0-9]+' || echo "0")
DIFF_DEL=$(git diff "$DIFF_BASE" --stat | tail -1 | grep -oE '[0-9]+ deletion' | grep -oE '[0-9]+' || echo "0") DIFF_DEL=$(git diff "$DIFF_BASE" --stat | tail -1 | grep -oE '[0-9]+ deletion' | grep -oE '[0-9]+' || echo "0")
DIFF_TOTAL=$((DIFF_INS + DIFF_DEL)) DIFF_TOTAL=$((DIFF_INS + DIFF_DEL))
which codex 2>/dev/null && echo "CODEX_AVAILABLE" || echo "CODEX_NOT_AVAILABLE" command -v codex >/dev/null 2>&1 && echo "CODEX_AVAILABLE" || echo "CODEX_NOT_AVAILABLE"
# Legacy opt-out — only gates Codex passes, Claude always runs # Legacy opt-out — only gates Codex passes, Claude always runs
OLD_CFG=$(~/.claude/skills/gstack/bin/gstack-config get codex_reviews 2>/dev/null || true) OLD_CFG=$(~/.claude/skills/gstack/bin/gstack-config get codex_reviews 2>/dev/null || true)
echo "DIFF_SIZE: $DIFF_TOTAL" echo "DIFF_SIZE: $DIFF_TOTAL"

4
test/skill-e2e-plan.test.ts
View File

@ -775,8 +775,8 @@ Write your summary to ${testDir}/${testName}-summary.md`,
expect(fs.existsSync(summaryPath)).toBe(true); expect(fs.existsSync(summaryPath)).toBe(true);
const summary = fs.readFileSync(summaryPath, 'utf-8').toLowerCase(); const summary = fs.readFileSync(summaryPath, 'utf-8').toLowerCase();
// All skills should have codex availability check // All skills should have codex availability check (command -v per #1197)
expect(summary).toMatch(/which codex/); expect(summary).toMatch(/command -v codex/);
// All skills should have fallback behavior // All skills should have fallback behavior
expect(summary).toMatch(/fallback|subagent|unavailable|not available|skip/); expect(summary).toMatch(/fallback|subagent|unavailable|not available|skip/);
// All skills should show it's optional/non-blocking // All skills should show it's optional/non-blocking

8
test/skill-validation.test.ts
View File

@ -1325,10 +1325,14 @@ describe('Codex skill', () => {
expect(content).toContain('gstack-review-log'); expect(content).toContain('gstack-review-log');
}); });
test('codex/SKILL.md uses which for binary discovery, not hardcoded path', () => { test('codex/SKILL.md uses command -v for binary discovery, not hardcoded path', () => {
const content = fs.readFileSync(path.join(ROOT, 'codex', 'SKILL.md'), 'utf-8'); const content = fs.readFileSync(path.join(ROOT, 'codex', 'SKILL.md'), 'utf-8');
expect(content).toContain('which codex'); expect(content).toContain('command -v codex');
expect(content).not.toContain('/opt/homebrew/bin/codex'); expect(content).not.toContain('/opt/homebrew/bin/codex');
// Defensive: catch any future regression that reintroduces `which codex`,
// which fails in environments where `which` isn't on PATH (some Windows
// shells, BusyBox-only containers). #1197.
expect(content).not.toContain('which codex');
}); });
test('codex/SKILL.md contains error handling for missing binary and auth', () => { test('codex/SKILL.md contains error handling for missing binary and auth', () => {