Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mvanhorn wants to merge 2 commits into
base: main
Choose a base branch
from
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1af70e7
fix(skills): use command -v instead of which for codex detection
mvanhorn Apr 24, 2026
b6102d8
chore: regenerate SKILL.md files and refresh golden fixtures
mvanhorn Apr 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion codex/SKILL.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1092,7 +1092,7 @@ assumptions, catches things you might miss. Present its output faithfully, not s
## Step 0: Check codex binary

```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"
```

Expand Down
2 changes: 1 addition & 1 deletion codex/SKILL.md.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ assumptions, catches things you might miss. Present its output faithfully, not s
## Step 0: Check codex binary

```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"
```

Expand Down
2 changes: 1 addition & 1 deletion design-consultation/SKILL.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1345,7 +1345,7 @@ If user chooses B, skip this step and continue.

**Check Codex availability:**
```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:
Expand Down
2 changes: 1 addition & 1 deletion design-review/SKILL.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1965,7 +1965,7 @@ Record baseline design score and AI slop score at end of Phase 6.

**Check Codex availability:**
```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:
Expand Down
4 changes: 2 additions & 2 deletions office-hours/SKILL.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1470,7 +1470,7 @@ Use AskUserQuestion to confirm. If the user disagrees with a premise, revise und
**Binary check first:**

```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):
Expand Down Expand Up @@ -1740,7 +1740,7 @@ The screenshot file at `/tmp/gstack-sketch.png` can be referenced by downstream
After the wireframe is approved, offer outside design perspectives:

```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:
Expand Down
2 changes: 1 addition & 1 deletion plan-ceo-review/SKILL.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1865,7 +1865,7 @@ thorough review.
**Check tool availability:**

```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:
Expand Down
2 changes: 1 addition & 1 deletion plan-design-review/SKILL.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1519,7 +1519,7 @@ If user chooses B, skip this step and continue.

**Check Codex availability:**
```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:
Expand Down
2 changes: 1 addition & 1 deletion plan-devex-review/SKILL.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1861,7 +1861,7 @@ thorough review.
**Check tool availability:**

