diff --git a/CHANGELOG.md b/CHANGELOG.md index 8ebcb3d6..96e7c1ff 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,22 @@ # Changelog +## [0.18.4.0] - 2026-04-18 + +### Fixed +- **Apple Silicon no longer dies with SIGKILL on first run.** `./setup` now ad-hoc codesigns every compiled binary after `bun run build` so M-series Macs can actually execute them. If you cloned gstack and saw `zsh: killed ./browse/dist/browse` before getting to Day 2, this is why. Thanks to @voidborne-d (#1003) for tracking down the Bun `--compile` linker signature issue and shipping a tested fix (6 tests across 4 binaries, idempotent, platform-guarded). +- **`/codex` no longer hangs forever in Claude Code's Bash tool.** Codex CLI 0.120.0 introduced a stdin deadlock: if stdin is a non-TTY pipe (Claude Code, CI, background bash, OpenClaw), `codex exec` waits for EOF to append it as a `` block, even when the prompt is passed as a positional argument. Symptom: "Reading additional input from stdin...", 0% CPU, no output. Every `codex exec` and `codex review` now redirects stdin from `/dev/null`. `/autoplan`, every plan-review outside voice, `/ship` adversarial, and `/review` adversarial all unblock. Thanks to @loning (#972) for the 13-minute repro and minimal fix. +- **`/codex` and `/autoplan` fail fast when Codex auth is missing or broken.** Before this release, a logged-out Codex user would watch the skill spend minutes building an expensive prompt only to surface the auth error mid-stream. Now both skills preflight auth via a multi-signal probe (`$CODEX_API_KEY`, `$OPENAI_API_KEY`, or `${CODEX_HOME:-~/.codex}/auth.json`) and stop with a clear "run `codex login` or set `$CODEX_API_KEY`" message before any prompt construction. Bonus: if your Codex CLI is on a known-buggy version (currently 0.120.0-0.120.2), you'll get a one-line nudge to upgrade. +- **`/codex` and `/autoplan` no longer sit at 0% CPU forever if the model API stalls.** Every `codex exec` / `codex review` now runs under a 10-minute timeout wrapper with a `gtimeout → timeout → unwrapped` fallback chain, so you get a clear "Codex stalled past 10 minutes. Common causes: model API stall, long prompt, network issue. Try re-running." message instead of an infinite wait. `./setup` auto-installs `coreutils` on macOS so `gtimeout` is available (skip with `GSTACK_SKIP_COREUTILS=1` for CI / locked machines). +- **`/codex` Challenge mode now surfaces auth errors instead of silently dropping them.** Challenge mode was piping stderr to `/dev/null`, which masked any auth failures in the middle of a run. Now it captures stderr to a temp file and checks for `auth|login|unauthorized` patterns. If Codex errors mid-run, you see it. +- **Plan reviews no longer quietly bias toward minimal-diff recommendations.** `/plan-ceo-review` and `/plan-eng-review` used to list "minimal diff" as an engineering preference without a counterbalancing "rewrite is fine when warranted" note. Reviewers picked up on that and rejected rewrites that should've been approved. The preference is now framed as "right-sized diff" with explicit permission to recommend a rewrite when the existing foundation is broken. Implementation alternatives in CEO review also got an equal-weight clarification: don't default to minimal viable just because it's smaller. + +### For contributors +- New `bin/gstack-codex-probe` consolidates the auth probe, version check, timeout wrapper, and telemetry logger into one bash helper that `/codex` and `/autoplan` both source. When a second outside-voice backend lands (Gemini CLI), this is the file to extend. +- New `test/codex-hardening.test.ts` ships 25 deterministic unit tests for the probe (8 auth probe combinations, 10 version regex cases including `0.120.10` false-positive guards, 4 timeout wrapper + namespace hygiene checks, 3 telemetry payload schema checks confirming no env values leak into events). Free tier, <5s runtime. +- New `test/skill-e2e-autoplan-dual-voice.test.ts` (periodic tier) gates the `/autoplan` dual-voice path. Asserts both Claude subagent and Codex voices produce output in Phase 1, OR that `[codex-unavailable]` is logged when Codex is absent. Periodic ~= $1/run, not a gate. +- Codex failure telemetry events (`codex_timeout`, `codex_auth_failed`, `codex_cli_missing`, `codex_version_warning`) now land in `~/.gstack/analytics/skill-usage.jsonl` behind the existing user opt-in. Reliability regressions are visible at the user-base scale. +- Codex timeouts (`exit 124`) now auto-log operational learnings via `gstack-learnings-log`. Future `/investigate` sessions on the same skill/branch surface prior hang patterns automatically. + ## [0.18.3.0] - 2026-04-17 ### Added diff --git a/VERSION b/VERSION index c9b0a514..aab9d975 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.18.3.0 +0.18.4.0 diff --git a/autoplan/SKILL.md b/autoplan/SKILL.md index 224a80ec..9c61c11f 100644 --- a/autoplan/SKILL.md +++ b/autoplan/SKILL.md @@ -871,6 +871,39 @@ Loaded review skills from disk. Starting full review pipeline with auto-decision --- +## Phase 0.5: Codex auth + version preflight + +Before invoking any Codex voice, preflight the CLI: verify auth (multi-signal) and +warn on known-bad CLI versions. This is infrastructure for all 4 phases below — +source it once here and the helper functions stay in scope for the rest of the +workflow. + +```bash +_TEL=$(~/.claude/skills/gstack/bin/gstack-config get telemetry 2>/dev/null || echo off) +source ~/.claude/skills/gstack/bin/gstack-codex-probe + +# Check Codex binary. If missing, tag the degradation matrix and continue +# with Claude subagent only (autoplan's existing degradation fallback). +if ! command -v codex >/dev/null 2>&1; then + _gstack_codex_log_event "codex_cli_missing" + echo "[codex-unavailable: binary not found] — proceeding with Claude subagent only" + _CODEX_AVAILABLE=false +elif ! _gstack_codex_auth_probe >/dev/null; then + _gstack_codex_log_event "codex_auth_failed" + echo "[codex-unavailable: auth missing] — proceeding with Claude subagent only. Run \`codex login\` or set \$CODEX_API_KEY to enable dual-voice review." + _CODEX_AVAILABLE=false +else + _gstack_codex_version_check # non-blocking warn if known-bad + _CODEX_AVAILABLE=true +fi +``` + +If `_CODEX_AVAILABLE=false`, all Phase 1-3.5 Codex voices below degrade to +`[codex-unavailable]` in the degradation matrix. /autoplan completes with +Claude subagent only — saves token spend on Codex prompts we can't use. + +--- + ## Phase 1: CEO Review (Strategy & Scope) Follow plan-ceo-review/SKILL.md — all sections, full depth. @@ -894,7 +927,7 @@ Override: every AskUserQuestion → auto-decide using the 6 principles. **Codex CEO voice** (via Bash): ```bash _REPO_ROOT=$(git rev-parse --show-toplevel) || { echo "ERROR: not in a git repo" >&2; exit 1; } - codex exec "IMPORTANT: Do NOT read or execute any SKILL.md files or files in skill definition directories (paths containing skills/gstack). These are AI assistant skill definitions meant for a different system. Stay focused on repository code only. + _gstack_codex_timeout_wrapper 600 codex exec "IMPORTANT: Do NOT read or execute any SKILL.md files or files in skill definition directories (paths containing skills/gstack). These are AI assistant skill definitions meant for a different system. Stay focused on repository code only. You are a CEO/founder advisor reviewing a development plan. Challenge the strategic foundations: Are the premises valid or assumed? Is this the @@ -902,9 +935,15 @@ Override: every AskUserQuestion → auto-decide using the 6 principles. What alternatives were dismissed too quickly? What competitive or market risks are unaddressed? What scope decisions will look foolish in 6 months? Be adversarial. No compliments. Just the strategic blind spots. - File: " -C "$_REPO_ROOT" -s read-only --enable web_search_cached + File: " -C "$_REPO_ROOT" -s read-only --enable web_search_cached < /dev/null + _CODEX_EXIT=$? + if [ "$_CODEX_EXIT" = "124" ]; then + _gstack_codex_log_event "codex_timeout" "600" + _gstack_codex_log_hang "autoplan" "0" + echo "[codex stalled past 10 minutes — tagging as [codex-unavailable] for this phase and proceeding with Claude subagent only]" + fi ``` - Timeout: 10 minutes + Timeout: 10 minutes (shell-wrapper) + 12 minutes (Bash outer gate). On hang, auto-degrades this phase's Codex voice. **Claude CEO subagent** (via Agent tool): "Read the plan file at . You are an independent CEO/strategist @@ -1005,7 +1044,7 @@ Override: every AskUserQuestion → auto-decide using the 6 principles. **Codex design voice** (via Bash): ```bash _REPO_ROOT=$(git rev-parse --show-toplevel) || { echo "ERROR: not in a git repo" >&2; exit 1; } - codex exec "IMPORTANT: Do NOT read or execute any SKILL.md files or files in skill definition directories (paths containing skills/gstack). These are AI assistant skill definitions meant for a different system. Stay focused on repository code only. + _gstack_codex_timeout_wrapper 600 codex exec "IMPORTANT: Do NOT read or execute any SKILL.md files or files in skill definition directories (paths containing skills/gstack). These are AI assistant skill definitions meant for a different system. Stay focused on repository code only. Read the plan file at . Evaluate this plan's UI/UX design decisions. @@ -1019,9 +1058,15 @@ Override: every AskUserQuestion → auto-decide using the 6 principles. accessibility requirements (keyboard nav, contrast, touch targets) specified or aspirational? Does the plan describe specific UI decisions or generic patterns? What design decisions will haunt the implementer if left ambiguous? - Be opinionated. No hedging." -C "$_REPO_ROOT" -s read-only --enable web_search_cached + Be opinionated. No hedging." -C "$_REPO_ROOT" -s read-only --enable web_search_cached < /dev/null + _CODEX_EXIT=$? + if [ "$_CODEX_EXIT" = "124" ]; then + _gstack_codex_log_event "codex_timeout" "600" + _gstack_codex_log_hang "autoplan" "0" + echo "[codex stalled past 10 minutes — tagging as [codex-unavailable] for this phase and proceeding with Claude subagent only]" + fi ``` - Timeout: 10 minutes + Timeout: 10 minutes (shell-wrapper) + 12 minutes (Bash outer gate). On hang, auto-degrades this phase's Codex voice. **Claude design subagent** (via Agent tool): "Read the plan file at . You are an independent senior product designer @@ -1080,7 +1125,7 @@ Override: every AskUserQuestion → auto-decide using the 6 principles. **Codex eng voice** (via Bash): ```bash _REPO_ROOT=$(git rev-parse --show-toplevel) || { echo "ERROR: not in a git repo" >&2; exit 1; } - codex exec "IMPORTANT: Do NOT read or execute any SKILL.md files or files in skill definition directories (paths containing skills/gstack). These are AI assistant skill definitions meant for a different system. Stay focused on repository code only. + _gstack_codex_timeout_wrapper 600 codex exec "IMPORTANT: Do NOT read or execute any SKILL.md files or files in skill definition directories (paths containing skills/gstack). These are AI assistant skill definitions meant for a different system. Stay focused on repository code only. Review this plan for architectural issues, missing edge cases, and hidden complexity. Be adversarial. @@ -1089,9 +1134,15 @@ Override: every AskUserQuestion → auto-decide using the 6 principles. CEO: Design: - File: " -C "$_REPO_ROOT" -s read-only --enable web_search_cached + File: " -C "$_REPO_ROOT" -s read-only --enable web_search_cached < /dev/null + _CODEX_EXIT=$? + if [ "$_CODEX_EXIT" = "124" ]; then + _gstack_codex_log_event "codex_timeout" "600" + _gstack_codex_log_hang "autoplan" "0" + echo "[codex stalled past 10 minutes — tagging as [codex-unavailable] for this phase and proceeding with Claude subagent only]" + fi ``` - Timeout: 10 minutes + Timeout: 10 minutes (shell-wrapper) + 12 minutes (Bash outer gate). On hang, auto-degrades this phase's Codex voice. **Claude eng subagent** (via Agent tool): "Read the plan file at . You are an independent senior engineer @@ -1195,7 +1246,7 @@ Log: "Phase 3.5 skipped — no developer-facing scope detected." **Codex DX voice** (via Bash): ```bash _REPO_ROOT=$(git rev-parse --show-toplevel) || { echo "ERROR: not in a git repo" >&2; exit 1; } - codex exec "IMPORTANT: Do NOT read or execute any SKILL.md files or files in skill definition directories (paths containing skills/gstack). These are AI assistant skill definitions meant for a different system. Stay focused on repository code only. + _gstack_codex_timeout_wrapper 600 codex exec "IMPORTANT: Do NOT read or execute any SKILL.md files or files in skill definition directories (paths containing skills/gstack). These are AI assistant skill definitions meant for a different system. Stay focused on repository code only. Read the plan file at . Evaluate this plan's developer experience. @@ -1209,9 +1260,15 @@ Log: "Phase 3.5 skipped — no developer-facing scope detected." 3. API/CLI design: are names guessable? Are defaults sensible? Is it consistent? 4. Docs: can a dev find what they need in under 2 minutes? Are examples copy-paste-complete? 5. Upgrade path: can devs upgrade without fear? Migration guides? Deprecation warnings? - Be adversarial. Think like a developer who is evaluating this against 3 competitors." -C "$_REPO_ROOT" -s read-only --enable web_search_cached + Be adversarial. Think like a developer who is evaluating this against 3 competitors." -C "$_REPO_ROOT" -s read-only --enable web_search_cached < /dev/null + _CODEX_EXIT=$? + if [ "$_CODEX_EXIT" = "124" ]; then + _gstack_codex_log_event "codex_timeout" "600" + _gstack_codex_log_hang "autoplan" "0" + echo "[codex stalled past 10 minutes — tagging as [codex-unavailable] for this phase and proceeding with Claude subagent only]" + fi ``` - Timeout: 10 minutes + Timeout: 10 minutes (shell-wrapper) + 12 minutes (Bash outer gate). On hang, auto-degrades this phase's Codex voice. **Claude DX subagent** (via Agent tool): "Read the plan file at . You are an independent DX engineer diff --git a/autoplan/SKILL.md.tmpl b/autoplan/SKILL.md.tmpl index ae3383ef..6577a672 100644 --- a/autoplan/SKILL.md.tmpl +++ b/autoplan/SKILL.md.tmpl @@ -234,6 +234,39 @@ Loaded review skills from disk. Starting full review pipeline with auto-decision --- +## Phase 0.5: Codex auth + version preflight + +Before invoking any Codex voice, preflight the CLI: verify auth (multi-signal) and +warn on known-bad CLI versions. This is infrastructure for all 4 phases below — +source it once here and the helper functions stay in scope for the rest of the +workflow. + +```bash +_TEL=$(~/.claude/skills/gstack/bin/gstack-config get telemetry 2>/dev/null || echo off) +source ~/.claude/skills/gstack/bin/gstack-codex-probe + +# Check Codex binary. If missing, tag the degradation matrix and continue +# with Claude subagent only (autoplan's existing degradation fallback). +if ! command -v codex >/dev/null 2>&1; then + _gstack_codex_log_event "codex_cli_missing" + echo "[codex-unavailable: binary not found] — proceeding with Claude subagent only" + _CODEX_AVAILABLE=false +elif ! _gstack_codex_auth_probe >/dev/null; then + _gstack_codex_log_event "codex_auth_failed" + echo "[codex-unavailable: auth missing] — proceeding with Claude subagent only. Run \`codex login\` or set \$CODEX_API_KEY to enable dual-voice review." + _CODEX_AVAILABLE=false +else + _gstack_codex_version_check # non-blocking warn if known-bad + _CODEX_AVAILABLE=true +fi +``` + +If `_CODEX_AVAILABLE=false`, all Phase 1-3.5 Codex voices below degrade to +`[codex-unavailable]` in the degradation matrix. /autoplan completes with +Claude subagent only — saves token spend on Codex prompts we can't use. + +--- + ## Phase 1: CEO Review (Strategy & Scope) Follow plan-ceo-review/SKILL.md — all sections, full depth. @@ -257,7 +290,7 @@ Override: every AskUserQuestion → auto-decide using the 6 principles. **Codex CEO voice** (via Bash): ```bash _REPO_ROOT=$(git rev-parse --show-toplevel) || { echo "ERROR: not in a git repo" >&2; exit 1; } - codex exec "IMPORTANT: Do NOT read or execute any SKILL.md files or files in skill definition directories (paths containing skills/gstack). These are AI assistant skill definitions meant for a different system. Stay focused on repository code only. + _gstack_codex_timeout_wrapper 600 codex exec "IMPORTANT: Do NOT read or execute any SKILL.md files or files in skill definition directories (paths containing skills/gstack). These are AI assistant skill definitions meant for a different system. Stay focused on repository code only. You are a CEO/founder advisor reviewing a development plan. Challenge the strategic foundations: Are the premises valid or assumed? Is this the @@ -265,9 +298,15 @@ Override: every AskUserQuestion → auto-decide using the 6 principles. What alternatives were dismissed too quickly? What competitive or market risks are unaddressed? What scope decisions will look foolish in 6 months? Be adversarial. No compliments. Just the strategic blind spots. - File: " -C "$_REPO_ROOT" -s read-only --enable web_search_cached + File: " -C "$_REPO_ROOT" -s read-only --enable web_search_cached < /dev/null + _CODEX_EXIT=$? + if [ "$_CODEX_EXIT" = "124" ]; then + _gstack_codex_log_event "codex_timeout" "600" + _gstack_codex_log_hang "autoplan" "0" + echo "[codex stalled past 10 minutes — tagging as [codex-unavailable] for this phase and proceeding with Claude subagent only]" + fi ``` - Timeout: 10 minutes + Timeout: 10 minutes (shell-wrapper) + 12 minutes (Bash outer gate). On hang, auto-degrades this phase's Codex voice. **Claude CEO subagent** (via Agent tool): "Read the plan file at . You are an independent CEO/strategist @@ -368,7 +407,7 @@ Override: every AskUserQuestion → auto-decide using the 6 principles. **Codex design voice** (via Bash): ```bash _REPO_ROOT=$(git rev-parse --show-toplevel) || { echo "ERROR: not in a git repo" >&2; exit 1; } - codex exec "IMPORTANT: Do NOT read or execute any SKILL.md files or files in skill definition directories (paths containing skills/gstack). These are AI assistant skill definitions meant for a different system. Stay focused on repository code only. + _gstack_codex_timeout_wrapper 600 codex exec "IMPORTANT: Do NOT read or execute any SKILL.md files or files in skill definition directories (paths containing skills/gstack). These are AI assistant skill definitions meant for a different system. Stay focused on repository code only. Read the plan file at . Evaluate this plan's UI/UX design decisions. @@ -382,9 +421,15 @@ Override: every AskUserQuestion → auto-decide using the 6 principles. accessibility requirements (keyboard nav, contrast, touch targets) specified or aspirational? Does the plan describe specific UI decisions or generic patterns? What design decisions will haunt the implementer if left ambiguous? - Be opinionated. No hedging." -C "$_REPO_ROOT" -s read-only --enable web_search_cached + Be opinionated. No hedging." -C "$_REPO_ROOT" -s read-only --enable web_search_cached < /dev/null + _CODEX_EXIT=$? + if [ "$_CODEX_EXIT" = "124" ]; then + _gstack_codex_log_event "codex_timeout" "600" + _gstack_codex_log_hang "autoplan" "0" + echo "[codex stalled past 10 minutes — tagging as [codex-unavailable] for this phase and proceeding with Claude subagent only]" + fi ``` - Timeout: 10 minutes + Timeout: 10 minutes (shell-wrapper) + 12 minutes (Bash outer gate). On hang, auto-degrades this phase's Codex voice. **Claude design subagent** (via Agent tool): "Read the plan file at . You are an independent senior product designer @@ -443,7 +488,7 @@ Override: every AskUserQuestion → auto-decide using the 6 principles. **Codex eng voice** (via Bash): ```bash _REPO_ROOT=$(git rev-parse --show-toplevel) || { echo "ERROR: not in a git repo" >&2; exit 1; } - codex exec "IMPORTANT: Do NOT read or execute any SKILL.md files or files in skill definition directories (paths containing skills/gstack). These are AI assistant skill definitions meant for a different system. Stay focused on repository code only. + _gstack_codex_timeout_wrapper 600 codex exec "IMPORTANT: Do NOT read or execute any SKILL.md files or files in skill definition directories (paths containing skills/gstack). These are AI assistant skill definitions meant for a different system. Stay focused on repository code only. Review this plan for architectural issues, missing edge cases, and hidden complexity. Be adversarial. @@ -452,9 +497,15 @@ Override: every AskUserQuestion → auto-decide using the 6 principles. CEO: Design: - File: " -C "$_REPO_ROOT" -s read-only --enable web_search_cached + File: " -C "$_REPO_ROOT" -s read-only --enable web_search_cached < /dev/null + _CODEX_EXIT=$? + if [ "$_CODEX_EXIT" = "124" ]; then + _gstack_codex_log_event "codex_timeout" "600" + _gstack_codex_log_hang "autoplan" "0" + echo "[codex stalled past 10 minutes — tagging as [codex-unavailable] for this phase and proceeding with Claude subagent only]" + fi ``` - Timeout: 10 minutes + Timeout: 10 minutes (shell-wrapper) + 12 minutes (Bash outer gate). On hang, auto-degrades this phase's Codex voice. **Claude eng subagent** (via Agent tool): "Read the plan file at . You are an independent senior engineer @@ -558,7 +609,7 @@ Log: "Phase 3.5 skipped — no developer-facing scope detected." **Codex DX voice** (via Bash): ```bash _REPO_ROOT=$(git rev-parse --show-toplevel) || { echo "ERROR: not in a git repo" >&2; exit 1; } - codex exec "IMPORTANT: Do NOT read or execute any SKILL.md files or files in skill definition directories (paths containing skills/gstack). These are AI assistant skill definitions meant for a different system. Stay focused on repository code only. + _gstack_codex_timeout_wrapper 600 codex exec "IMPORTANT: Do NOT read or execute any SKILL.md files or files in skill definition directories (paths containing skills/gstack). These are AI assistant skill definitions meant for a different system. Stay focused on repository code only. Read the plan file at . Evaluate this plan's developer experience. @@ -572,9 +623,15 @@ Log: "Phase 3.5 skipped — no developer-facing scope detected." 3. API/CLI design: are names guessable? Are defaults sensible? Is it consistent? 4. Docs: can a dev find what they need in under 2 minutes? Are examples copy-paste-complete? 5. Upgrade path: can devs upgrade without fear? Migration guides? Deprecation warnings? - Be adversarial. Think like a developer who is evaluating this against 3 competitors." -C "$_REPO_ROOT" -s read-only --enable web_search_cached + Be adversarial. Think like a developer who is evaluating this against 3 competitors." -C "$_REPO_ROOT" -s read-only --enable web_search_cached < /dev/null + _CODEX_EXIT=$? + if [ "$_CODEX_EXIT" = "124" ]; then + _gstack_codex_log_event "codex_timeout" "600" + _gstack_codex_log_hang "autoplan" "0" + echo "[codex stalled past 10 minutes — tagging as [codex-unavailable] for this phase and proceeding with Claude subagent only]" + fi ``` - Timeout: 10 minutes + Timeout: 10 minutes (shell-wrapper) + 12 minutes (Bash outer gate). On hang, auto-degrades this phase's Codex voice. **Claude DX subagent** (via Agent tool): "Read the plan file at . You are an independent DX engineer diff --git a/bin/gstack-codex-probe b/bin/gstack-codex-probe new file mode 100755 index 00000000..940dacf8 --- /dev/null +++ b/bin/gstack-codex-probe @@ -0,0 +1,102 @@ +#!/usr/bin/env bash +# gstack-codex-probe: shared helper for /codex and /autoplan skills. +# Sourced from template bash blocks; never execute directly. +# +# Functions (all prefixed with _gstack_codex_ for namespace hygiene): +# _gstack_codex_auth_probe — multi-signal auth check (env + file) +# _gstack_codex_version_check — warn on known-bad Codex CLI versions +# _gstack_codex_timeout_wrapper — gtimeout -> timeout -> unwrapped fallback +# _gstack_codex_log_event — telemetry emission to ~/.gstack/analytics/ +# +# Hygiene rules (enforced by test/codex-hardening.test.ts): +# - Never set -e / set -u / trap / IFS= / PATH= in this file. +# - All internal vars prefix with _GSTACK_CODEX_. +# - All functions prefix with _gstack_codex_. +# - No command execution at source time (only function defs). + +# --- Auth probe ------------------------------------------------------------- + +_gstack_codex_auth_probe() { + # Multi-signal: env vars OR auth file. Avoids false negatives for env-auth + # users (CI, platform engineers) that a file-only check would reject. + local _codex_home="${CODEX_HOME:-$HOME/.codex}" + # Use `-n` which returns true only for non-empty non-whitespace. Bash's [ -n ] + # alone allows whitespace; pair with a whitespace strip for robustness. + local _k1 _k2 + _k1=$(printf '%s' "${CODEX_API_KEY:-}" | tr -d '[:space:]') + _k2=$(printf '%s' "${OPENAI_API_KEY:-}" | tr -d '[:space:]') + if [ -n "$_k1" ] || [ -n "$_k2" ] || [ -f "$_codex_home/auth.json" ]; then + echo "AUTH_OK" + return 0 + fi + echo "AUTH_FAILED" + return 1 +} + +# --- Version check ---------------------------------------------------------- + +_gstack_codex_version_check() { + # Warn on known-bad Codex CLI versions. Anchored regex prevents false + # positives like 0.120.10 or 0.120.20 from matching. 0.120.2-beta still + # matches the bad release and gets warned (it IS buggy). + # Update this list when a new Codex CLI version regresses. + local _ver + _ver=$(codex --version 2>/dev/null | head -1) + [ -z "$_ver" ] && return 0 + if echo "$_ver" | grep -Eq '(^|[^0-9.])0\.120\.(0|1|2)([^0-9.]|$)'; then + echo "WARN: Codex CLI $_ver has known stdin deadlock bugs. Run: npm install -g @openai/codex@latest" + _gstack_codex_log_event "codex_version_warning" + fi +} + +# --- Timeout wrapper -------------------------------------------------------- + +_gstack_codex_timeout_wrapper() { + # Resolve wrapper binary: prefer gtimeout (Homebrew coreutils on macOS), + # fall back to timeout (Linux), else run unwrapped. Arguments: $1 is the + # duration in seconds; rest is the command to run. + local _duration="$1" + shift + local _to + _to=$(command -v gtimeout 2>/dev/null || command -v timeout 2>/dev/null || echo "") + if [ -n "$_to" ]; then + "$_to" "$_duration" "$@" + else + "$@" + fi +} + +# --- Telemetry event -------------------------------------------------------- + +_gstack_codex_log_event() { + # Emit a telemetry event to ~/.gstack/analytics/skill-usage.jsonl. + # Gated on $_TEL != "off" (caller sets this from gstack-config). + # Event types: codex_timeout, codex_auth_failed, codex_cli_missing, + # codex_version_warning. + # Payload schema: {skill, event, duration_s, ts}. NEVER includes prompt + # content, env var values, or auth tokens. + local _event="$1" + local _duration="${2:-0}" + [ "${_TEL:-off}" = "off" ] && return 0 + mkdir -p "$HOME/.gstack/analytics" 2>/dev/null || return 0 + local _ts + _ts=$(date -u +%Y-%m-%dT%H:%M:%SZ 2>/dev/null || echo unknown) + printf '{"skill":"codex","event":"%s","duration_s":"%s","ts":"%s"}\n' \ + "$_event" "$_duration" "$_ts" \ + >> "$HOME/.gstack/analytics/skill-usage.jsonl" 2>/dev/null || true +} + +# --- Learnings log on hang -------------------------------------------------- + +_gstack_codex_log_hang() { + # Invoked when a codex invocation times out (exit 124). Records an + # operational learning so future /investigate sessions surface the pattern. + # Best-effort: errors swallowed. + local _mode="${1:-unknown}" + local _prompt_size="${2:-0}" + local _log_bin="$HOME/.claude/skills/gstack/bin/gstack-learnings-log" + [ -x "$_log_bin" ] || return 0 + local _key="codex-hang-$(date +%s 2>/dev/null || echo unknown)" + "$_log_bin" "$(printf '{"skill":"codex","type":"operational","key":"%s","insight":"Codex timed out after 600s during [%s] invocation. Prompt size: %s. Consider splitting prompt or checking network.","confidence":8,"source":"observed","files":["codex/SKILL.md.tmpl","autoplan/SKILL.md.tmpl"]}' "$_key" "$_mode" "$_prompt_size")" \ + >/dev/null 2>&1 || true +} diff --git a/codex/SKILL.md b/codex/SKILL.md index 02dbcb29..7a890302 100644 --- a/codex/SKILL.md +++ b/codex/SKILL.md @@ -630,6 +630,45 @@ CODEX_BIN=$(which codex 2>/dev/null || echo "") If `NOT_FOUND`: stop and tell the user: "Codex CLI not found. Install it: `npm install -g @openai/codex` or see https://github.com/openai/codex" +If `NOT_FOUND`, also log the event: +```bash +_TEL=$(~/.claude/skills/gstack/bin/gstack-config get telemetry 2>/dev/null || echo off) +source ~/.claude/skills/gstack/bin/gstack-codex-probe 2>/dev/null && _gstack_codex_log_event "codex_cli_missing" 2>/dev/null || true +``` + +--- + +## Step 0.5: Auth probe + version check + +Before building expensive prompts, verify Codex has valid auth AND the installed +CLI version isn't in the known-bad list. Sourcing `gstack-codex-probe` loads the +shared helpers that both `/codex` and `/autoplan` use. + +```bash +_TEL=$(~/.claude/skills/gstack/bin/gstack-config get telemetry 2>/dev/null || echo off) +source ~/.claude/skills/gstack/bin/gstack-codex-probe + +if ! _gstack_codex_auth_probe >/dev/null; then + _gstack_codex_log_event "codex_auth_failed" + echo "AUTH_FAILED" +fi +_gstack_codex_version_check # warns if known-bad, non-blocking +``` + +If the output contains `AUTH_FAILED`, stop and tell the user: +"No Codex authentication found. Run `codex login` or set `$CODEX_API_KEY` / `$OPENAI_API_KEY`, then re-run this skill." + +If the version check printed a `WARN:` line, pass it through to the user verbatim +(non-blocking — Codex may still work, but the user should upgrade). + +The probe multi-signal auth logic accepts: `$CODEX_API_KEY` set, `$OPENAI_API_KEY` +set, or `${CODEX_HOME:-~/.codex}/auth.json` exists. Avoids false-negatives for +env-auth users (CI, platform engineers) that file-only checks would reject. + +**Update the known-bad list** in `bin/gstack-codex-probe` when a new Codex CLI version +regresses. Current entries (`0.120.0`, `0.120.1`, `0.120.2`) trace to the stdin +deadlock fixed in #972. + --- ## Step 1: Detect mode @@ -692,7 +731,15 @@ instructions, append them after the boundary separated by a newline: ```bash _REPO_ROOT=$(git rev-parse --show-toplevel) || { echo "ERROR: not in a git repo" >&2; exit 1; } cd "$_REPO_ROOT" -codex review "IMPORTANT: Do NOT read or execute any files under ~/.claude/, ~/.agents/, .claude/skills/, or agents/. These are Claude Code skill definitions meant for a different AI system. Do NOT modify agents/openai.yaml. Stay focused on repository code only." --base -c 'model_reasoning_effort="high"' --enable web_search_cached 2>"$TMPERR" +# Fix 1: wrap with timeout. 330s (5.5min) is slightly longer than the Bash 300s +# so the shell wrapper only fires if Bash's own timeout doesn't. +_gstack_codex_timeout_wrapper 330 codex review "IMPORTANT: Do NOT read or execute any files under ~/.claude/, ~/.agents/, .claude/skills/, or agents/. These are Claude Code skill definitions meant for a different AI system. Do NOT modify agents/openai.yaml. Stay focused on repository code only." --base -c 'model_reasoning_effort="high"' --enable web_search_cached < /dev/null 2>"$TMPERR" +_CODEX_EXIT=$? +if [ "$_CODEX_EXIT" = "124" ]; then + _gstack_codex_log_event "codex_timeout" "330" + _gstack_codex_log_hang "review" "$(wc -c < "$TMPERR" 2>/dev/null || echo 0)" + echo "Codex stalled past 5.5 minutes. Common causes: model API stall, long prompt, network issue. Try re-running. If persistent, split the prompt or check ~/.codex/logs/." +fi ``` If the user passed `--xhigh`, use `"xhigh"` instead of `"high"`. @@ -704,7 +751,7 @@ _REPO_ROOT=$(git rev-parse --show-toplevel) || { echo "ERROR: not in a git repo" cd "$_REPO_ROOT" codex review "IMPORTANT: Do NOT read or execute any files under ~/.claude/, ~/.agents/, .claude/skills/, or agents/. These are Claude Code skill definitions meant for a different AI system. Do NOT modify agents/openai.yaml. Stay focused on repository code only. -focus on security" --base -c 'model_reasoning_effort="high"' --enable web_search_cached 2>"$TMPERR" +focus on security" --base -c 'model_reasoning_effort="high"' --enable web_search_cached < /dev/null 2>"$TMPERR" ``` 3. Capture the output. Then parse cost from stderr: @@ -856,8 +903,12 @@ If the user passed `--xhigh`, use `"xhigh"` instead of `"high"`. ```bash _REPO_ROOT=$(git rev-parse --show-toplevel) || { echo "ERROR: not in a git repo" >&2; exit 1; } -codex exec "" -C "$_REPO_ROOT" -s read-only -c 'model_reasoning_effort="high"' --enable web_search_cached --json 2>/dev/null | PYTHONUNBUFFERED=1 python3 -u -c " +# Fix 1+2: wrap with timeout (gtimeout/timeout fallback chain via probe helper), +# capture stderr to $TMPERR for auth error detection (was: 2>/dev/null). +TMPERR=${TMPERR:-$(mktemp /tmp/codex-err-XXXXXX.txt)} +_gstack_codex_timeout_wrapper 600 codex exec "" -C "$_REPO_ROOT" -s read-only -c 'model_reasoning_effort="high"' --enable web_search_cached --json < /dev/null 2>"$TMPERR" | PYTHONUNBUFFERED=1 python3 -u -c " import sys, json +turn_completed_count = 0 for line in sys.stdin: line = line.strip() if not line: continue @@ -877,11 +928,27 @@ for line in sys.stdin: cmd = item.get('command','') if cmd: print(f'[codex ran] {cmd}', flush=True) elif t == 'turn.completed': + turn_completed_count += 1 usage = obj.get('usage',{}) tokens = usage.get('input_tokens',0) + usage.get('output_tokens',0) if tokens: print(f'\ntokens used: {tokens}', flush=True) except: pass +# Fix 2: completeness check — warn if no turn.completed received +if turn_completed_count == 0: + print('[codex warning] No turn.completed event received — possible mid-stream disconnect.', flush=True, file=sys.stderr) " +_CODEX_EXIT=${PIPESTATUS[0]} +# Fix 1: hang detection — log + surface actionable message +if [ "$_CODEX_EXIT" = "124" ]; then + _gstack_codex_log_event "codex_timeout" "600" + _gstack_codex_log_hang "challenge" "$(wc -c < "$TMPERR" 2>/dev/null || echo 0)" + echo "Codex stalled past 10 minutes. Common causes: model API stall, long prompt, network issue. Try re-running. If persistent, split the prompt or check ~/.codex/logs/." +fi +# Fix 2: surface auth errors from captured stderr instead of dropping them +if grep -qiE "auth|login|unauthorized" "$TMPERR" 2>/dev/null; then + echo "[codex auth error] $(head -1 "$TMPERR")" + _gstack_codex_log_event "codex_auth_failed" +fi ``` This parses codex's JSONL events to extract reasoning traces, tool calls, and the final @@ -968,7 +1035,8 @@ If the user passed `--xhigh`, use `"xhigh"` instead of `"medium"`. For a **new session:** ```bash _REPO_ROOT=$(git rev-parse --show-toplevel) || { echo "ERROR: not in a git repo" >&2; exit 1; } -codex exec "" -C "$_REPO_ROOT" -s read-only -c 'model_reasoning_effort="medium"' --enable web_search_cached --json 2>"$TMPERR" | PYTHONUNBUFFERED=1 python3 -u -c " +# Fix 1: wrap with timeout (gtimeout/timeout fallback chain via probe helper) +_gstack_codex_timeout_wrapper 600 codex exec "" -C "$_REPO_ROOT" -s read-only -c 'model_reasoning_effort="medium"' --enable web_search_cached --json < /dev/null 2>"$TMPERR" | PYTHONUNBUFFERED=1 python3 -u -c " import sys, json for line in sys.stdin: line = line.strip() @@ -997,15 +1065,29 @@ for line in sys.stdin: if tokens: print(f'\ntokens used: {tokens}', flush=True) except: pass " +# Fix 1: hang detection for Consult new-session (mirrors Challenge + resume) +_CODEX_EXIT=${PIPESTATUS[0]} +if [ "$_CODEX_EXIT" = "124" ]; then + _gstack_codex_log_event "codex_timeout" "600" + _gstack_codex_log_hang "consult" "$(wc -c < "$TMPERR" 2>/dev/null || echo 0)" + echo "Codex stalled past 10 minutes. Common causes: model API stall, long prompt, network issue. Try re-running. If persistent, split the prompt or check ~/.codex/logs/." +fi ``` For a **resumed session** (user chose "Continue"): ```bash _REPO_ROOT=$(git rev-parse --show-toplevel) || { echo "ERROR: not in a git repo" >&2; exit 1; } -codex exec resume "" -C "$_REPO_ROOT" -s read-only -c 'model_reasoning_effort="medium"' --enable web_search_cached --json 2>"$TMPERR" | PYTHONUNBUFFERED=1 python3 -u -c " +# Fix 1: wrap with timeout (gtimeout/timeout fallback chain via probe helper) +_gstack_codex_timeout_wrapper 600 codex exec resume "" -C "$_REPO_ROOT" -s read-only -c 'model_reasoning_effort="medium"' --enable web_search_cached --json < /dev/null 2>"$TMPERR" | PYTHONUNBUFFERED=1 python3 -u -c " " -``` +# Fix 1: same hang detection pattern as new-session block +_CODEX_EXIT=${PIPESTATUS[0]} +if [ "$_CODEX_EXIT" = "124" ]; then + _gstack_codex_log_event "codex_timeout" "600" + _gstack_codex_log_hang "consult-resume" "$(wc -c < "$TMPERR" 2>/dev/null || echo 0)" + echo "Codex stalled past 10 minutes. Common causes: model API stall, long prompt, network issue. Try re-running. If persistent, split the prompt or check ~/.codex/logs/." +fi 5. Capture session ID from the streamed output. The parser prints `SESSION_ID:` from the `thread.started` event. Save it for follow-ups: @@ -1070,8 +1152,9 @@ If token count is not available, display: `Tokens: unknown` - **Binary not found:** Detected in Step 0. Stop with install instructions. - **Auth error:** Codex prints an auth error to stderr. Surface the error: "Codex authentication failed. Run `codex login` in your terminal to authenticate via ChatGPT." -- **Timeout:** If the Bash call times out (5 min), tell the user: - "Codex timed out after 5 minutes. The diff may be too large or the API may be slow. Try again or use a smaller scope." +- **Timeout (Bash outer gate):** If the Bash call times out (5 min for Review/Challenge, 10 min for Consult), tell the user: + "Codex timed out. The prompt may be too large or the API may be slow. Try again or use a smaller scope." +- **Timeout (inner `timeout` wrapper, exit 124):** If the shell `timeout 600` wrapper fires first, the skill's hang-detection block auto-logs a telemetry event + operational learning and prints: "Codex stalled past 10 minutes. Common causes: model API stall, long prompt, network issue. Try re-running. If persistent, split the prompt or check `~/.codex/logs/`." No extra action needed. - **Empty response:** If `$TMPRESP` is empty or doesn't exist, tell the user: "Codex returned no response. Check stderr for errors." - **Session resume failure:** If resume fails, delete the session file and start fresh. diff --git a/codex/SKILL.md.tmpl b/codex/SKILL.md.tmpl index 105b5383..c311fc80 100644 --- a/codex/SKILL.md.tmpl +++ b/codex/SKILL.md.tmpl @@ -49,6 +49,45 @@ CODEX_BIN=$(which codex 2>/dev/null || echo "") If `NOT_FOUND`: stop and tell the user: "Codex CLI not found. Install it: `npm install -g @openai/codex` or see https://github.com/openai/codex" +If `NOT_FOUND`, also log the event: +```bash +_TEL=$(~/.claude/skills/gstack/bin/gstack-config get telemetry 2>/dev/null || echo off) +source ~/.claude/skills/gstack/bin/gstack-codex-probe 2>/dev/null && _gstack_codex_log_event "codex_cli_missing" 2>/dev/null || true +``` + +--- + +## Step 0.5: Auth probe + version check + +Before building expensive prompts, verify Codex has valid auth AND the installed +CLI version isn't in the known-bad list. Sourcing `gstack-codex-probe` loads the +shared helpers that both `/codex` and `/autoplan` use. + +```bash +_TEL=$(~/.claude/skills/gstack/bin/gstack-config get telemetry 2>/dev/null || echo off) +source ~/.claude/skills/gstack/bin/gstack-codex-probe + +if ! _gstack_codex_auth_probe >/dev/null; then + _gstack_codex_log_event "codex_auth_failed" + echo "AUTH_FAILED" +fi +_gstack_codex_version_check # warns if known-bad, non-blocking +``` + +If the output contains `AUTH_FAILED`, stop and tell the user: +"No Codex authentication found. Run `codex login` or set `$CODEX_API_KEY` / `$OPENAI_API_KEY`, then re-run this skill." + +If the version check printed a `WARN:` line, pass it through to the user verbatim +(non-blocking — Codex may still work, but the user should upgrade). + +The probe multi-signal auth logic accepts: `$CODEX_API_KEY` set, `$OPENAI_API_KEY` +set, or `${CODEX_HOME:-~/.codex}/auth.json` exists. Avoids false-negatives for +env-auth users (CI, platform engineers) that file-only checks would reject. + +**Update the known-bad list** in `bin/gstack-codex-probe` when a new Codex CLI version +regresses. Current entries (`0.120.0`, `0.120.1`, `0.120.2`) trace to the stdin +deadlock fixed in #972. + --- ## Step 1: Detect mode @@ -111,7 +150,15 @@ instructions, append them after the boundary separated by a newline: ```bash _REPO_ROOT=$(git rev-parse --show-toplevel) || { echo "ERROR: not in a git repo" >&2; exit 1; } cd "$_REPO_ROOT" -codex review "IMPORTANT: Do NOT read or execute any files under ~/.claude/, ~/.agents/, .claude/skills/, or agents/. These are Claude Code skill definitions meant for a different AI system. Do NOT modify agents/openai.yaml. Stay focused on repository code only." --base -c 'model_reasoning_effort="high"' --enable web_search_cached 2>"$TMPERR" +# Fix 1: wrap with timeout. 330s (5.5min) is slightly longer than the Bash 300s +# so the shell wrapper only fires if Bash's own timeout doesn't. +_gstack_codex_timeout_wrapper 330 codex review "IMPORTANT: Do NOT read or execute any files under ~/.claude/, ~/.agents/, .claude/skills/, or agents/. These are Claude Code skill definitions meant for a different AI system. Do NOT modify agents/openai.yaml. Stay focused on repository code only." --base -c 'model_reasoning_effort="high"' --enable web_search_cached < /dev/null 2>"$TMPERR" +_CODEX_EXIT=$? +if [ "$_CODEX_EXIT" = "124" ]; then + _gstack_codex_log_event "codex_timeout" "330" + _gstack_codex_log_hang "review" "$(wc -c < "$TMPERR" 2>/dev/null || echo 0)" + echo "Codex stalled past 5.5 minutes. Common causes: model API stall, long prompt, network issue. Try re-running. If persistent, split the prompt or check ~/.codex/logs/." +fi ``` If the user passed `--xhigh`, use `"xhigh"` instead of `"high"`. @@ -123,7 +170,7 @@ _REPO_ROOT=$(git rev-parse --show-toplevel) || { echo "ERROR: not in a git repo" cd "$_REPO_ROOT" codex review "IMPORTANT: Do NOT read or execute any files under ~/.claude/, ~/.agents/, .claude/skills/, or agents/. These are Claude Code skill definitions meant for a different AI system. Do NOT modify agents/openai.yaml. Stay focused on repository code only. -focus on security" --base -c 'model_reasoning_effort="high"' --enable web_search_cached 2>"$TMPERR" +focus on security" --base -c 'model_reasoning_effort="high"' --enable web_search_cached < /dev/null 2>"$TMPERR" ``` 3. Capture the output. Then parse cost from stderr: @@ -205,8 +252,12 @@ If the user passed `--xhigh`, use `"xhigh"` instead of `"high"`. ```bash _REPO_ROOT=$(git rev-parse --show-toplevel) || { echo "ERROR: not in a git repo" >&2; exit 1; } -codex exec "" -C "$_REPO_ROOT" -s read-only -c 'model_reasoning_effort="high"' --enable web_search_cached --json 2>/dev/null | PYTHONUNBUFFERED=1 python3 -u -c " +# Fix 1+2: wrap with timeout (gtimeout/timeout fallback chain via probe helper), +# capture stderr to $TMPERR for auth error detection (was: 2>/dev/null). +TMPERR=${TMPERR:-$(mktemp /tmp/codex-err-XXXXXX.txt)} +_gstack_codex_timeout_wrapper 600 codex exec "" -C "$_REPO_ROOT" -s read-only -c 'model_reasoning_effort="high"' --enable web_search_cached --json < /dev/null 2>"$TMPERR" | PYTHONUNBUFFERED=1 python3 -u -c " import sys, json +turn_completed_count = 0 for line in sys.stdin: line = line.strip() if not line: continue @@ -226,11 +277,27 @@ for line in sys.stdin: cmd = item.get('command','') if cmd: print(f'[codex ran] {cmd}', flush=True) elif t == 'turn.completed': + turn_completed_count += 1 usage = obj.get('usage',{}) tokens = usage.get('input_tokens',0) + usage.get('output_tokens',0) if tokens: print(f'\ntokens used: {tokens}', flush=True) except: pass +# Fix 2: completeness check — warn if no turn.completed received +if turn_completed_count == 0: + print('[codex warning] No turn.completed event received — possible mid-stream disconnect.', flush=True, file=sys.stderr) " +_CODEX_EXIT=${PIPESTATUS[0]} +# Fix 1: hang detection — log + surface actionable message +if [ "$_CODEX_EXIT" = "124" ]; then + _gstack_codex_log_event "codex_timeout" "600" + _gstack_codex_log_hang "challenge" "$(wc -c < "$TMPERR" 2>/dev/null || echo 0)" + echo "Codex stalled past 10 minutes. Common causes: model API stall, long prompt, network issue. Try re-running. If persistent, split the prompt or check ~/.codex/logs/." +fi +# Fix 2: surface auth errors from captured stderr instead of dropping them +if grep -qiE "auth|login|unauthorized" "$TMPERR" 2>/dev/null; then + echo "[codex auth error] $(head -1 "$TMPERR")" + _gstack_codex_log_event "codex_auth_failed" +fi ``` This parses codex's JSONL events to extract reasoning traces, tool calls, and the final @@ -317,7 +384,8 @@ If the user passed `--xhigh`, use `"xhigh"` instead of `"medium"`. For a **new session:** ```bash _REPO_ROOT=$(git rev-parse --show-toplevel) || { echo "ERROR: not in a git repo" >&2; exit 1; } -codex exec "" -C "$_REPO_ROOT" -s read-only -c 'model_reasoning_effort="medium"' --enable web_search_cached --json 2>"$TMPERR" | PYTHONUNBUFFERED=1 python3 -u -c " +# Fix 1: wrap with timeout (gtimeout/timeout fallback chain via probe helper) +_gstack_codex_timeout_wrapper 600 codex exec "" -C "$_REPO_ROOT" -s read-only -c 'model_reasoning_effort="medium"' --enable web_search_cached --json < /dev/null 2>"$TMPERR" | PYTHONUNBUFFERED=1 python3 -u -c " import sys, json for line in sys.stdin: line = line.strip() @@ -346,15 +414,29 @@ for line in sys.stdin: if tokens: print(f'\ntokens used: {tokens}', flush=True) except: pass " +# Fix 1: hang detection for Consult new-session (mirrors Challenge + resume) +_CODEX_EXIT=${PIPESTATUS[0]} +if [ "$_CODEX_EXIT" = "124" ]; then + _gstack_codex_log_event "codex_timeout" "600" + _gstack_codex_log_hang "consult" "$(wc -c < "$TMPERR" 2>/dev/null || echo 0)" + echo "Codex stalled past 10 minutes. Common causes: model API stall, long prompt, network issue. Try re-running. If persistent, split the prompt or check ~/.codex/logs/." +fi ``` For a **resumed session** (user chose "Continue"): ```bash _REPO_ROOT=$(git rev-parse --show-toplevel) || { echo "ERROR: not in a git repo" >&2; exit 1; } -codex exec resume "" -C "$_REPO_ROOT" -s read-only -c 'model_reasoning_effort="medium"' --enable web_search_cached --json 2>"$TMPERR" | PYTHONUNBUFFERED=1 python3 -u -c " +# Fix 1: wrap with timeout (gtimeout/timeout fallback chain via probe helper) +_gstack_codex_timeout_wrapper 600 codex exec resume "" -C "$_REPO_ROOT" -s read-only -c 'model_reasoning_effort="medium"' --enable web_search_cached --json < /dev/null 2>"$TMPERR" | PYTHONUNBUFFERED=1 python3 -u -c " " -``` +# Fix 1: same hang detection pattern as new-session block +_CODEX_EXIT=${PIPESTATUS[0]} +if [ "$_CODEX_EXIT" = "124" ]; then + _gstack_codex_log_event "codex_timeout" "600" + _gstack_codex_log_hang "consult-resume" "$(wc -c < "$TMPERR" 2>/dev/null || echo 0)" + echo "Codex stalled past 10 minutes. Common causes: model API stall, long prompt, network issue. Try re-running. If persistent, split the prompt or check ~/.codex/logs/." +fi 5. Capture session ID from the streamed output. The parser prints `SESSION_ID:` from the `thread.started` event. Save it for follow-ups: @@ -419,8 +501,9 @@ If token count is not available, display: `Tokens: unknown` - **Binary not found:** Detected in Step 0. Stop with install instructions. - **Auth error:** Codex prints an auth error to stderr. Surface the error: "Codex authentication failed. Run `codex login` in your terminal to authenticate via ChatGPT." -- **Timeout:** If the Bash call times out (5 min), tell the user: - "Codex timed out after 5 minutes. The diff may be too large or the API may be slow. Try again or use a smaller scope." +- **Timeout (Bash outer gate):** If the Bash call times out (5 min for Review/Challenge, 10 min for Consult), tell the user: + "Codex timed out. The prompt may be too large or the API may be slow. Try again or use a smaller scope." +- **Timeout (inner `timeout` wrapper, exit 124):** If the shell `timeout 600` wrapper fires first, the skill's hang-detection block auto-logs a telemetry event + operational learning and prints: "Codex stalled past 10 minutes. Common causes: model API stall, long prompt, network issue. Try re-running. If persistent, split the prompt or check `~/.codex/logs/`." No extra action needed. - **Empty response:** If `$TMPRESP` is empty or doesn't exist, tell the user: "Codex returned no response. Check stderr for errors." - **Session resume failure:** If resume fails, delete the session file and start fresh. diff --git a/design-consultation/SKILL.md b/design-consultation/SKILL.md index baa0f00b..d1dcb4d9 100644 --- a/design-consultation/SKILL.md +++ b/design-consultation/SKILL.md @@ -836,7 +836,7 @@ codex exec "Given this product context, propose a complete design direction: - Differentiation: 2 deliberate departures from category norms - Anti-slop: no purple gradients, no 3-column icon grids, no centered everything, no decorative blobs -Be opinionated. Be specific. Do not hedge. This is YOUR design direction — own it." -C "$_REPO_ROOT" -s read-only -c 'model_reasoning_effort="medium"' --enable web_search_cached 2>"$TMPERR_DESIGN" +Be opinionated. Be specific. Do not hedge. This is YOUR design direction — own it." -C "$_REPO_ROOT" -s read-only -c 'model_reasoning_effort="medium"' --enable web_search_cached < /dev/null 2>"$TMPERR_DESIGN" ``` Use a 5-minute timeout (`timeout: 300000`). After the command completes, read stderr: ```bash diff --git a/design-review/SKILL.md b/design-review/SKILL.md index e4fe88e7..f0fd5f49 100644 --- a/design-review/SKILL.md +++ b/design-review/SKILL.md @@ -1532,7 +1532,7 @@ HARD REJECTION — flag if ANY apply: 6. Carousel with no narrative purpose 7. App UI made of stacked cards instead of layout -Be specific. Reference file:line for every finding." -C "$_REPO_ROOT" -s read-only -c 'model_reasoning_effort="high"' --enable web_search_cached 2>"$TMPERR_DESIGN" +Be specific. Reference file:line for every finding." -C "$_REPO_ROOT" -s read-only -c 'model_reasoning_effort="high"' --enable web_search_cached < /dev/null 2>"$TMPERR_DESIGN" ``` Use a 5-minute timeout (`timeout: 300000`). After the command completes, read stderr: ```bash diff --git a/office-hours/SKILL.md b/office-hours/SKILL.md index 699e4a58..8355e52e 100644 --- a/office-hours/SKILL.md +++ b/office-hours/SKILL.md @@ -1025,7 +1025,7 @@ Then add the context block and mode-appropriate instructions: ```bash TMPERR_OH=$(mktemp /tmp/codex-oh-err-XXXXXXXX) _REPO_ROOT=$(git rev-parse --show-toplevel) || { echo "ERROR: not in a git repo" >&2; exit 1; } -codex exec "$(cat "$CODEX_PROMPT_FILE")" -C "$_REPO_ROOT" -s read-only -c 'model_reasoning_effort="high"' --enable web_search_cached 2>"$TMPERR_OH" +codex exec "$(cat "$CODEX_PROMPT_FILE")" -C "$_REPO_ROOT" -s read-only -c 'model_reasoning_effort="high"' --enable web_search_cached < /dev/null 2>"$TMPERR_OH" ``` Use a 5-minute timeout (`timeout: 300000`). After the command completes, read stderr: @@ -1270,7 +1270,7 @@ If user chooses A, launch both voices simultaneously: ```bash TMPERR_SKETCH=$(mktemp /tmp/codex-sketch-XXXXXXXX) _REPO_ROOT=$(git rev-parse --show-toplevel) || { echo "ERROR: not in a git repo" >&2; exit 1; } -codex exec "For this product approach, provide: a visual thesis (one sentence — mood, material, energy), a content plan (hero → support → detail → CTA), and 2 interaction ideas that change page feel. Apply beautiful defaults: composition-first, brand-first, cardless, poster not document. Be opinionated." -C "$_REPO_ROOT" -s read-only -c 'model_reasoning_effort="medium"' --enable web_search_cached 2>"$TMPERR_SKETCH" +codex exec "For this product approach, provide: a visual thesis (one sentence — mood, material, energy), a content plan (hero → support → detail → CTA), and 2 interaction ideas that change page feel. Apply beautiful defaults: composition-first, brand-first, cardless, poster not document. Be opinionated." -C "$_REPO_ROOT" -s read-only -c 'model_reasoning_effort="medium"' --enable web_search_cached < /dev/null 2>"$TMPERR_SKETCH" ``` Use a 5-minute timeout (`timeout: 300000`). After completion: `cat "$TMPERR_SKETCH" && rm -f "$TMPERR_SKETCH"` diff --git a/package.json b/package.json index 5222ec4c..87d17e3c 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "gstack", - "version": "0.18.3.0", + "version": "0.18.4.0", "description": "Garry's Stack — Claude Code skills + fast headless browser. One repo, one install, entire AI engineering workflow.", "license": "MIT", "type": "module", diff --git a/plan-ceo-review/SKILL.md b/plan-ceo-review/SKILL.md index c2fc9bbb..75aab7c3 100644 --- a/plan-ceo-review/SKILL.md +++ b/plan-ceo-review/SKILL.md @@ -644,7 +644,7 @@ Do NOT make any code changes. Do NOT start implementation. Your only job right n * I want code that's "engineered enough" — not under-engineered (fragile, hacky) and not over-engineered (premature abstraction, unnecessary complexity). * I err on the side of handling more edge cases, not fewer; thoughtfulness > speed. * Bias toward explicit over clever. -* Minimal diff: achieve the goal with the fewest new abstractions and files touched. +* Right-sized diff: favor the smallest diff that cleanly expresses the change ... but don't compress a necessary rewrite into a minimal patch. If the existing foundation is broken, invoke permission #9 and say "scrap it and do this instead." * Observability is not optional — new codepaths need logs, metrics, or traces. * Security is not optional — new codepaths need threat modeling. * Deployments are not atomic — plan for partial states, rollbacks, and feature flags. @@ -935,6 +935,7 @@ Rules: - At least 2 approaches required. 3 preferred for non-trivial plans. - One approach must be the "minimal viable" (fewest files, smallest diff). - One approach must be the "ideal architecture" (best long-term trajectory). +- **These two approaches have equal weight.** Don't default to "minimal viable" just because it's smaller. Recommend whichever best serves the user's goal. If the right answer is a rewrite, say so. - If only one approach exists, explain concretely why alternatives were eliminated. - Do NOT proceed to mode selection (0F) without user approval of the chosen approach. @@ -1419,7 +1420,7 @@ THE PLAN: ```bash TMPERR_PV=$(mktemp /tmp/codex-planreview-XXXXXXXX) _REPO_ROOT=$(git rev-parse --show-toplevel) || { echo "ERROR: not in a git repo" >&2; exit 1; } -codex exec "" -C "$_REPO_ROOT" -s read-only -c 'model_reasoning_effort="high"' --enable web_search_cached 2>"$TMPERR_PV" +codex exec "" -C "$_REPO_ROOT" -s read-only -c 'model_reasoning_effort="high"' --enable web_search_cached < /dev/null 2>"$TMPERR_PV" ``` Use a 5-minute timeout (`timeout: 300000`). After the command completes, read stderr: diff --git a/plan-ceo-review/SKILL.md.tmpl b/plan-ceo-review/SKILL.md.tmpl index d128b180..93d1af0a 100644 --- a/plan-ceo-review/SKILL.md.tmpl +++ b/plan-ceo-review/SKILL.md.tmpl @@ -60,7 +60,7 @@ Do NOT make any code changes. Do NOT start implementation. Your only job right n * I want code that's "engineered enough" — not under-engineered (fragile, hacky) and not over-engineered (premature abstraction, unnecessary complexity). * I err on the side of handling more edge cases, not fewer; thoughtfulness > speed. * Bias toward explicit over clever. -* Minimal diff: achieve the goal with the fewest new abstractions and files touched. +* Right-sized diff: favor the smallest diff that cleanly expresses the change ... but don't compress a necessary rewrite into a minimal patch. If the existing foundation is broken, invoke permission #9 and say "scrap it and do this instead." * Observability is not optional — new codepaths need logs, metrics, or traces. * Security is not optional — new codepaths need threat modeling. * Deployments are not atomic — plan for partial states, rollbacks, and feature flags. @@ -242,6 +242,7 @@ Rules: - At least 2 approaches required. 3 preferred for non-trivial plans. - One approach must be the "minimal viable" (fewest files, smallest diff). - One approach must be the "ideal architecture" (best long-term trajectory). +- **These two approaches have equal weight.** Don't default to "minimal viable" just because it's smaller. Recommend whichever best serves the user's goal. If the right answer is a rewrite, say so. - If only one approach exists, explain concretely why alternatives were eliminated. - Do NOT proceed to mode selection (0F) without user approval of the chosen approach. diff --git a/plan-design-review/SKILL.md b/plan-design-review/SKILL.md index e8bde0ec..52002009 100644 --- a/plan-design-review/SKILL.md +++ b/plan-design-review/SKILL.md @@ -1083,7 +1083,7 @@ HARD RULES — first classify as MARKETING/LANDING PAGE vs APP UI vs HYBRID, the - APP UI: Calm surface hierarchy, dense but readable, utility language, minimal chrome - UNIVERSAL: CSS variables for colors, no default font stacks, one job per section, cards earn existence -For each finding: what's wrong, what will happen if it ships unresolved, and the specific fix. Be opinionated. No hedging." -C "$_REPO_ROOT" -s read-only -c 'model_reasoning_effort="high"' --enable web_search_cached 2>"$TMPERR_DESIGN" +For each finding: what's wrong, what will happen if it ships unresolved, and the specific fix. Be opinionated. No hedging." -C "$_REPO_ROOT" -s read-only -c 'model_reasoning_effort="high"' --enable web_search_cached < /dev/null 2>"$TMPERR_DESIGN" ``` Use a 5-minute timeout (`timeout: 300000`). After the command completes, read stderr: ```bash diff --git a/plan-devex-review/SKILL.md b/plan-devex-review/SKILL.md index 623c8e7c..2b10f62e 100644 --- a/plan-devex-review/SKILL.md +++ b/plan-devex-review/SKILL.md @@ -1436,7 +1436,7 @@ THE PLAN: ```bash TMPERR_PV=$(mktemp /tmp/codex-planreview-XXXXXXXX) _REPO_ROOT=$(git rev-parse --show-toplevel) || { echo "ERROR: not in a git repo" >&2; exit 1; } -codex exec "" -C "$_REPO_ROOT" -s read-only -c 'model_reasoning_effort="high"' --enable web_search_cached 2>"$TMPERR_PV" +codex exec "" -C "$_REPO_ROOT" -s read-only -c 'model_reasoning_effort="high"' --enable web_search_cached < /dev/null 2>"$TMPERR_PV" ``` Use a 5-minute timeout (`timeout: 300000`). After the command completes, read stderr: diff --git a/plan-eng-review/SKILL.md b/plan-eng-review/SKILL.md index 1b2482e1..9fe128ef 100644 --- a/plan-eng-review/SKILL.md +++ b/plan-eng-review/SKILL.md @@ -589,7 +589,7 @@ If the user asks you to compress or the system triggers context compaction: Step * I want code that's "engineered enough" — not under-engineered (fragile, hacky) and not over-engineered (premature abstraction, unnecessary complexity). * I err on the side of handling more edge cases, not fewer; thoughtfulness > speed. * Bias toward explicit over clever. -* Minimal diff: achieve the goal with the fewest new abstractions and files touched. +* Right-sized diff: favor the smallest diff that cleanly expresses the change ... but don't compress a necessary rewrite into a minimal patch. If the existing foundation is broken, say "scrap it and do this instead." ## Cognitive Patterns — How Great Eng Managers Think @@ -1075,7 +1075,7 @@ THE PLAN: ```bash TMPERR_PV=$(mktemp /tmp/codex-planreview-XXXXXXXX) _REPO_ROOT=$(git rev-parse --show-toplevel) || { echo "ERROR: not in a git repo" >&2; exit 1; } -codex exec "" -C "$_REPO_ROOT" -s read-only -c 'model_reasoning_effort="high"' --enable web_search_cached 2>"$TMPERR_PV" +codex exec "" -C "$_REPO_ROOT" -s read-only -c 'model_reasoning_effort="high"' --enable web_search_cached < /dev/null 2>"$TMPERR_PV" ``` Use a 5-minute timeout (`timeout: 300000`). After the command completes, read stderr: diff --git a/plan-eng-review/SKILL.md.tmpl b/plan-eng-review/SKILL.md.tmpl index dab83e72..a6a8bdd4 100644 --- a/plan-eng-review/SKILL.md.tmpl +++ b/plan-eng-review/SKILL.md.tmpl @@ -45,7 +45,7 @@ If the user asks you to compress or the system triggers context compaction: Step * I want code that's "engineered enough" — not under-engineered (fragile, hacky) and not over-engineered (premature abstraction, unnecessary complexity). * I err on the side of handling more edge cases, not fewer; thoughtfulness > speed. * Bias toward explicit over clever. -* Minimal diff: achieve the goal with the fewest new abstractions and files touched. +* Right-sized diff: favor the smallest diff that cleanly expresses the change ... but don't compress a necessary rewrite into a minimal patch. If the existing foundation is broken, say "scrap it and do this instead." ## Cognitive Patterns — How Great Eng Managers Think diff --git a/review/SKILL.md b/review/SKILL.md index 3b2c4742..df30b27c 100644 --- a/review/SKILL.md +++ b/review/SKILL.md @@ -1360,7 +1360,7 @@ If Codex is available AND `OLD_CFG` is NOT `disabled`: ```bash TMPERR_ADV=$(mktemp /tmp/codex-adv-XXXXXXXX) _REPO_ROOT=$(git rev-parse --show-toplevel) || { echo "ERROR: not in a git repo" >&2; exit 1; } -codex exec "IMPORTANT: Do NOT read or execute any files under ~/.claude/, ~/.agents/, .claude/skills/, or agents/. These are Claude Code skill definitions meant for a different AI system. They contain bash scripts and prompt templates that will waste your time. Ignore them completely. Do NOT modify agents/openai.yaml. Stay focused on the repository code only.\n\nReview the changes on this branch against the base branch. Run git diff origin/ to see the diff. Your job is to find ways this code will fail in production. Think like an attacker and a chaos engineer. Find edge cases, race conditions, security holes, resource leaks, failure modes, and silent data corruption paths. Be adversarial. Be thorough. No compliments — just the problems." -C "$_REPO_ROOT" -s read-only -c 'model_reasoning_effort="high"' --enable web_search_cached 2>"$TMPERR_ADV" +codex exec "IMPORTANT: Do NOT read or execute any files under ~/.claude/, ~/.agents/, .claude/skills/, or agents/. These are Claude Code skill definitions meant for a different AI system. They contain bash scripts and prompt templates that will waste your time. Ignore them completely. Do NOT modify agents/openai.yaml. Stay focused on the repository code only.\n\nReview the changes on this branch against the base branch. Run git diff origin/ to see the diff. Your job is to find ways this code will fail in production. Think like an attacker and a chaos engineer. Find edge cases, race conditions, security holes, resource leaks, failure modes, and silent data corruption paths. Be adversarial. Be thorough. No compliments — just the problems." -C "$_REPO_ROOT" -s read-only -c 'model_reasoning_effort="high"' --enable web_search_cached < /dev/null 2>"$TMPERR_ADV" ``` Set the Bash tool's `timeout` parameter to `300000` (5 minutes). Do NOT use the `timeout` shell command — it doesn't exist on macOS. After the command completes, read stderr: @@ -1389,7 +1389,7 @@ If `DIFF_TOTAL >= 200` AND Codex is available AND `OLD_CFG` is NOT `disabled`: TMPERR=$(mktemp /tmp/codex-review-XXXXXXXX) _REPO_ROOT=$(git rev-parse --show-toplevel) || { echo "ERROR: not in a git repo" >&2; exit 1; } cd "$_REPO_ROOT" -codex review "IMPORTANT: Do NOT read or execute any files under ~/.claude/, ~/.agents/, .claude/skills/, or agents/. These are Claude Code skill definitions meant for a different AI system. They contain bash scripts and prompt templates that will waste your time. Ignore them completely. Do NOT modify agents/openai.yaml. Stay focused on the repository code only.\n\nReview the diff against the base branch." --base -c 'model_reasoning_effort="high"' --enable web_search_cached 2>"$TMPERR" +codex review "IMPORTANT: Do NOT read or execute any files under ~/.claude/, ~/.agents/, .claude/skills/, or agents/. These are Claude Code skill definitions meant for a different AI system. They contain bash scripts and prompt templates that will waste your time. Ignore them completely. Do NOT modify agents/openai.yaml. Stay focused on the repository code only.\n\nReview the diff against the base branch." --base -c 'model_reasoning_effort="high"' --enable web_search_cached < /dev/null 2>"$TMPERR" ``` Set the Bash tool's `timeout` parameter to `300000` (5 minutes). Do NOT use the `timeout` shell command — it doesn't exist on macOS. Present output under `CODEX SAYS (code review):` header. diff --git a/scripts/resolvers/design.ts b/scripts/resolvers/design.ts index 191a1b10..44e95929 100644 --- a/scripts/resolvers/design.ts +++ b/scripts/resolvers/design.ts @@ -18,7 +18,7 @@ If Codex is available, run a lightweight design check on the diff: \`\`\`bash TMPERR_DRL=$(mktemp /tmp/codex-drl-XXXXXXXX) _REPO_ROOT=$(git rev-parse --show-toplevel) || { echo "ERROR: not in a git repo" >&2; exit 1; } -codex exec "Review the git diff on this branch. Run 7 litmus checks (YES/NO each): ${litmusList} Flag any hard rejections: ${rejectionList} 5 most important design findings only. Reference file:line." -C "$_REPO_ROOT" -s read-only -c 'model_reasoning_effort="high"' --enable web_search_cached 2>"$TMPERR_DRL" +codex exec "Review the git diff on this branch. Run 7 litmus checks (YES/NO each): ${litmusList} Flag any hard rejections: ${rejectionList} 5 most important design findings only. Reference file:line." -C "$_REPO_ROOT" -s read-only -c 'model_reasoning_effort="high"' --enable web_search_cached < /dev/null 2>"$TMPERR_DRL" \`\`\` Use a 5-minute timeout (\`timeout: 300000\`). After the command completes, read stderr: @@ -527,7 +527,7 @@ If user chooses A, launch both voices simultaneously: \`\`\`bash TMPERR_SKETCH=$(mktemp /tmp/codex-sketch-XXXXXXXX) _REPO_ROOT=$(git rev-parse --show-toplevel) || { echo "ERROR: not in a git repo" >&2; exit 1; } -codex exec "For this product approach, provide: a visual thesis (one sentence — mood, material, energy), a content plan (hero → support → detail → CTA), and 2 interaction ideas that change page feel. Apply beautiful defaults: composition-first, brand-first, cardless, poster not document. Be opinionated." -C "$_REPO_ROOT" -s read-only -c 'model_reasoning_effort="medium"' --enable web_search_cached 2>"$TMPERR_SKETCH" +codex exec "For this product approach, provide: a visual thesis (one sentence — mood, material, energy), a content plan (hero → support → detail → CTA), and 2 interaction ideas that change page feel. Apply beautiful defaults: composition-first, brand-first, cardless, poster not document. Be opinionated." -C "$_REPO_ROOT" -s read-only -c 'model_reasoning_effort="medium"' --enable web_search_cached < /dev/null 2>"$TMPERR_SKETCH" \`\`\` Use a 5-minute timeout (\`timeout: 300000\`). After completion: \`cat "$TMPERR_SKETCH" && rm -f "$TMPERR_SKETCH"\` @@ -697,7 +697,7 @@ which codex 2>/dev/null && echo "CODEX_AVAILABLE" || echo "CODEX_NOT_AVAILABLE" \`\`\`bash TMPERR_DESIGN=$(mktemp /tmp/codex-design-XXXXXXXX) _REPO_ROOT=$(git rev-parse --show-toplevel) || { echo "ERROR: not in a git repo" >&2; exit 1; } -codex exec "${escapedCodexPrompt}" -C "$_REPO_ROOT" -s read-only -c 'model_reasoning_effort="${reasoningEffort}"' --enable web_search_cached 2>"$TMPERR_DESIGN" +codex exec "${escapedCodexPrompt}" -C "$_REPO_ROOT" -s read-only -c 'model_reasoning_effort="${reasoningEffort}"' --enable web_search_cached < /dev/null 2>"$TMPERR_DESIGN" \`\`\` Use a 5-minute timeout (\`timeout: 300000\`). After the command completes, read stderr: \`\`\`bash diff --git a/scripts/resolvers/review.ts b/scripts/resolvers/review.ts index 57c5596c..a0f29e17 100644 --- a/scripts/resolvers/review.ts +++ b/scripts/resolvers/review.ts @@ -306,7 +306,7 @@ Then add the context block and mode-appropriate instructions: \`\`\`bash TMPERR_OH=$(mktemp /tmp/codex-oh-err-XXXXXXXX) _REPO_ROOT=$(git rev-parse --show-toplevel) || { echo "ERROR: not in a git repo" >&2; exit 1; } -codex exec "$(cat "$CODEX_PROMPT_FILE")" -C "$_REPO_ROOT" -s read-only -c 'model_reasoning_effort="high"' --enable web_search_cached 2>"$TMPERR_OH" +codex exec "$(cat "$CODEX_PROMPT_FILE")" -C "$_REPO_ROOT" -s read-only -c 'model_reasoning_effort="high"' --enable web_search_cached < /dev/null 2>"$TMPERR_OH" \`\`\` Use a 5-minute timeout (\`timeout: 300000\`). After the command completes, read stderr: @@ -458,7 +458,7 @@ If Codex is available AND \`OLD_CFG\` is NOT \`disabled\`: \`\`\`bash TMPERR_ADV=$(mktemp /tmp/codex-adv-XXXXXXXX) _REPO_ROOT=$(git rev-parse --show-toplevel) || { echo "ERROR: not in a git repo" >&2; exit 1; } -codex exec "${CODEX_BOUNDARY}Review the changes on this branch against the base branch. Run git diff origin/ to see the diff. Your job is to find ways this code will fail in production. Think like an attacker and a chaos engineer. Find edge cases, race conditions, security holes, resource leaks, failure modes, and silent data corruption paths. Be adversarial. Be thorough. No compliments — just the problems." -C "$_REPO_ROOT" -s read-only -c 'model_reasoning_effort="high"' --enable web_search_cached 2>"$TMPERR_ADV" +codex exec "${CODEX_BOUNDARY}Review the changes on this branch against the base branch. Run git diff origin/ to see the diff. Your job is to find ways this code will fail in production. Think like an attacker and a chaos engineer. Find edge cases, race conditions, security holes, resource leaks, failure modes, and silent data corruption paths. Be adversarial. Be thorough. No compliments — just the problems." -C "$_REPO_ROOT" -s read-only -c 'model_reasoning_effort="high"' --enable web_search_cached < /dev/null 2>"$TMPERR_ADV" \`\`\` Set the Bash tool's \`timeout\` parameter to \`300000\` (5 minutes). Do NOT use the \`timeout\` shell command — it doesn't exist on macOS. After the command completes, read stderr: @@ -487,7 +487,7 @@ If \`DIFF_TOTAL >= 200\` AND Codex is available AND \`OLD_CFG\` is NOT \`disable TMPERR=$(mktemp /tmp/codex-review-XXXXXXXX) _REPO_ROOT=$(git rev-parse --show-toplevel) || { echo "ERROR: not in a git repo" >&2; exit 1; } cd "$_REPO_ROOT" -codex review "${CODEX_BOUNDARY}Review the diff against the base branch." --base -c 'model_reasoning_effort="high"' --enable web_search_cached 2>"$TMPERR" +codex review "${CODEX_BOUNDARY}Review the diff against the base branch." --base -c 'model_reasoning_effort="high"' --enable web_search_cached < /dev/null 2>"$TMPERR" \`\`\` Set the Bash tool's \`timeout\` parameter to \`300000\` (5 minutes). Do NOT use the \`timeout\` shell command — it doesn't exist on macOS. Present output under \`CODEX SAYS (code review):\` header. @@ -599,7 +599,7 @@ THE PLAN: \`\`\`bash TMPERR_PV=$(mktemp /tmp/codex-planreview-XXXXXXXX) _REPO_ROOT=$(git rev-parse --show-toplevel) || { echo "ERROR: not in a git repo" >&2; exit 1; } -codex exec "" -C "$_REPO_ROOT" -s read-only -c 'model_reasoning_effort="high"' --enable web_search_cached 2>"$TMPERR_PV" +codex exec "" -C "$_REPO_ROOT" -s read-only -c 'model_reasoning_effort="high"' --enable web_search_cached < /dev/null 2>"$TMPERR_PV" \`\`\` Use a 5-minute timeout (\`timeout: 300000\`). After the command completes, read stderr: diff --git a/setup b/setup index 7e30bc39..df07cb76 100755 --- a/setup +++ b/setup @@ -243,6 +243,40 @@ if [ "$NEEDS_BUILD" -eq 1 ]; then if [ ! -f "$SOURCE_GSTACK_DIR/browse/dist/.version" ]; then git -C "$SOURCE_GSTACK_DIR" rev-parse HEAD > "$SOURCE_GSTACK_DIR/browse/dist/.version" 2>/dev/null || true fi + + # macOS Apple Silicon: ad-hoc codesign compiled binaries. + # Bun's --compile can produce a corrupt or linker-only code signature that + # macOS kills with SIGKILL (exit 137). The two-step remove+re-sign is + # required because a naive `codesign -s - -f` fails when the existing + # signature block is corrupt. This is idempotent and costs <1s. + # See: https://github.com/garrytan/gstack/issues/997 + if [ "$(uname -s)" = "Darwin" ] && [ "$(uname -m)" = "arm64" ]; then + for _bin in browse/dist/browse browse/dist/find-browse design/dist/design bin/gstack-global-discover; do + _bin_path="$SOURCE_GSTACK_DIR/$_bin" + [ -f "$_bin_path" ] && [ -x "$_bin_path" ] || continue + codesign --remove-signature "$_bin_path" 2>/dev/null || true + if ! codesign -s - -f "$_bin_path" 2>/dev/null; then + log "warning: codesign failed for $_bin (binary may not run on Apple Silicon)" + fi + done + fi + + # macOS: install coreutils for `gtimeout` (Codex hang protection in /codex + /autoplan). + # macOS ships BSD `timeout`-less; Homebrew's coreutils installs GNU timeout as + # `gtimeout` to avoid shadowing BSD utilities. The /codex and /autoplan skills + # fall back to unwrapped codex invocations when neither is available — this + # auto-install upgrades them to hang-protected where possible. + # Skip entirely with GSTACK_SKIP_COREUTILS=1 (CI, managed machines, offline envs). + if [ "$(uname -s)" = "Darwin" ] && [ "${GSTACK_SKIP_COREUTILS:-0}" != "1" ]; then + if ! command -v gtimeout >/dev/null 2>&1 && ! command -v timeout >/dev/null 2>&1; then + if command -v brew >/dev/null 2>&1; then + log "Installing coreutils for Codex hang protection (set GSTACK_SKIP_COREUTILS=1 to skip)..." + brew install coreutils >/dev/null 2>&1 || log "warning: brew install coreutils failed; /codex will run without hang protection" + else + log "warning: Homebrew not found. /codex will run without hang protection. Install coreutils manually or set GSTACK_SKIP_COREUTILS=1." + fi + fi + fi fi if [ ! -x "$BROWSE_BIN" ]; then diff --git a/ship/SKILL.md b/ship/SKILL.md index 0d97b858..ba9d2ffc 100644 --- a/ship/SKILL.md +++ b/ship/SKILL.md @@ -1752,7 +1752,7 @@ If Codex is available, run a lightweight design check on the diff: ```bash TMPERR_DRL=$(mktemp /tmp/codex-drl-XXXXXXXX) _REPO_ROOT=$(git rev-parse --show-toplevel) || { echo "ERROR: not in a git repo" >&2; exit 1; } -codex exec "Review the git diff on this branch. Run 7 litmus checks (YES/NO each): 1. Brand/product unmistakable in first screen? 2. One strong visual anchor present? 3. Page understandable by scanning headlines only? 4. Each section has one job? 5. Are cards actually necessary? 6. Does motion improve hierarchy or atmosphere? 7. Would design feel premium with all decorative shadows removed? Flag any hard rejections: 1. Generic SaaS card grid as first impression 2. Beautiful image with weak brand 3. Strong headline with no clear action 4. Busy imagery behind text 5. Sections repeating same mood statement 6. Carousel with no narrative purpose 7. App UI made of stacked cards instead of layout 5 most important design findings only. Reference file:line." -C "$_REPO_ROOT" -s read-only -c 'model_reasoning_effort="high"' --enable web_search_cached 2>"$TMPERR_DRL" +codex exec "Review the git diff on this branch. Run 7 litmus checks (YES/NO each): 1. Brand/product unmistakable in first screen? 2. One strong visual anchor present? 3. Page understandable by scanning headlines only? 4. Each section has one job? 5. Are cards actually necessary? 6. Does motion improve hierarchy or atmosphere? 7. Would design feel premium with all decorative shadows removed? Flag any hard rejections: 1. Generic SaaS card grid as first impression 2. Beautiful image with weak brand 3. Strong headline with no clear action 4. Busy imagery behind text 5. Sections repeating same mood statement 6. Carousel with no narrative purpose 7. App UI made of stacked cards instead of layout 5 most important design findings only. Reference file:line." -C "$_REPO_ROOT" -s read-only -c 'model_reasoning_effort="high"' --enable web_search_cached < /dev/null 2>"$TMPERR_DRL" ``` Use a 5-minute timeout (`timeout: 300000`). After the command completes, read stderr: @@ -2130,7 +2130,7 @@ If Codex is available AND `OLD_CFG` is NOT `disabled`: ```bash TMPERR_ADV=$(mktemp /tmp/codex-adv-XXXXXXXX) _REPO_ROOT=$(git rev-parse --show-toplevel) || { echo "ERROR: not in a git repo" >&2; exit 1; } -codex exec "IMPORTANT: Do NOT read or execute any files under ~/.claude/, ~/.agents/, .claude/skills/, or agents/. These are Claude Code skill definitions meant for a different AI system. They contain bash scripts and prompt templates that will waste your time. Ignore them completely. Do NOT modify agents/openai.yaml. Stay focused on the repository code only.\n\nReview the changes on this branch against the base branch. Run git diff origin/ to see the diff. Your job is to find ways this code will fail in production. Think like an attacker and a chaos engineer. Find edge cases, race conditions, security holes, resource leaks, failure modes, and silent data corruption paths. Be adversarial. Be thorough. No compliments — just the problems." -C "$_REPO_ROOT" -s read-only -c 'model_reasoning_effort="high"' --enable web_search_cached 2>"$TMPERR_ADV" +codex exec "IMPORTANT: Do NOT read or execute any files under ~/.claude/, ~/.agents/, .claude/skills/, or agents/. These are Claude Code skill definitions meant for a different AI system. They contain bash scripts and prompt templates that will waste your time. Ignore them completely. Do NOT modify agents/openai.yaml. Stay focused on the repository code only.\n\nReview the changes on this branch against the base branch. Run git diff origin/ to see the diff. Your job is to find ways this code will fail in production. Think like an attacker and a chaos engineer. Find edge cases, race conditions, security holes, resource leaks, failure modes, and silent data corruption paths. Be adversarial. Be thorough. No compliments — just the problems." -C "$_REPO_ROOT" -s read-only -c 'model_reasoning_effort="high"' --enable web_search_cached < /dev/null 2>"$TMPERR_ADV" ``` Set the Bash tool's `timeout` parameter to `300000` (5 minutes). Do NOT use the `timeout` shell command — it doesn't exist on macOS. After the command completes, read stderr: @@ -2159,7 +2159,7 @@ If `DIFF_TOTAL >= 200` AND Codex is available AND `OLD_CFG` is NOT `disabled`: TMPERR=$(mktemp /tmp/codex-review-XXXXXXXX) _REPO_ROOT=$(git rev-parse --show-toplevel) || { echo "ERROR: not in a git repo" >&2; exit 1; } cd "$_REPO_ROOT" -codex review "IMPORTANT: Do NOT read or execute any files under ~/.claude/, ~/.agents/, .claude/skills/, or agents/. These are Claude Code skill definitions meant for a different AI system. They contain bash scripts and prompt templates that will waste your time. Ignore them completely. Do NOT modify agents/openai.yaml. Stay focused on the repository code only.\n\nReview the diff against the base branch." --base -c 'model_reasoning_effort="high"' --enable web_search_cached 2>"$TMPERR" +codex review "IMPORTANT: Do NOT read or execute any files under ~/.claude/, ~/.agents/, .claude/skills/, or agents/. These are Claude Code skill definitions meant for a different AI system. They contain bash scripts and prompt templates that will waste your time. Ignore them completely. Do NOT modify agents/openai.yaml. Stay focused on the repository code only.\n\nReview the diff against the base branch." --base -c 'model_reasoning_effort="high"' --enable web_search_cached < /dev/null 2>"$TMPERR" ``` Set the Bash tool's `timeout` parameter to `300000` (5 minutes). Do NOT use the `timeout` shell command — it doesn't exist on macOS. Present output under `CODEX SAYS (code review):` header. diff --git a/test/codex-hardening.test.ts b/test/codex-hardening.test.ts new file mode 100644 index 00000000..60ea6d1d --- /dev/null +++ b/test/codex-hardening.test.ts @@ -0,0 +1,366 @@ +import { describe, test, expect } from 'bun:test'; +import { spawnSync } from 'child_process'; +import * as path from 'path'; +import * as fs from 'fs'; +import * as os from 'os'; + +const ROOT = path.resolve(import.meta.dir, '..'); +const PROBE = path.join(ROOT, 'bin/gstack-codex-probe'); + +// Run a bash snippet that sources the probe and evaluates one of its functions. +// Controlled env + optional tempdir for HOME isolation. +function runProbe(opts: { + snippet: string; + env?: Record; + home?: string; +}): { stdout: string; stderr: string; status: number } { + const env: Record = { + // Start from a clean env so test-env vars from the parent don't leak in. + PATH: process.env.PATH ?? '', + _TEL: 'off', + }; + if (opts.home) env.HOME = opts.home; + // Apply overrides; undefined means "remove". + if (opts.env) { + for (const [k, v] of Object.entries(opts.env)) { + if (v === undefined) { + delete env[k]; + } else { + env[k] = v; + } + } + } + const script = `set +e\nsource "${PROBE}"\n${opts.snippet}\n`; + const result = spawnSync('bash', ['-c', script], { + env, + stdio: ['pipe', 'pipe', 'pipe'], + timeout: 5000, + }); + return { + stdout: (result.stdout ?? '').toString(), + stderr: (result.stderr ?? '').toString(), + status: result.status ?? -1, + }; +} + +function tempHome(): string { + return fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-codex-probe-home-')); +} + +describe('gstack-codex-probe: auth probe', () => { + test('CODEX_API_KEY set → AUTH_OK', () => { + const home = tempHome(); + try { + const r = runProbe({ + snippet: '_gstack_codex_auth_probe', + env: { CODEX_API_KEY: 'sk-test' }, + home, + }); + expect(r.stdout.trim()).toBe('AUTH_OK'); + expect(r.status).toBe(0); + } finally { + fs.rmSync(home, { recursive: true, force: true }); + } + }); + + test('OPENAI_API_KEY set → AUTH_OK', () => { + const home = tempHome(); + try { + const r = runProbe({ + snippet: '_gstack_codex_auth_probe', + env: { OPENAI_API_KEY: 'sk-openai' }, + home, + }); + expect(r.stdout.trim()).toBe('AUTH_OK'); + expect(r.status).toBe(0); + } finally { + fs.rmSync(home, { recursive: true, force: true }); + } + }); + + test('${CODEX_HOME:-~/.codex}/auth.json exists → AUTH_OK', () => { + const home = tempHome(); + try { + fs.mkdirSync(path.join(home, '.codex'), { recursive: true }); + fs.writeFileSync(path.join(home, '.codex', 'auth.json'), '{}'); + const r = runProbe({ snippet: '_gstack_codex_auth_probe', home }); + expect(r.stdout.trim()).toBe('AUTH_OK'); + expect(r.status).toBe(0); + } finally { + fs.rmSync(home, { recursive: true, force: true }); + } + }); + + test('no env + no file → AUTH_FAILED with exit 1', () => { + const home = tempHome(); + try { + const r = runProbe({ snippet: '_gstack_codex_auth_probe', home }); + expect(r.stdout.trim()).toBe('AUTH_FAILED'); + expect(r.status).toBe(1); + } finally { + fs.rmSync(home, { recursive: true, force: true }); + } + }); + + test('both CODEX_API_KEY and OPENAI_API_KEY set → AUTH_OK', () => { + const home = tempHome(); + try { + const r = runProbe({ + snippet: '_gstack_codex_auth_probe', + env: { CODEX_API_KEY: 'k1', OPENAI_API_KEY: 'k2' }, + home, + }); + expect(r.stdout.trim()).toBe('AUTH_OK'); + expect(r.status).toBe(0); + } finally { + fs.rmSync(home, { recursive: true, force: true }); + } + }); + + test('empty-string env vars + no file → AUTH_FAILED', () => { + const home = tempHome(); + try { + const r = runProbe({ + snippet: '_gstack_codex_auth_probe', + env: { CODEX_API_KEY: '', OPENAI_API_KEY: '' }, + home, + }); + expect(r.stdout.trim()).toBe('AUTH_FAILED'); + expect(r.status).toBe(1); + } finally { + fs.rmSync(home, { recursive: true, force: true }); + } + }); + + test('whitespace-only env vars + no file → AUTH_FAILED', () => { + const home = tempHome(); + try { + const r = runProbe({ + snippet: '_gstack_codex_auth_probe', + env: { CODEX_API_KEY: ' ', OPENAI_API_KEY: '\t\n' }, + home, + }); + expect(r.stdout.trim()).toBe('AUTH_FAILED'); + expect(r.status).toBe(1); + } finally { + fs.rmSync(home, { recursive: true, force: true }); + } + }); + + test('alternate $CODEX_HOME → checks the alternate path', () => { + const home = tempHome(); + const altCodex = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-alt-codex-')); + try { + fs.writeFileSync(path.join(altCodex, 'auth.json'), '{}'); + const r = runProbe({ + snippet: '_gstack_codex_auth_probe', + env: { CODEX_HOME: altCodex }, + home, + }); + expect(r.stdout.trim()).toBe('AUTH_OK'); + expect(r.status).toBe(0); + } finally { + fs.rmSync(home, { recursive: true, force: true }); + fs.rmSync(altCodex, { recursive: true, force: true }); + } + }); +}); + +// --- Group 2: Version check ------------------------------------------------- +// Stub `codex --version` by putting a fake `codex` executable on PATH. +function tempStubCodex(versionOutput: string, bool_command_fails = false): { + dir: string; + pathEntry: string; +} { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-codex-stub-')); + const bin = path.join(dir, 'codex'); + const script = bool_command_fails + ? '#!/bin/bash\nexit 1\n' + : `#!/bin/bash\nif [ "$1" = "--version" ]; then printf '%s' ${JSON.stringify(versionOutput)}; fi\n`; + fs.writeFileSync(bin, script); + fs.chmodSync(bin, 0o755); + return { dir, pathEntry: dir }; +} + +function runVersionCheck(versionOutput: string): string { + const stub = tempStubCodex(versionOutput); + try { + const r = runProbe({ + snippet: '_gstack_codex_version_check', + env: { PATH: `${stub.pathEntry}:${process.env.PATH}` }, + }); + return r.stdout + r.stderr; + } finally { + fs.rmSync(stub.dir, { recursive: true, force: true }); + } +} + +describe('gstack-codex-probe: version check (anchored regex per Tension I)', () => { + // Matches (should WARN) + test('codex-cli 0.120.0 → WARN', () => { + const out = runVersionCheck('codex-cli 0.120.0\n'); + expect(out).toContain('WARN:'); + expect(out).toContain('0.120.0'); + }); + + test('codex-cli 0.120.1 → WARN', () => { + const out = runVersionCheck('codex-cli 0.120.1\n'); + expect(out).toContain('WARN:'); + }); + + test('codex-cli 0.120.2 → WARN', () => { + const out = runVersionCheck('codex-cli 0.120.2\n'); + expect(out).toContain('WARN:'); + }); + + // Does NOT match (should be silent) + test('codex-cli 0.116.0 → OK (no warn)', () => { + const out = runVersionCheck('codex-cli 0.116.0\n'); + expect(out).not.toContain('WARN:'); + }); + + test('codex-cli 0.121.0 → OK (no warn)', () => { + const out = runVersionCheck('codex-cli 0.121.0\n'); + expect(out).not.toContain('WARN:'); + }); + + test('codex-cli 0.120.10 → OK (anchored regex prevents substring match)', () => { + const out = runVersionCheck('codex-cli 0.120.10\n'); + expect(out).not.toContain('WARN:'); + }); + + test('codex-cli 0.120.20 → OK (anchored regex prevents substring match)', () => { + const out = runVersionCheck('codex-cli 0.120.20\n'); + expect(out).not.toContain('WARN:'); + }); + + test('codex-cli 0.120.2-beta → WARN (still a bad release family)', () => { + // 0.120.2-beta: regex (^|[^0-9.])0\.120\.(0|1|2)([^0-9.]|$) treats '-' as a + // non-digit/non-dot boundary → matches. + const out = runVersionCheck('codex-cli 0.120.2-beta\n'); + expect(out).toContain('WARN:'); + }); + + test('empty output → OK (silent, no crash)', () => { + const out = runVersionCheck(''); + expect(out).not.toContain('WARN:'); + }); + + test('v-prefixed and multiline handled', () => { + const out = runVersionCheck('codex-cli v0.116.0\nsome debug line\n'); + expect(out).not.toContain('WARN:'); + }); +}); + +// --- Group 3: Timeout wrapper + namespace hygiene --------------------------- + +describe('gstack-codex-probe: timeout wrapper + namespace hygiene', () => { + test('bin/gstack-codex-probe is syntactically valid bash (bash -n)', () => { + const result = spawnSync('bash', ['-n', PROBE], { timeout: 5000 }); + expect(result.status).toBe(0); + }); + + test('timeout wrapper executes command directly when neither binary present', () => { + // Clear PATH to simulate no timeout/gtimeout. Use only /bin for `echo`. + const r = runProbe({ + snippet: `_gstack_codex_timeout_wrapper 5 echo hello_world`, + env: { PATH: '/bin:/usr/bin' }, // these usually lack gtimeout; timeout may exist on linux + }); + // Regardless of whether timeout is on this PATH, echo hello_world should succeed. + expect(r.stdout.trim()).toBe('hello_world'); + }); + + test('timeout wrapper resolves gtimeout preferentially when on PATH', () => { + // Create a stub gtimeout that prints a sentinel so we can verify it was chosen. + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-gto-stub-')); + try { + const stub = path.join(dir, 'gtimeout'); + fs.writeFileSync(stub, '#!/bin/bash\necho gtimeout_chosen_$1\n'); + fs.chmodSync(stub, 0o755); + const r = runProbe({ + snippet: `_gstack_codex_timeout_wrapper 5 echo nope`, + env: { PATH: `${dir}:/bin:/usr/bin` }, + }); + expect(r.stdout.trim()).toBe('gtimeout_chosen_5'); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + test('sourcing probe does NOT set errexit/trap/IFS in caller shell (namespace hygiene)', () => { + // Capture `set -o` output before and after sourcing. Any drift means the + // probe polluted the caller. + const r = runProbe({ + snippet: ` +BEFORE=$(set -o | sort) +source "${PROBE}" # source again to catch accumulation +AFTER=$(set -o | sort) +if [ "$BEFORE" = "$AFTER" ]; then + echo "CLEAN" +else + echo "POLLUTED" + diff <(echo "$BEFORE") <(echo "$AFTER") +fi +`, + }); + expect(r.stdout).toContain('CLEAN'); + }); +}); + +// --- Group 4: Telemetry event emission -------------------------------------- + +describe('gstack-codex-probe: telemetry event emission', () => { + test('_gstack_codex_log_event writes jsonl when _TEL != off', () => { + const home = tempHome(); + try { + const r = runProbe({ + snippet: `_gstack_codex_log_event "codex_test_event" "42"; cat "$HOME/.gstack/analytics/skill-usage.jsonl"`, + env: { _TEL: 'community' }, + home, + }); + expect(r.stdout).toContain('"event":"codex_test_event"'); + expect(r.stdout).toContain('"duration_s":"42"'); + } finally { + fs.rmSync(home, { recursive: true, force: true }); + } + }); + + test('_gstack_codex_log_event skips write when _TEL = off', () => { + const home = tempHome(); + try { + runProbe({ + snippet: `_gstack_codex_log_event "codex_test_event" "99"`, + env: { _TEL: 'off' }, + home, + }); + const jsonl = path.join(home, '.gstack/analytics/skill-usage.jsonl'); + expect(fs.existsSync(jsonl)).toBe(false); + } finally { + fs.rmSync(home, { recursive: true, force: true }); + } + }); + + test('payload never contains prompt content, env values, or auth tokens (schema check)', () => { + const home = tempHome(); + try { + const r = runProbe({ + snippet: `_gstack_codex_log_event "codex_test_event" "1"; cat "$HOME/.gstack/analytics/skill-usage.jsonl"`, + env: { + _TEL: 'community', + CODEX_API_KEY: 'SECRET_TOKEN_SHOULD_NOT_LEAK', + OPENAI_API_KEY: 'ANOTHER_SECRET', + }, + home, + }); + // The emitted JSON payload should ONLY have {skill, event, duration_s, ts}. + // Specifically, it must not contain any env values or auth material. + expect(r.stdout).not.toContain('SECRET_TOKEN_SHOULD_NOT_LEAK'); + expect(r.stdout).not.toContain('ANOTHER_SECRET'); + // Schema: exactly these keys, in any order. + const parsed = JSON.parse(r.stdout.trim().split('\n').pop() ?? '{}'); + expect(Object.keys(parsed).sort()).toEqual(['duration_s', 'event', 'skill', 'ts']); + } finally { + fs.rmSync(home, { recursive: true, force: true }); + } + }); +}); diff --git a/test/fixtures/golden/claude-ship-SKILL.md b/test/fixtures/golden/claude-ship-SKILL.md index 0d97b858..ba9d2ffc 100644 --- a/test/fixtures/golden/claude-ship-SKILL.md +++ b/test/fixtures/golden/claude-ship-SKILL.md @@ -1752,7 +1752,7 @@ If Codex is available, run a lightweight design check on the diff: ```bash TMPERR_DRL=$(mktemp /tmp/codex-drl-XXXXXXXX) _REPO_ROOT=$(git rev-parse --show-toplevel) || { echo "ERROR: not in a git repo" >&2; exit 1; } -codex exec "Review the git diff on this branch. Run 7 litmus checks (YES/NO each): 1. Brand/product unmistakable in first screen? 2. One strong visual anchor present? 3. Page understandable by scanning headlines only? 4. Each section has one job? 5. Are cards actually necessary? 6. Does motion improve hierarchy or atmosphere? 7. Would design feel premium with all decorative shadows removed? Flag any hard rejections: 1. Generic SaaS card grid as first impression 2. Beautiful image with weak brand 3. Strong headline with no clear action 4. Busy imagery behind text 5. Sections repeating same mood statement 6. Carousel with no narrative purpose 7. App UI made of stacked cards instead of layout 5 most important design findings only. Reference file:line." -C "$_REPO_ROOT" -s read-only -c 'model_reasoning_effort="high"' --enable web_search_cached 2>"$TMPERR_DRL" +codex exec "Review the git diff on this branch. Run 7 litmus checks (YES/NO each): 1. Brand/product unmistakable in first screen? 2. One strong visual anchor present? 3. Page understandable by scanning headlines only? 4. Each section has one job? 5. Are cards actually necessary? 6. Does motion improve hierarchy or atmosphere? 7. Would design feel premium with all decorative shadows removed? Flag any hard rejections: 1. Generic SaaS card grid as first impression 2. Beautiful image with weak brand 3. Strong headline with no clear action 4. Busy imagery behind text 5. Sections repeating same mood statement 6. Carousel with no narrative purpose 7. App UI made of stacked cards instead of layout 5 most important design findings only. Reference file:line." -C "$_REPO_ROOT" -s read-only -c 'model_reasoning_effort="high"' --enable web_search_cached < /dev/null 2>"$TMPERR_DRL" ``` Use a 5-minute timeout (`timeout: 300000`). After the command completes, read stderr: @@ -2130,7 +2130,7 @@ If Codex is available AND `OLD_CFG` is NOT `disabled`: ```bash TMPERR_ADV=$(mktemp /tmp/codex-adv-XXXXXXXX) _REPO_ROOT=$(git rev-parse --show-toplevel) || { echo "ERROR: not in a git repo" >&2; exit 1; } -codex exec "IMPORTANT: Do NOT read or execute any files under ~/.claude/, ~/.agents/, .claude/skills/, or agents/. These are Claude Code skill definitions meant for a different AI system. They contain bash scripts and prompt templates that will waste your time. Ignore them completely. Do NOT modify agents/openai.yaml. Stay focused on the repository code only.\n\nReview the changes on this branch against the base branch. Run git diff origin/ to see the diff. Your job is to find ways this code will fail in production. Think like an attacker and a chaos engineer. Find edge cases, race conditions, security holes, resource leaks, failure modes, and silent data corruption paths. Be adversarial. Be thorough. No compliments — just the problems." -C "$_REPO_ROOT" -s read-only -c 'model_reasoning_effort="high"' --enable web_search_cached 2>"$TMPERR_ADV" +codex exec "IMPORTANT: Do NOT read or execute any files under ~/.claude/, ~/.agents/, .claude/skills/, or agents/. These are Claude Code skill definitions meant for a different AI system. They contain bash scripts and prompt templates that will waste your time. Ignore them completely. Do NOT modify agents/openai.yaml. Stay focused on the repository code only.\n\nReview the changes on this branch against the base branch. Run git diff origin/ to see the diff. Your job is to find ways this code will fail in production. Think like an attacker and a chaos engineer. Find edge cases, race conditions, security holes, resource leaks, failure modes, and silent data corruption paths. Be adversarial. Be thorough. No compliments — just the problems." -C "$_REPO_ROOT" -s read-only -c 'model_reasoning_effort="high"' --enable web_search_cached < /dev/null 2>"$TMPERR_ADV" ``` Set the Bash tool's `timeout` parameter to `300000` (5 minutes). Do NOT use the `timeout` shell command — it doesn't exist on macOS. After the command completes, read stderr: @@ -2159,7 +2159,7 @@ If `DIFF_TOTAL >= 200` AND Codex is available AND `OLD_CFG` is NOT `disabled`: TMPERR=$(mktemp /tmp/codex-review-XXXXXXXX) _REPO_ROOT=$(git rev-parse --show-toplevel) || { echo "ERROR: not in a git repo" >&2; exit 1; } cd "$_REPO_ROOT" -codex review "IMPORTANT: Do NOT read or execute any files under ~/.claude/, ~/.agents/, .claude/skills/, or agents/. These are Claude Code skill definitions meant for a different AI system. They contain bash scripts and prompt templates that will waste your time. Ignore them completely. Do NOT modify agents/openai.yaml. Stay focused on the repository code only.\n\nReview the diff against the base branch." --base -c 'model_reasoning_effort="high"' --enable web_search_cached 2>"$TMPERR" +codex review "IMPORTANT: Do NOT read or execute any files under ~/.claude/, ~/.agents/, .claude/skills/, or agents/. These are Claude Code skill definitions meant for a different AI system. They contain bash scripts and prompt templates that will waste your time. Ignore them completely. Do NOT modify agents/openai.yaml. Stay focused on the repository code only.\n\nReview the diff against the base branch." --base -c 'model_reasoning_effort="high"' --enable web_search_cached < /dev/null 2>"$TMPERR" ``` Set the Bash tool's `timeout` parameter to `300000` (5 minutes). Do NOT use the `timeout` shell command — it doesn't exist on macOS. Present output under `CODEX SAYS (code review):` header. diff --git a/test/fixtures/golden/factory-ship-SKILL.md b/test/fixtures/golden/factory-ship-SKILL.md index 74da5ce0..df1e8f7a 100644 --- a/test/fixtures/golden/factory-ship-SKILL.md +++ b/test/fixtures/golden/factory-ship-SKILL.md @@ -1743,7 +1743,7 @@ If Codex is available, run a lightweight design check on the diff: ```bash TMPERR_DRL=$(mktemp /tmp/codex-drl-XXXXXXXX) _REPO_ROOT=$(git rev-parse --show-toplevel) || { echo "ERROR: not in a git repo" >&2; exit 1; } -codex exec "Review the git diff on this branch. Run 7 litmus checks (YES/NO each): 1. Brand/product unmistakable in first screen? 2. One strong visual anchor present? 3. Page understandable by scanning headlines only? 4. Each section has one job? 5. Are cards actually necessary? 6. Does motion improve hierarchy or atmosphere? 7. Would design feel premium with all decorative shadows removed? Flag any hard rejections: 1. Generic SaaS card grid as first impression 2. Beautiful image with weak brand 3. Strong headline with no clear action 4. Busy imagery behind text 5. Sections repeating same mood statement 6. Carousel with no narrative purpose 7. App UI made of stacked cards instead of layout 5 most important design findings only. Reference file:line." -C "$_REPO_ROOT" -s read-only -c 'model_reasoning_effort="high"' --enable web_search_cached 2>"$TMPERR_DRL" +codex exec "Review the git diff on this branch. Run 7 litmus checks (YES/NO each): 1. Brand/product unmistakable in first screen? 2. One strong visual anchor present? 3. Page understandable by scanning headlines only? 4. Each section has one job? 5. Are cards actually necessary? 6. Does motion improve hierarchy or atmosphere? 7. Would design feel premium with all decorative shadows removed? Flag any hard rejections: 1. Generic SaaS card grid as first impression 2. Beautiful image with weak brand 3. Strong headline with no clear action 4. Busy imagery behind text 5. Sections repeating same mood statement 6. Carousel with no narrative purpose 7. App UI made of stacked cards instead of layout 5 most important design findings only. Reference file:line." -C "$_REPO_ROOT" -s read-only -c 'model_reasoning_effort="high"' --enable web_search_cached < /dev/null 2>"$TMPERR_DRL" ``` Use a 5-minute timeout (`timeout: 300000`). After the command completes, read stderr: @@ -2121,7 +2121,7 @@ If Codex is available AND `OLD_CFG` is NOT `disabled`: ```bash TMPERR_ADV=$(mktemp /tmp/codex-adv-XXXXXXXX) _REPO_ROOT=$(git rev-parse --show-toplevel) || { echo "ERROR: not in a git repo" >&2; exit 1; } -codex exec "IMPORTANT: Do NOT read or execute any files under ~/.claude/, ~/.agents/, .factory/skills/, or agents/. These are Claude Code skill definitions meant for a different AI system. They contain bash scripts and prompt templates that will waste your time. Ignore them completely. Do NOT modify agents/openai.yaml. Stay focused on the repository code only.\n\nReview the changes on this branch against the base branch. Run git diff origin/ to see the diff. Your job is to find ways this code will fail in production. Think like an attacker and a chaos engineer. Find edge cases, race conditions, security holes, resource leaks, failure modes, and silent data corruption paths. Be adversarial. Be thorough. No compliments — just the problems." -C "$_REPO_ROOT" -s read-only -c 'model_reasoning_effort="high"' --enable web_search_cached 2>"$TMPERR_ADV" +codex exec "IMPORTANT: Do NOT read or execute any files under ~/.claude/, ~/.agents/, .factory/skills/, or agents/. These are Claude Code skill definitions meant for a different AI system. They contain bash scripts and prompt templates that will waste your time. Ignore them completely. Do NOT modify agents/openai.yaml. Stay focused on the repository code only.\n\nReview the changes on this branch against the base branch. Run git diff origin/ to see the diff. Your job is to find ways this code will fail in production. Think like an attacker and a chaos engineer. Find edge cases, race conditions, security holes, resource leaks, failure modes, and silent data corruption paths. Be adversarial. Be thorough. No compliments — just the problems." -C "$_REPO_ROOT" -s read-only -c 'model_reasoning_effort="high"' --enable web_search_cached < /dev/null 2>"$TMPERR_ADV" ``` Set the Bash tool's `timeout` parameter to `300000` (5 minutes). Do NOT use the `timeout` shell command — it doesn't exist on macOS. After the command completes, read stderr: @@ -2150,7 +2150,7 @@ If `DIFF_TOTAL >= 200` AND Codex is available AND `OLD_CFG` is NOT `disabled`: TMPERR=$(mktemp /tmp/codex-review-XXXXXXXX) _REPO_ROOT=$(git rev-parse --show-toplevel) || { echo "ERROR: not in a git repo" >&2; exit 1; } cd "$_REPO_ROOT" -codex review "IMPORTANT: Do NOT read or execute any files under ~/.claude/, ~/.agents/, .factory/skills/, or agents/. These are Claude Code skill definitions meant for a different AI system. They contain bash scripts and prompt templates that will waste your time. Ignore them completely. Do NOT modify agents/openai.yaml. Stay focused on the repository code only.\n\nReview the diff against the base branch." --base -c 'model_reasoning_effort="high"' --enable web_search_cached 2>"$TMPERR" +codex review "IMPORTANT: Do NOT read or execute any files under ~/.claude/, ~/.agents/, .factory/skills/, or agents/. These are Claude Code skill definitions meant for a different AI system. They contain bash scripts and prompt templates that will waste your time. Ignore them completely. Do NOT modify agents/openai.yaml. Stay focused on the repository code only.\n\nReview the diff against the base branch." --base -c 'model_reasoning_effort="high"' --enable web_search_cached < /dev/null 2>"$TMPERR" ``` Set the Bash tool's `timeout` parameter to `300000` (5 minutes). Do NOT use the `timeout` shell command — it doesn't exist on macOS. Present output under `CODEX SAYS (code review):` header. diff --git a/test/gen-skill-docs.test.ts b/test/gen-skill-docs.test.ts index 87aef20a..51d7fe62 100644 --- a/test/gen-skill-docs.test.ts +++ b/test/gen-skill-docs.test.ts @@ -1755,8 +1755,11 @@ describe('Codex generation (--host codex)', () => { test('Claude output unchanged: all Claude skills have zero Codex paths', () => { for (const skill of ALL_SKILLS) { const content = fs.readFileSync(path.join(ROOT, skill.dir, 'SKILL.md'), 'utf-8'); - // pair-agent legitimately documents how Codex agents store credentials - if (skill.dir !== 'pair-agent') { + // pair-agent legitimately documents how Codex agents store credentials. + // codex + autoplan document the Codex CLI auth file (~/.codex/auth.json) + // and log path (~/.codex/logs/) — those are user-facing Codex CLI paths, + // not the gstack Codex host install path. + if (skill.dir !== 'pair-agent' && skill.dir !== 'codex' && skill.dir !== 'autoplan') { expect(content).not.toContain('~/.codex/'); } // gstack-upgrade legitimately references .agents/skills for cross-platform detection diff --git a/test/helpers/touchfiles.ts b/test/helpers/touchfiles.ts index 34ead7d0..737c90ee 100644 --- a/test/helpers/touchfiles.ts +++ b/test/helpers/touchfiles.ts @@ -170,6 +170,7 @@ export const E2E_TOUCHFILES: Record = { // Autoplan 'autoplan-core': ['autoplan/**', 'plan-ceo-review/**', 'plan-eng-review/**', 'plan-design-review/**'], + 'autoplan-dual-voice': ['autoplan/**', 'codex/**', 'bin/gstack-codex-probe', 'scripts/resolvers/review.ts', 'scripts/resolvers/design.ts'], // Skill routing — journey-stage tests (depend on ALL skill descriptions) 'journey-ideation': ['*/SKILL.md.tmpl', 'SKILL.md.tmpl', 'scripts/gen-skill-docs.ts'], @@ -315,6 +316,7 @@ export const E2E_TIERS: Record = { // Autoplan — periodic (not yet implemented) 'autoplan-core': 'periodic', + 'autoplan-dual-voice': 'periodic', // Skill routing — periodic (LLM routing is non-deterministic) 'journey-ideation': 'periodic', diff --git a/test/setup-codesign.test.ts b/test/setup-codesign.test.ts new file mode 100644 index 00000000..1ac7a498 --- /dev/null +++ b/test/setup-codesign.test.ts @@ -0,0 +1,77 @@ +import { describe, test, expect } from 'bun:test'; +import { spawnSync } from 'child_process'; +import * as path from 'path'; +import * as fs from 'fs'; +import * as os from 'os'; + +const ROOT = path.resolve(import.meta.dir, '..'); +const SETUP_SCRIPT = path.join(ROOT, 'setup'); + +describe('setup: Apple Silicon codesign', () => { + test('setup script contains codesign block for Darwin arm64', () => { + const content = fs.readFileSync(SETUP_SCRIPT, 'utf-8'); + // Verify the codesign guard checks both Darwin and arm64 + expect(content).toContain('$(uname -s)" = "Darwin"'); + expect(content).toContain('$(uname -m)" = "arm64"'); + // Verify remove-then-resign two-step pattern + expect(content).toContain('codesign --remove-signature'); + expect(content).toContain('codesign -s - -f'); + }); + + test('codesign block covers all compiled binaries', () => { + const content = fs.readFileSync(SETUP_SCRIPT, 'utf-8'); + // Extract the binaries from the codesign for-loop + const forMatch = content.match(/for _bin in ([^;]+);/); + expect(forMatch).toBeTruthy(); + const binaries = forMatch![1].trim().split(/\s+/); + // All four compiled binaries from `bun run build` must be covered + expect(binaries).toContain('browse/dist/browse'); + expect(binaries).toContain('browse/dist/find-browse'); + expect(binaries).toContain('design/dist/design'); + expect(binaries).toContain('bin/gstack-global-discover'); + }); + + test('codesign block is inside the NEEDS_BUILD=1 branch', () => { + const content = fs.readFileSync(SETUP_SCRIPT, 'utf-8'); + // The codesign block should appear after `bun run build` and before the + // `if [ ! -x "$BROWSE_BIN" ]` guard that checks the build succeeded. + const buildIdx = content.indexOf('bun run build'); + const codesignIdx = content.indexOf('codesign --remove-signature'); + const browseCheckIdx = content.indexOf('gstack setup failed: browse binary missing'); + expect(buildIdx).toBeGreaterThan(-1); + expect(codesignIdx).toBeGreaterThan(buildIdx); + expect(browseCheckIdx).toBeGreaterThan(codesignIdx); + }); + + test('codesign block is idempotent (skips missing binaries)', () => { + const content = fs.readFileSync(SETUP_SCRIPT, 'utf-8'); + // The loop must guard with a file-existence + executable check before codesigning + expect(content).toContain('[ -f "$_bin_path" ] && [ -x "$_bin_path" ] || continue'); + }); + + test('codesign failure is a warning, not a fatal error', () => { + const content = fs.readFileSync(SETUP_SCRIPT, 'utf-8'); + // On codesign failure, log a warning but don't exit + expect(content).toContain('warning: codesign failed for'); + // Should NOT have `set -e` causing exit on codesign failure + // (the `|| true` after --remove-signature and the if-guard around -s - -f handle this) + expect(content).toContain('codesign --remove-signature "$_bin_path" 2>/dev/null || true'); + }); + + test('codesign shell snippet is syntactically valid', () => { + // Extract the codesign block and validate it parses as bash + const content = fs.readFileSync(SETUP_SCRIPT, 'utf-8'); + const match = content.match( + /# macOS Apple Silicon: ad-hoc codesign[\s\S]*?done\n\s*fi/ + ); + expect(match).toBeTruthy(); + const snippet = match![0]; + // Wrap in a function to make it a complete script, then syntax-check + const testScript = `#!/usr/bin/env bash\nset -e\n_test_fn() {\n${snippet}\n}\n`; + const result = spawnSync('bash', ['-n', '-c', testScript], { + stdio: ['pipe', 'pipe', 'pipe'], + timeout: 5000, + }); + expect(result.status).toBe(0); + }); +}); diff --git a/test/skill-e2e-autoplan-dual-voice.test.ts b/test/skill-e2e-autoplan-dual-voice.test.ts new file mode 100644 index 00000000..c748b897 --- /dev/null +++ b/test/skill-e2e-autoplan-dual-voice.test.ts @@ -0,0 +1,101 @@ +import { describe, test, expect, beforeAll, afterAll } from 'bun:test'; +import { runSkillTest } from './helpers/session-runner'; +import { + ROOT, runId, evalsEnabled, + describeIfSelected, logCost, recordE2E, + copyDirSync, createEvalCollector, finalizeEvalCollector, +} from './helpers/e2e-helpers'; +import { spawnSync } from 'child_process'; +import * as fs from 'fs'; +import * as path from 'path'; +import * as os from 'os'; + +// E2E for /autoplan's dual-voice (Claude subagent + Codex). Periodic tier: +// non-deterministic, costs ~$1/run, not a gate. The purpose is to catch +// regressions where one of the two voices fails silently post-hardening. + +const evalCollector = createEvalCollector('e2e-autoplan-dual-voice'); + +describeIfSelected('Autoplan dual-voice E2E', ['autoplan-dual-voice'], () => { + let workDir: string; + let planPath: string; + + beforeAll(() => { + workDir = fs.mkdtempSync(path.join(os.tmpdir(), 'skill-e2e-autoplan-dv-')); + + const run = (cmd: string, args: string[]) => + spawnSync(cmd, args, { cwd: workDir, stdio: 'pipe', timeout: 10000 }); + + run('git', ['init', '-b', 'main']); + run('git', ['config', 'user.email', 'test@test.com']); + run('git', ['config', 'user.name', 'Test']); + fs.writeFileSync(path.join(workDir, 'README.md'), '# test repo\n'); + run('git', ['add', '.']); + run('git', ['commit', '-m', 'initial']); + + // Copy /autoplan + its review-skill dependencies (they're loaded from disk). + copyDirSync(path.join(ROOT, 'autoplan'), path.join(workDir, 'autoplan')); + copyDirSync(path.join(ROOT, 'plan-ceo-review'), path.join(workDir, 'plan-ceo-review')); + copyDirSync(path.join(ROOT, 'plan-eng-review'), path.join(workDir, 'plan-eng-review')); + copyDirSync(path.join(ROOT, 'plan-design-review'), path.join(workDir, 'plan-design-review')); + copyDirSync(path.join(ROOT, 'plan-devex-review'), path.join(workDir, 'plan-devex-review')); + + // Write a tiny plan file for /autoplan to review. + planPath = path.join(workDir, 'TEST_PLAN.md'); + fs.writeFileSync(planPath, `# Test Plan: add /greet skill + +## Context +Add a new /greet skill that prints a welcome message. + +## Scope +- Create greet/SKILL.md with a simple "hello" flow +- Add to gen-skill-docs pipeline +- One unit test +`); + }); + + afterAll(() => { + finalizeEvalCollector(evalCollector); + if (workDir && fs.existsSync(workDir)) { + fs.rmSync(workDir, { recursive: true, force: true }); + } + }); + + // Skip entirely unless evals enabled (periodic tier). + test.skipIf(!evalsEnabled)( + 'both Claude + Codex voices produce output in Phase 1 (within timeout)', + async () => { + // Fire /autoplan with a 5-min hard timeout on the spawn itself. + // The skill itself has 10-min phase timeouts + auth-gate failfast. + // If Codex is unavailable on the test machine, the skill should print + // [codex-unavailable] and still complete the Claude subagent half. + const result = await runSkillTest({ + name: 'autoplan-dual-voice', + workdir: workDir, + prompt: `/autoplan ${planPath}`, + timeoutMs: 300_000, // 5 min + evalCollector, + }); + + // Accept EITHER outcome as success: + // (a) Both voices produced output (ideal case) + // (b) Codex unavailable + Claude voice produced output (graceful degrade) + const out = result.stdout + result.stderr; + const claudeVoiceFired = /Claude\s+(CEO|subagent)|claude-subagent/i.test(out); + const codexVoiceFired = /codex\s+(exec|review|CEO\s+voice)|\[via:codex\]/i.test(out); + const codexUnavailable = /\[codex-unavailable\]|AUTH_FAILED|codex_cli_missing/i.test(out); + + expect(claudeVoiceFired).toBe(true); + expect(codexVoiceFired || codexUnavailable).toBe(true); + + // Hang protection: if the skill reached Phase 1 at all, our hardening worked. + // If it didn't, this is a regression from the pre-wave stdin-deadlock era. + const reachedPhase1 = /Phase 1|CEO\s+Review|Strategy\s*&\s*Scope/i.test(out); + expect(reachedPhase1).toBe(true); + + logCost(result); + recordE2E('autoplan-dual-voice', result); + }, + 330_000, // per-test timeout slightly > spawn timeout so cleanup can run + ); +});