```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:
Expand Down
2 changes: 1 addition & 1 deletion plan-eng-review/SKILL.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1478,7 +1478,7 @@ thorough review.
**Check tool availability:**

```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:
Expand Down
2 changes: 1 addition & 1 deletion review/SKILL.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1818,7 +1818,7 @@ Every diff gets adversarial review from both Claude and Codex. LOC is not a prox
DIFF_INS=$(git diff origin/<base> --stat | tail -1 | grep -oE '[0-9]+ insertion' | grep -oE '[0-9]+' || echo "0")
DIFF_DEL=$(git diff origin/<base> --stat | tail -1 | grep -oE '[0-9]+ deletion' | grep -oE '[0-9]+' || echo "0")
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
OLD_CFG=$(~/.claude/skills/gstack/bin/gstack-config get codex_reviews 2>/dev/null || true)
echo "DIFF_SIZE: $DIFF_TOTAL"
Expand Down
6 changes: 3 additions & 3 deletions scripts/resolvers/design.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ export function generateDesignReviewLite(ctx: TemplateContext): string {
7. **Codex design voice** (optional, automatic if available):

\`\`\`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:
Expand Down Expand Up @@ -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:

\`\`\`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:
Expand Down Expand Up @@ -688,7 +688,7 @@ ${optInSection}

**Check Codex availability:**
\`\`\`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:
Expand Down
6 changes: 3 additions & 3 deletions scripts/resolvers/review.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ export function generateCodexSecondOpinion(ctx: TemplateContext): string {
**Binary check first:**

\`\`\`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):
Expand Down Expand Up @@ -425,7 +425,7 @@ Every diff gets adversarial review from both Claude and Codex. LOC is not a prox
DIFF_INS=$(git diff origin/<base> --stat | tail -1 | grep -oE '[0-9]+ insertion' | grep -oE '[0-9]+' || echo "0")
DIFF_DEL=$(git diff origin/<base> --stat | tail -1 | grep -oE '[0-9]+ deletion' | grep -oE '[0-9]+' || echo "0")
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
OLD_CFG=$(~/.claude/skills/gstack/bin/gstack-config get codex_reviews 2>/dev/null || true)
echo "DIFF_SIZE: $DIFF_TOTAL"
Expand Down Expand Up @@ -554,7 +554,7 @@ thorough review.
**Check tool availability:**

\`\`\`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:
Expand Down
4 changes: 2 additions & 2 deletions ship/SKILL.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2191,7 +2191,7 @@ Substitute: TIMESTAMP = ISO 8601 datetime, STATUS = "clean" if 0 findings or "is
7. **Codex design voice** (optional, automatic if available):

```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:
Expand Down Expand Up @@ -2544,7 +2544,7 @@ Every diff gets adversarial review from both Claude and Codex. LOC is not a prox
DIFF_INS=$(git diff origin/<base> --stat | tail -1 | grep -oE '[0-9]+ insertion' | grep -oE '[0-9]+' || echo "0")
DIFF_DEL=$(git diff origin/<base> --stat | tail -1 | grep -oE '[0-9]+ deletion' | grep -oE '[0-9]+' || echo "0")
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
OLD_CFG=$(~/.claude/skills/gstack/bin/gstack-config get codex_reviews 2>/dev/null || true)
echo "DIFF_SIZE: $DIFF_TOTAL"
Expand Down
4 changes: 2 additions & 2 deletions test/fixtures/golden/claude-ship-SKILL.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2191,7 +2191,7 @@ Substitute: TIMESTAMP = ISO 8601 datetime, STATUS = "clean" if 0 findings or "is
7. **Codex design voice** (optional, automatic if available):

```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:
Expand Down Expand Up @@ -2544,7 +2544,7 @@ Every diff gets adversarial review from both Claude and Codex. LOC is not a prox
DIFF_INS=$(git diff origin/<base> --stat | tail -1 | grep -oE '[0-9]+ insertion' | grep -oE '[0-9]+' || echo "0")
DIFF_DEL=$(git diff origin/<base> --stat | tail -1 | grep -oE '[0-9]+ deletion' | grep -oE '[0-9]+' || echo "0")
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
OLD_CFG=$(~/.claude/skills/gstack/bin/gstack-config get codex_reviews 2>/dev/null || true)
echo "DIFF_SIZE: $DIFF_TOTAL"
Expand Down
4 changes: 2 additions & 2 deletions test/fixtures/golden/factory-ship-SKILL.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2182,7 +2182,7 @@ Substitute: TIMESTAMP = ISO 8601 datetime, STATUS = "clean" if 0 findings or "is
7. **Codex design voice** (optional, automatic if available):

```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:
Expand Down Expand Up @@ -2535,7 +2535,7 @@ Every diff gets adversarial review from both Claude and Codex. LOC is not a prox
DIFF_INS=$(git diff origin/<base> --stat | tail -1 | grep -oE '[0-9]+ insertion' | grep -oE '[0-9]+' || echo "0")
DIFF_DEL=$(git diff origin/<base> --stat | tail -1 | grep -oE '[0-9]+ deletion' | grep -oE '[0-9]+' || echo "0")
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
OLD_CFG=$($GSTACK_ROOT/bin/gstack-config get codex_reviews 2>/dev/null || true)
echo "DIFF_SIZE: $DIFF_TOTAL"
Expand Down
2 changes: 1 addition & 1 deletion test/skill-e2e-plan.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@
recordE2E(evalCollector, '/plan-ceo-review-expansion-energy', 'Plan CEO Review Expansion Energy E2E', result, {
passed: ['success', 'error_max_turns'].includes(result.exitReason),
});
expect(['success', 'error_max_turns']).toContain(result.exitReason);

Check failure on line 243 in test/skill-e2e-plan.test.ts

View workflow job for this annotation

GitHub Actions / evals (e2e-plan, test/skill-e2e-plan.test.ts)

error: expect(received).toContain(expected) 

Expected to contain: "error_api"
Received: [ "success", "error_max_turns" ]

      at <anonymous> (/__w/gstack/gstack/test/skill-e2e-plan.test.ts:243:44)

Check failure on line 243 in test/skill-e2e-plan.test.ts

View workflow job for this annotation

GitHub Actions / evals (e2e-plan, test/skill-e2e-plan.test.ts)

error: expect(received).toContain(expected) 

Expected to contain: "error_api"
Received: [ "success", "error_max_turns" ]

      at <anonymous> (/__w/gstack/gstack/test/skill-e2e-plan.test.ts:243:44)

Check failure on line 243 in test/skill-e2e-plan.test.ts

View workflow job for this annotation

GitHub Actions / evals (e2e-plan, test/skill-e2e-plan.test.ts)

error: expect(received).toContain(expected) 

Expected to contain: "error_api"
Received: [ "success", "error_max_turns" ]

      at <anonymous> (/__w/gstack/gstack/test/skill-e2e-plan.test.ts:243:44)

const proposalsPath = path.join(planDir, 'proposals.md');
if (!fs.existsSync(proposalsPath)) {
Expand Down Expand Up @@ -539,7 +539,7 @@

logCost('/office-hours spec review', result);
recordE2E(evalCollector, '/office-hours-spec-review', 'Office Hours Spec Review E2E', result);
expect(result.exitReason).toBe('success');

Check failure on line 542 in test/skill-e2e-plan.test.ts

View workflow job for this annotation

GitHub Actions / evals (e2e-plan, test/skill-e2e-plan.test.ts)

error: expect(received).toBe(expected) 

Expected: "success"
Received: "error_api"

      at <anonymous> (/__w/gstack/gstack/test/skill-e2e-plan.test.ts:542:31)

Check failure on line 542 in test/skill-e2e-plan.test.ts

View workflow job for this annotation

GitHub Actions / evals (e2e-plan, test/skill-e2e-plan.test.ts)

error: expect(received).toBe(expected) 

Expected: "success"
Received: "error_api"

      at <anonymous> (/__w/gstack/gstack/test/skill-e2e-plan.test.ts:542:31)

Check failure on line 542 in test/skill-e2e-plan.test.ts

View workflow job for this annotation

GitHub Actions / evals (e2e-plan, test/skill-e2e-plan.test.ts)

error: expect(received).toBe(expected) 

Expected: "success"
Received: "error_api"

      at <anonymous> (/__w/gstack/gstack/test/skill-e2e-plan.test.ts:542:31)

const summaryPath = path.join(ohDir, 'spec-review-summary.md');
if (fs.existsSync(summaryPath)) {
Expand Down Expand Up @@ -598,7 +598,7 @@

logCost('/plan-ceo-review benefits-from', result);
recordE2E(evalCollector, '/plan-ceo-review-benefits', 'Plan CEO Review Benefits-From E2E', result);
expect(result.exitReason).toBe('success');

Check failure on line 601 in test/skill-e2e-plan.test.ts

View workflow job for this annotation

GitHub Actions / evals (e2e-plan, test/skill-e2e-plan.test.ts)

error: expect(received).toBe(expected) 

Expected: "success"
Received: "error_api"

      at <anonymous> (/__w/gstack/gstack/test/skill-e2e-plan.test.ts:601:31)

Check failure on line 601 in test/skill-e2e-plan.test.ts

View workflow job for this annotation

GitHub Actions / evals (e2e-plan, test/skill-e2e-plan.test.ts)

error: expect(received).toBe(expected) 

Expected: "success"
Received: "error_api"

      at <anonymous> (/__w/gstack/gstack/test/skill-e2e-plan.test.ts:601:31)

Check failure on line 601 in test/skill-e2e-plan.test.ts

View workflow job for this annotation

GitHub Actions / evals (e2e-plan, test/skill-e2e-plan.test.ts)

error: expect(received).toBe(expected) 

Expected: "success"
Received: "error_api"

      at <anonymous> (/__w/gstack/gstack/test/skill-e2e-plan.test.ts:601:31)

const summaryPath = path.join(benefitsDir, 'benefits-summary.md');
if (fs.existsSync(summaryPath)) {
Expand Down Expand Up @@ -686,7 +686,7 @@
recordE2E(evalCollector, '/plan-review-report', 'Plan Review Report E2E', result, {
passed: ['success', 'error_max_turns'].includes(result.exitReason),
});
expect(['success', 'error_max_turns']).toContain(result.exitReason);

Check failure on line 689 in test/skill-e2e-plan.test.ts

View workflow job for this annotation

GitHub Actions / evals (e2e-plan, test/skill-e2e-plan.test.ts)

error: expect(received).toContain(expected) 

Expected to contain: "error_api"
Received: [ "success", "error_max_turns" ]

      at <anonymous> (/__w/gstack/gstack/test/skill-e2e-plan.test.ts:689:44)

// Verify the review report was written to the plan file
const planContent = fs.readFileSync(path.join(planDir, 'plan.md'), 'utf-8');
Expand Down Expand Up @@ -776,7 +776,7 @@

const summary = fs.readFileSync(summaryPath, 'utf-8').toLowerCase();
// All skills should have codex availability check
expect(summary).toMatch(/which codex/);
expect(summary).toMatch(/command -v codex/);
// All skills should have fallback behavior
expect(summary).toMatch(/fallback|subagent|unavailable|not available|skip/);
// All skills should show it's optional/non-blocking
Expand Down
4 changes: 2 additions & 2 deletions test/skill-validation.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1315,9 +1315,9 @@ describe('Codex skill', () => {
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');
expect(content).toContain('which codex');
expect(content).toContain('command -v codex');
expect(content).not.toContain('/opt/homebrew/bin/codex');
});

Expand Down
Loading