diff --git a/bin/gstack-codex-probe b/bin/gstack-codex-probe index 8fa33ea0e..6b09ee254 100755 --- a/bin/gstack-codex-probe +++ b/bin/gstack-codex-probe @@ -5,6 +5,8 @@ # Functions (all prefixed with _gstack_codex_ for namespace hygiene): # _gstack_codex_auth_probe — multi-signal auth check (env + file) # _gstack_codex_select_model — resolve + print the runtime model (#2914) +# _gstack_codex_sandbox_mode — read-only, or full access only for GSTACK_CODEX_NO_SANDBOX=1 +# _gstack_codex_sandbox_preflight — free `codex sandbox` check before paid calls (Linux) # _gstack_codex_model_probe — round-trip probe of the selected model (#2477) # _gstack_codex_version_check — warn on known-bad Codex CLI versions # _gstack_codex_timeout_wrapper — gtimeout -> timeout -> bash-native watchdog @@ -65,6 +67,43 @@ _gstack_codex_select_model() { fi _GSTACK_CODEX_SEL_KIND="$_kind" echo "CODEX_MODEL: $_GSTACK_CODEX_SEL ($_kind; source: $_GSTACK_CODEX_SEL_SRC)" >&2 + _gstack_codex_sandbox_mode +} + +# --- Sandbox (B1) ----------------------------------------------------------- + +_gstack_codex_sandbox_mode() { + # Every gstack Codex command passes -s / sandbox_mode from this variable, so + # one place decides it. Codex has no read-only mode without its sandbox, so + # the escape hatch is full access: honored only for exactly 1, only from the + # shell environment (never a dotenv file or config), and announced each use. + if [ "${GSTACK_CODEX_NO_SANDBOX:-}" = "1" ]; then + _GSTACK_CODEX_SANDBOX="danger-full-access" + echo "WARNING: GSTACK_CODEX_NO_SANDBOX=1: Codex runs this review without a sandbox and can read and write anything your user can. Use it only inside a container you trust." >&2 + else + _GSTACK_CODEX_SANDBOX="read-only" + fi +} + +_gstack_codex_sandbox_preflight() { + # Free check before the first paid call: `codex sandbox` starts the same + # bubblewrap sandbox with no model. Only a recognized sandbox failure blocks + # (exit 3, printed as missing coverage); success, a timeout, or an older CLI + # without the subcommand defers to the post-run classifier. Linux only, and + # skipped when the user turned the sandbox off. Fixtures: + # test/fixtures/codex-sandbox/. + [ "${GSTACK_CODEX_NO_SANDBOX:-}" = "1" ] && return 0 + [ "$(uname -s 2>/dev/null)" = "Linux" ] || return 0 + local _out _code _line + _out=$(_gstack_codex_timeout_wrapper 10 codex sandbox -c 'sandbox_mode="read-only"' true &1) + _code=$? + { [ "$_code" -eq 0 ] || [ "$_code" -eq 124 ]; } && return 0 + _line=$(printf '%s\n' "$_out" | grep -m1 -iE '^[[:space:]]*bwrap: |bubblewrap is unavailable|landlock.{0,60}(fail|error|not supported|unsupported)|seccomp.{0,60}(fail|error)|user namespaces?.{0,80}(not (allowed|permitted|supported)|denied|disabled)' | sed 's/^[[:space:]]*//' | cut -c1-240) + [ -n "$_line" ] || return 0 + echo "CODEX_SANDBOX: unavailable" + echo "Codex outside review unavailable: Codex's sandbox could not start here ($_line). No review ran; this is missing coverage, not a pass. Fix: enable unprivileged user namespaces for this container, or set GSTACK_CODEX_NO_SANDBOX=1." >&2 + _gstack_codex_log_event "codex_sandbox_unavailable" 2>/dev/null || true + return 3 } # --- Model round-trip probe (#2477) ------------------------------------------ @@ -89,9 +128,10 @@ _gstack_codex_model_probe() { # ENOENT, non-executable binary, missing vendor payload). Deterministic, # so fail-open is wrong: retrying never helps. Never cached — a reinstall # fixes it and must be picked up on the very next probe (#2742). - # MODEL_PROBE_INCONCLUSIVE (exit 0) — timeout/transient; FAIL-OPEN so a - # slow network never wedges codex mode (the per-invocation Error - # Handling entry still covers a later 400). Never cached. + # MODEL_PROBE_INCONCLUSIVE (exit 0) — timeout/transient; the review still + # runs so a slow network never wedges codex mode, but readiness is + # unverified (_GSTACK_CODEX_PROBE_STATE=inconclusive, CODEX_MODE: + # unverified) and the review's own classification decides. Never cached. # # Only call this AFTER _gstack_codex_auth_probe passes — probing without # auth just measures the auth failure again. @@ -190,7 +230,10 @@ _gstack_codex_model_probe() { fi # Timeout (124) or transient failure: fail-open with a warning. The probe # exists to catch the deterministic model 400, not to gate on network luck. - echo "MODEL_PROBE_INCONCLUSIVE (exit $_code) — proceeding; if invocations fail with a model 400, see the codex skill's Error Handling entry." + # Callers report CODEX_MODE: unverified, never ready; the exit stays 0 so + # skills rendered before this marker keep running the review. + _GSTACK_CODEX_PROBE_STATE="inconclusive" + echo "MODEL_PROBE_INCONCLUSIVE (exit $_code) — CODEX_MODE: unverified; proceeding, and the review's own result is still checked. If invocations fail with a model 400, see the codex skill's Error Handling entry." return 0 } @@ -288,7 +331,7 @@ _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, codex_model_unusable. + # codex_version_warning, codex_model_unusable, codex_sandbox_unavailable. # Payload schema: {skill, event, duration_s, ts}. NEVER includes prompt # content, env var values, or auth tokens. local _event="$1" diff --git a/codex/SKILL.md.tmpl b/codex/SKILL.md.tmpl index c12529692..69b730ced 100644 --- a/codex/SKILL.md.tmpl +++ b/codex/SKILL.md.tmpl @@ -83,7 +83,7 @@ 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" -else +elif _gstack_codex_sandbox_preflight; then # free; Linux only _gstack_codex_model_probe # ~10s round trip on first run, cached 1h (#2477) fi _gstack_codex_version_check # warns if known-bad, non-blocking @@ -101,8 +101,14 @@ follow the "Model not supported (HTTP 400)" recovery steps in `## Error Handling` below. Running the modes anyway just burns four invocations on the same 400 (#2477). -`MODEL_PROBE_INCONCLUSIVE` is non-blocking (timeout/transient network): pass -the warning through and continue. +If the output contains `CODEX_SANDBOX: unavailable`, stop: Codex's sandbox cannot +start here, so every command it runs would fail and its review would read nothing. +Relay the `Codex outside review unavailable: ...` line verbatim, including its fix. +No paid call was made. + +`MODEL_PROBE_INCONCLUSIVE` is non-blocking (timeout/transient network): report +`CODEX_MODE: unverified`, pass the warning through and continue; the mode's own +validator still decides the result. 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). @@ -299,6 +305,9 @@ If token count is not available, display: `Tokens: unknown` them to a model the account can use. 3. If Codex printed `[notice.model_migrations]`, use that replacement model. Never present this as a model stall or a PASS — it is a fail-closed gate result. +- **`VERDICT: unavailable`:** the shared validator found the run did not execute (for + example `Codex's sandbox could not start here`). Relay its line and fix verbatim; it is + missing coverage, never a PASS. Details: `docs/troubleshooting.md` in the gstack checkout. - **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. @@ -307,7 +316,8 @@ If token count is not available, display: `Tokens: unknown` ## Important Rules -- **Never modify files.** This skill is read-only. Codex runs in read-only sandbox mode. +- **Never modify files.** This skill is read-only. Codex runs in read-only sandbox mode + (full access only when the user exported `GSTACK_CODEX_NO_SANDBOX=1`; it warns on every use). - **Present output verbatim.** Do not truncate, summarize, or editorialize Codex's output before showing it. Show it in full inside the CODEX SAYS block. - **Add synthesis after, not instead of.** Any Claude commentary comes after the full output. diff --git a/codex/sections/challenge-mode.md.tmpl b/codex/sections/challenge-mode.md.tmpl index 921cfc83d..5de4e26ad 100644 --- a/codex/sections/challenge-mode.md.tmpl +++ b/codex/sections/challenge-mode.md.tmpl @@ -33,9 +33,10 @@ fi # 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). [ -n "${TMPERR:-}" ] || TMPERR=$(mktemp "$TMP_ROOT/codex-err-XXXXXX") || { echo "ERROR: mktemp failed in TMP_ROOT=$TMP_ROOT; not running codex without its temp file" >&2; exit 1; } +TMPRESP=$(mktemp "$TMP_ROOT/codex-resp-XXXXXX") || { echo "ERROR: mktemp failed in TMP_ROOT=$TMP_ROOT; not running codex without its temp file" >&2; exit 1; } source ~/.claude/skills/gstack/bin/gstack-codex-probe || exit 1 _gstack_codex_select_model exec || exit 1 -_gstack_codex_timeout_wrapper 540 codex exec "" -C "$_REPO_ROOT" -s read-only {{CODEX_MODEL_CONFIG_FLAG}} -c 'model_reasoning_effort="high"' {{CODEX_WEB_SEARCH_FLAG}} --json < /dev/null 2>"$TMPERR" | PYTHONUNBUFFERED=1 "$PYTHON_CMD" -u -c " +_gstack_codex_timeout_wrapper 540 codex exec "" -C "$_REPO_ROOT" -s "${_GSTACK_CODEX_SANDBOX:?}" {{CODEX_MODEL_CONFIG_FLAG}} -c 'model_reasoning_effort="high"' {{CODEX_WEB_SEARCH_FLAG}} --json -o "$TMPRESP" < /dev/null 2>"$TMPERR" | tee "$TMPRESP.events" | PYTHONUNBUFFERED=1 "$PYTHON_CMD" -u -c " import sys, json turn_completed_count = 0 turn_failed = False @@ -92,8 +93,14 @@ 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 +bun ~/.claude/skills/gstack/lib/outside-review-result.ts --label 'Codex challenge' --exit "$_CODEX_EXIT" --stderr "$TMPERR" --events "$TMPRESP.events" execution "$TMPRESP" +rm -f "$TMPRESP" "$TMPRESP.events" ``` +`VERDICT: unavailable` means the challenge did not run (its line names why, such as a +sandbox that could not start): say so and present no findings from it. Otherwise +present the output below. + This parses codex's JSONL events to extract reasoning traces, tool calls, and the final response. The `[codex thinking]` lines show what codex reasoned through before its answer. diff --git a/codex/sections/consult-mode.md.tmpl b/codex/sections/consult-mode.md.tmpl index 4f1f4e1df..2ff341209 100644 --- a/codex/sections/consult-mode.md.tmpl +++ b/codex/sections/consult-mode.md.tmpl @@ -78,7 +78,7 @@ fi source ~/.claude/skills/gstack/bin/gstack-codex-probe || exit 1 _gstack_codex_select_model exec || exit 1 # Fix 1: wrap with timeout (gtimeout/timeout fallback chain via probe helper) -_gstack_codex_timeout_wrapper 540 codex exec "" -C "$_REPO_ROOT" -s read-only {{CODEX_MODEL_CONFIG_FLAG}} -c 'model_reasoning_effort="medium"' {{CODEX_WEB_SEARCH_FLAG}} --json < /dev/null 2>"$TMPERR" | PYTHONUNBUFFERED=1 "$PYTHON_CMD" -u -c " +_gstack_codex_timeout_wrapper 540 codex exec "" -C "$_REPO_ROOT" -s "${_GSTACK_CODEX_SANDBOX:?}" {{CODEX_MODEL_CONFIG_FLAG}} -c 'model_reasoning_effort="medium"' {{CODEX_WEB_SEARCH_FLAG}} --json -o "$TMPRESP" < /dev/null 2>"$TMPERR" | tee "$TMPRESP.events" | PYTHONUNBUFFERED=1 "$PYTHON_CMD" -u -c " import sys, json turn_completed_count = 0 turn_failed = False @@ -133,6 +133,7 @@ elif [ "$_CODEX_EXIT" != "0" ]; then head -20 "$TMPERR" 2>/dev/null | sed 's/^/ /' || true _gstack_codex_log_event "codex_nonzero_exit" "consult:$_CODEX_EXIT" fi +bun ~/.claude/skills/gstack/lib/outside-review-result.ts --label 'Codex consult' --exit "$_CODEX_EXIT" --stderr "$TMPERR" --events "$TMPRESP.events" execution "$TMPRESP" ``` **Session-cost reality (#2387, measured):** every `codex exec` call — resumed @@ -155,7 +156,7 @@ cd "$_REPO_ROOT" || exit 1 source ~/.claude/skills/gstack/bin/gstack-codex-probe || exit 1 _gstack_codex_select_model exec || exit 1 # Fix 1: wrap with timeout (gtimeout/timeout fallback chain via probe helper) -_gstack_codex_timeout_wrapper 540 codex exec resume "" -c 'sandbox_mode="read-only"' {{CODEX_MODEL_CONFIG_FLAG}} -c 'model_reasoning_effort="medium"' {{CODEX_WEB_SEARCH_FLAG}} --json < /dev/null 2>"$TMPERR" | PYTHONUNBUFFERED=1 "$PYTHON_CMD" -u -c " +_gstack_codex_timeout_wrapper 540 codex exec resume "" -c "sandbox_mode=\"${_GSTACK_CODEX_SANDBOX:?}\"" {{CODEX_MODEL_CONFIG_FLAG}} -c 'model_reasoning_effort="medium"' {{CODEX_WEB_SEARCH_FLAG}} --json -o "$TMPRESP" < /dev/null 2>"$TMPERR" | tee "$TMPRESP.events" | PYTHONUNBUFFERED=1 "$PYTHON_CMD" -u -c " " # Fix 1: same hang detection pattern as new-session block @@ -171,8 +172,13 @@ elif [ "$_CODEX_EXIT" != "0" ]; then head -20 "$TMPERR" 2>/dev/null | sed 's/^/ /' || true _gstack_codex_log_event "codex_nonzero_exit" "consult-resume:$_CODEX_EXIT" fi +bun ~/.claude/skills/gstack/lib/outside-review-result.ts --label 'Codex consult' --exit "$_CODEX_EXIT" --stderr "$TMPERR" --events "$TMPRESP.events" execution "$TMPRESP" ``` +`VERDICT: unavailable` from either block means Codex did not answer from a working +environment (its line names why, such as a sandbox that could not start): relay that +line and do not present the output as Codex's answer. + 5. Capture session ID from the streamed output. The parser prints `SESSION_ID:` from the `thread.started` event. Save it for follow-ups: ```bash diff --git a/codex/sections/review-mode.md.tmpl b/codex/sections/review-mode.md.tmpl index 4083c1958..607383399 100644 --- a/codex/sections/review-mode.md.tmpl +++ b/codex/sections/review-mode.md.tmpl @@ -25,6 +25,7 @@ instructions get their own path (below). 1. Create temp files for output capture: ```bash TMPERR=$(mktemp "$TMP_ROOT/codex-err-XXXXXX") || { echo "ERROR: mktemp failed in TMP_ROOT=$TMP_ROOT; not running codex without its temp file" >&2; exit 1; } +TMPOUT=$(mktemp "$TMP_ROOT/codex-out-XXXXXX") || { echo "ERROR: mktemp failed in TMP_ROOT=$TMP_ROOT; not running codex without its temp file" >&2; exit 1; } ``` 2. Run the review. No prompt argument — scope comes from `--base` (or `--commit ` @@ -32,10 +33,11 @@ when reviewing a single commit, or `--uncommitted` for the working tree). Use only one command path below. Remember its printed start token as CODEX_REVIEW_START before the review reads or receives the diff. Capture a new token only before a genuine rerun, never just to log fixes. -**Sandbox is pinned read-only via config override.** Top-level `codex review` has no +**Sandbox is pinned via config override.** Top-level `codex review` has no `-s`/`--sandbox` flag (verified on 0.147.0: `codex review --help` lists none), so the -read-only sandbox is set with `-c 'sandbox_mode="read-only"'` — the same form the -consult resume path uses. Without it the call inherits the user's +sandbox is set with `-c "sandbox_mode=..."` from `_GSTACK_CODEX_SANDBOX`, which +`_gstack_codex_select_model` sets to `read-only` (full access only for +`GSTACK_CODEX_NO_SANDBOX=1`, with a warning). Without it the call inherits the user's `~/.codex/config.toml` default, which on a trusted project can be WRITE access — contradicting this skill's read-only contract (#2496, #2524): @@ -48,8 +50,9 @@ _gstack_codex_select_model review || exit 1 # The 330s wrapper sits BELOW the 360s Bash gate so the wrapper fires FIRST # and a stall surfaces as a diagnosable exit 124 with an explicit message, # never as a silent harness kill that downstream reads as "no findings". -_gstack_codex_timeout_wrapper 330 codex review --base -c 'sandbox_mode="read-only"' {{CODEX_REVIEW_MODEL_CONFIG_FLAG}} -c 'model_reasoning_effort="high"' {{CODEX_WEB_SEARCH_FLAG}} < /dev/null 2>"$TMPERR" +_gstack_codex_timeout_wrapper 330 codex review --base -c "sandbox_mode=\"${_GSTACK_CODEX_SANDBOX:?}\"" {{CODEX_REVIEW_MODEL_CONFIG_FLAG}} -c 'model_reasoning_effort="high"' {{CODEX_WEB_SEARCH_FLAG}} < /dev/null >"$TMPOUT" 2>"$TMPERR" _CODEX_EXIT=$? +cat "$TMPOUT" 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)" @@ -62,6 +65,7 @@ elif [ "$_CODEX_EXIT" != "0" ]; then head -20 "$TMPERR" 2>/dev/null | sed 's/^/ /' || true _gstack_codex_log_event "codex_nonzero_exit" "review:$_CODEX_EXIT" fi +bun ~/.claude/skills/gstack/lib/outside-review-result.ts --label 'Codex review' --exit "$_CODEX_EXIT" --stderr "$TMPERR" structured "$TMPOUT" ``` If the user passed `--xhigh`, use `"xhigh"` instead of `"high"`. @@ -92,14 +96,16 @@ _PROMPT_FILE=$(mktemp "$TMP_ROOT/codex-prompt-XXXXXX") || { echo "ERROR: mktemp git diff "...HEAD" 2>/dev/null printf '\nDIFF_END\n' } > "$_PROMPT_FILE" -_gstack_codex_timeout_wrapper 330 codex exec -s read-only "$(cat "$_PROMPT_FILE")" {{CODEX_MODEL_CONFIG_FLAG}} -c 'model_reasoning_effort="high"' {{CODEX_WEB_SEARCH_FLAG}} < /dev/null 2>"$TMPERR" +_gstack_codex_timeout_wrapper 330 codex exec -s "${_GSTACK_CODEX_SANDBOX:?}" "$(cat "$_PROMPT_FILE")" {{CODEX_MODEL_CONFIG_FLAG}} -c 'model_reasoning_effort="high"' {{CODEX_WEB_SEARCH_FLAG}} --json -o "$TMPOUT" < /dev/null >"$TMPOUT.events" 2>"$TMPERR" _CODEX_EXIT=$? rm -f "$_PROMPT_FILE" +cat "$TMPOUT" 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." fi +bun ~/.claude/skills/gstack/lib/outside-review-result.ts --label 'Codex review' --exit "$_CODEX_EXIT" --stderr "$TMPERR" --events "$TMPOUT.events" structured "$TMPOUT" ``` When you take this path, say so in the output header — `CODEX SAYS (code review — custom @@ -121,8 +127,9 @@ instead of the harness killing the call silently. grep "tokens used" "$TMPERR" 2>/dev/null || echo "tokens: unknown" ``` -4. Determine the gate verdict: **PASS**, **FAIL**, or **UNVERIFIED**. -**The gate FAILS CLOSED**: a run that did not complete is a FAIL, and output the +4. Determine the gate verdict: **PASS**, **FAIL**, or **UNVERIFIED**. The command +above ends with the shared validator's `VERDICT:` and `FINDINGS:` lines; the checks +below are what it computes. **The gate FAILS CLOSED**: a run that did not complete is a FAIL, and output the gate cannot check is UNVERIFIED, never a PASS. Work through these checks IN ORDER; the first match wins: @@ -131,7 +138,11 @@ match wins: verified result). Expired auth, a bad flag, a timeout, or a model-entitlement 400 all land here instead of masquerading as a clean pass. 2. The captured review output is empty or whitespace-only → **GATE: FAIL** - (fail-closed: empty output — nothing was reviewed). + (fail-closed: empty output — nothing was reviewed). Any other + `VERDICT: unavailable` is also **GATE: FAIL**: Codex's sandbox could not + start, every command it ran failed, or it said it could not read the diff. + Relay the validator's `unavailable:` line; a "no findings" written after + that is not a review. 3. The output contains `[P0]` or `[P1]` (or codex's native unbracketed `P0:` / `P1:` severity labels) → **GATE: FAIL** (N critical findings). Codex's own review rubric treats P0 as blocking; this gate does too. @@ -145,7 +156,9 @@ match wins: 5. Severity tags are present and none is P0/P1 (only P2/P3 advisory) → **GATE: PASS**. - There is no default branch: PASS is only reachable through check 5. For FAIL + Check 3 is `VERDICT: findings`; check 4 is `VERDICT: unverified`, or + `VERDICT: clean` with `FINDINGS: none`; check 5 is `VERDICT: clean` with + `FINDINGS: P2` or `P3`. There is no default branch: PASS is only reachable through check 5. For FAIL via checks 1 or 2, say explicitly that the review did not complete. For UNVERIFIED, tell the user to read Codex's verdict above and decide; it is not a blocker by itself and not a pass. @@ -213,12 +226,13 @@ Substitute: TIMESTAMP (ISO 8601), STATUS ("clean" if PASS, "issues_found" if FAI "unverified" if UNVERIFIED), GATE ("pass", "fail" or "unverified" — fail-closed verdicts log as "fail"), findings (count of [P0]–[P3] markers; 0 for fail-closed and UNVERIFIED runs), -findings_fixed (count of findings that were addressed/fixed before shipping). +findings_fixed (count of findings that were addressed/fixed before shipping). If the +`WARNING: GSTACK_CODEX_NO_SANDBOX=1` line printed, add `"sandbox":"danger-full-access"` to the record. CODEX_REVIEW_START is the original token from the command path that ran. COMPLETED is true only when the review completed with coverage of the branch diff and current working-tree changes; timeout, failure, refusal, or missing coverage is false. A limited `--commit`/`--uncommitted` review, or a committed-only custom prompt that omitted dirty/untracked source, does not establish whole-branch coverage: log completed false and explain the limitation. CONVERGED is true only for a completed pass with zero edits. Fixes stay stale until a genuine rerun reads the updated diff with a new start token. These evidence fields do not change the gate verdict above. 8. Clean up temp files: ```bash -rm -f "$TMPERR" +rm -f "$TMPERR" "$TMPOUT" "$TMPOUT.events" ``` --- diff --git a/lib/outside-review-result.ts b/lib/outside-review-result.ts index 4d2b6ff1d..f5b180848 100644 --- a/lib/outside-review-result.ts +++ b/lib/outside-review-result.ts @@ -2,8 +2,9 @@ * Review-specific completion evidence, separate from provider transport success. * * classifyOutsideReview() is the INV-1 contract: it takes the review text, the - * provider's stderr and exit status and returns three separate answers: did - * the review execute, what is the highest finding severity, and the verdict. validateOutsideReview() is the + * provider's stderr and exit status (and, for `codex exec --json`, the event + * stream) and returns three separate answers: did the review execute, what is + * the highest finding severity, and the verdict. validateOutsideReview() is the * older text-only shape, kept for direct importers and stale rendered skills. */ import { GATE_OUTCOMES, gateOutcomeLine, type GateReason } from './gate-outcomes'; @@ -17,6 +18,8 @@ export interface OutsideReviewInput { gate: OutsideGate; stderr?: string; exit?: number; + /** JSONL from `codex exec --json`: command events are positive execution evidence. */ + events?: string; } export interface OutsideReviewClassification { @@ -31,18 +34,57 @@ export interface OutsideReviewClassification { const REFUSAL = /\b(?:(?:I (?:cannot|can't|won't|will not|am unable to)|I'm unable to)\s+(?:review|analy[sz]e|evaluate|assess|inspect|access|complete|perform|provide|assist|help|proceed)|unable to (?:review|analy[sz]e)|I must (?:decline|refuse))\b/i; +/** + * Sandbox setup failures as Codex and bubblewrap print them on stderr (fixtures: + * test/fixtures/codex-sandbox/). Read from stderr and failed command output + * only: a review that merely discusses bwrap or namespaces must still pass, and + * a healthy run warns "could not find bubblewrap on PATH" while working. + */ +const SANDBOX_FAILURE = /^\s*bwrap: \S.*$|^.*\bbubblewrap is unavailable\b.*$|^.*\blandlock\b.{0,60}\b(?:fail\w*|error|not supported|unsupported)\b.*$|^.*\bseccomp\b.{0,60}\b(?:fail\w*|error)\b.*$|^.*\buser namespaces?\b.{0,80}\b(?:not (?:allowed|permitted|supported)|denied|disabled)\b.*$/im; +/** Exact phrases a reviewer uses when it could not execute; the last fallback, used only without positive evidence. */ +const EXECUTION_FAILURE_PHRASE = /\b(?:commands? (?:could not|couldn't|cannot|can't) (?:be )?run|(?:could not|couldn't|was unable to|am unable to|unable to) (?:run (?:any )?(?:shell )?commands|execute (?:any )?commands|inspect the diff|read the diff|access the diff)|the diff could not be (?:read|inspected|accessed)|every (?:shell )?(?:command|invocation) failed)\b/i; +/** `codex review` transcript on stderr: a command that ran prints " succeeded in Nms:". */ +const TRANSCRIPT_SUCCESS = /^\s*succeeded in \d+(?:\.\d+)?m?s:?\s*$/m; + +function commandEvidence(events: string): { attempted: number; succeeded: number; failedOutput: string } { + let attempted = 0, succeeded = 0, failedOutput = ''; + for (const line of events.split(/\r?\n/)) { + if (!line.trim().startsWith('{')) continue; + let event: any; + try { event = JSON.parse(line); } catch { continue; } + const item = event?.item; + if (event?.type !== 'item.completed' || item?.type !== 'command_execution') continue; + attempted++; + if (item.status === 'completed' && item.exit_code === 0) succeeded++; + else failedOutput += `${typeof item.aggregated_output === 'string' ? item.aggregated_output : ''}\n`; + } + return { attempted, succeeded, failedOutput }; +} + function stderrHead(stderr: string): string | undefined { return stderr.split(/\r?\n/).map(line => line.trim()).find(Boolean)?.slice(0, 240); } function execution(input: OutsideReviewInput): OutsideReviewClassification['execution'] { + const stderr = input.stderr ?? ''; const exit = input.exit ?? 0; + const sandbox = (output: string) => output.match(SANDBOX_FAILURE)?.[0].trim().slice(0, 240); if (exit !== 0) { - const head = stderrHead(input.stderr ?? ''); - const detail = head ? `exit ${exit}: ${head}` : `exit ${exit}`; - return { state: 'unavailable', reason: exit === 124 ? 'timeout' : 'execution_failed', detail }; + const detail = sandbox(stderr); + if (detail) return { state: 'unavailable', reason: 'sandbox_unavailable', detail }; + const head = stderrHead(stderr); + return { state: 'unavailable', reason: exit === 124 ? 'timeout' : 'execution_failed', detail: head ? `exit ${exit}: ${head}` : `exit ${exit}` }; + } + const commands = commandEvidence(input.events ?? ''); + const executed = commands.succeeded > 0 || TRANSCRIPT_SUCCESS.test(stderr); + if (!executed) { + const detail = sandbox(stderr) ?? sandbox(commands.failedOutput); + if (detail) return { state: 'unavailable', reason: 'sandbox_unavailable', detail }; + if (commands.attempted > 0) return { state: 'unavailable', reason: 'commands_failed', detail: `all ${commands.attempted} commands failed` }; } if (!input.text.trim()) return { state: 'unavailable', reason: 'empty_response' }; + const phrase = executed ? undefined : input.text.match(EXECUTION_FAILURE_PHRASE)?.[0]; + if (phrase) return { state: 'unavailable', reason: 'commands_failed', detail: `the review says "${phrase}"` }; if (REFUSAL.test(input.text)) return { state: 'unavailable', reason: 'review_refused' }; return { state: 'ran' }; } @@ -99,7 +141,7 @@ export function validateOutsideReview(text: string, gate: OutsideGate): { comple const VERDICT_EXIT: Record = { clean: 0, findings: 3, unverified: 4, unavailable: 1 }; const GATES = ['review', 'structured', 'spec', 'execution']; const USAGE = `Usage: outside-review-result.ts - outside-review-result.ts --verdict [--stderr ] [--exit ] [--label ] + outside-review-result.ts --verdict [--stderr ] [--exit ] [--events ] [--label ] Gates: ${GATES.join('|')}. Two-argument form exits 0 completed, 1 unavailable, 2 usage. Verdict form prints VERDICT: clean|findings|unverified|unavailable and exits 0, 3, 4 or 1.`; @@ -116,7 +158,7 @@ if (import.meta.main) { for (let i = 0; i < args.length; i++) { const arg = args[i]!; if (arg === '--verdict') verdictMode = true; - else if (['--stderr', '--exit', '--label'].includes(arg) && i + 1 < args.length) { flags[arg] = args[++i]!; verdictMode ||= arg !== '--label'; } + else if (['--stderr', '--exit', '--events', '--label'].includes(arg) && i + 1 < args.length) { flags[arg] = args[++i]!; verdictMode ||= arg !== '--label'; } else if (arg.startsWith('--')) { console.error(USAGE); process.exit(2); } else positional.push(arg); } @@ -135,7 +177,7 @@ if (import.meta.main) { process.exit(0); } const checked = classifyOutsideReview({ text, gate: gate as OutsideGate, exit, - stderr: await readOptional(flags['--stderr']) }); + stderr: await readOptional(flags['--stderr']), events: await readOptional(flags['--events']) }); console.log(`VERDICT: ${checked.verdict}`); console.log(`FINDINGS: ${checked.findings.highest ?? 'none'}`); if (checked.reason) { diff --git a/scripts/resolvers/constants.ts b/scripts/resolvers/constants.ts index 47915e043..7a16e689c 100644 --- a/scripts/resolvers/constants.ts +++ b/scripts/resolvers/constants.ts @@ -110,7 +110,8 @@ On any error: continue — ${feature} is informational, not a gate.`; * 5. sets ONE canonical mode var and echoes `CODEX_MODE: ` so the agent * gates later blocks on the echoed value. * - * Mode values: `disabled` (config off) | `not_installed` | `not_authed` | `ready`. + * Mode values: `disabled` (config off) | `not_installed` | `not_authed` | + * `broken_install` | `sandbox_unavailable` | `model_unusable` | `unverified` | `ready`. * The path is host-rewritten at gen-skill-docs time (pathRewrites), so the * literal `~/.claude/skills/gstack` is correct here and becomes `$GSTACK_ROOT` * etc. for non-Claude hosts. @@ -144,14 +145,19 @@ elif ! command -v codex >/dev/null 2>&1; then elif ! _gstack_codex_auth_probe >/dev/null 2>&1; then ${m}="not_authed"; _gstack_codex_log_event "codex_auth_failed" 2>/dev/null || true else - # Capture the probe's code: 2 means the CLI cannot execute at all, which is a - # different problem (and a different fix) from a model the account can't use. - _gstack_codex_model_probe; _CODEX_MP=$?${opts.nativeReview ? ` + # The free sandbox check runs before the paid model probe. Probe code 2 means + # the CLI cannot execute at all, a different fix from an unusable model. + _CODEX_MP=0; _gstack_codex_sandbox_preflight || _CODEX_MP=3 + [ "$_CODEX_MP" -ne 0 ] || { _gstack_codex_model_probe; _CODEX_MP=$?; }${opts.nativeReview ? ` [ "$_CODEX_MP" -ne 0 ] || { _gstack_codex_model_probe review; _CODEX_MP=$?; }` : ''} - if [ "$_CODEX_MP" -eq 2 ]; then + if [ "$_CODEX_MP" -eq 3 ]; then + ${m}="sandbox_unavailable" + elif [ "$_CODEX_MP" -eq 2 ]; then ${m}="broken_install" elif [ "$_CODEX_MP" -ne 0 ]; then ${m}="model_unusable" + elif [ "\${_GSTACK_CODEX_PROBE_STATE:-}" = inconclusive ]; then + ${m}="unverified" else ${m}="ready"; _gstack_codex_version_check 2>/dev/null || true fi @@ -165,8 +171,9 @@ Branch on the echoed \`CODEX_MODE\`: - **\`under_codex\`** — stale artifact selected its own harness. Print: "Codex outside review unavailable: harness mismatch; no outside process started. Missing coverage. Repair: setup --host codex." Skip the outside invocation and follow the workflow's native-review instructions below. Conflicting inherited harness markers are not grounds to guess another provider. - **\`not_authed\`** — installed but no credentials. Print: "Codex not authenticated; outside coverage unavailable. Run \`codex login\` or set \`$CODEX_API_KEY\`." ${nativeRoute} - **\`broken_install\`** — the CLI is on PATH but cannot execute (spawn ENOENT, non-executable binary, missing vendor payload). Print: "Codex is installed but its binary cannot run — Codex passes skipped. Reinstall: \`npm install -g @openai/codex\`." Relay the probe's HINT lines. ${nativeRoute} -- **\`model_unusable\`** — the selected model (see \`CODEX_MODEL:\`) is invalid or unavailable to the account (#2477: HTTP 400 on every call). Relay the probe's HINT lines and the fix (\`GSTACK_CODEX_MODEL=\` or config.toml \`model\`); never substitute a model. ${nativeRoute} The ~10s round trip is cached for 1h; timeouts fail open to \`ready\`. -- **\`ready\`** — run the Codex pass below.`; +- **\`model_unusable\`** — the selected model (see \`CODEX_MODEL:\`) is invalid or unavailable to the account (#2477: HTTP 400 on every call). Relay the probe's HINT lines and the fix (\`GSTACK_CODEX_MODEL=\` or config.toml \`model\`); never substitute a model. ${nativeRoute} The ~10s round trip is cached for 1h. +- **\`sandbox_unavailable\`** — Codex's sandbox cannot start here (containers without user namespaces); the probe printed the reason and fix. No paid call ran; outside coverage is unavailable. ${nativeRoute} +- **\`ready\`** or **\`unverified\`** — run the Codex pass below. \`unverified\` means the model check timed out; say so, and let the pass's own verdict decide.`; } /** diff --git a/scripts/resolvers/outside-voice-steps.ts b/scripts/resolvers/outside-voice-steps.ts index 1290c02ad..3bc3dcd08 100644 --- a/scripts/resolvers/outside-voice-steps.ts +++ b/scripts/resolvers/outside-voice-steps.ts @@ -141,7 +141,7 @@ If the subagent fails or times out, record native coverage as incomplete. Contin } function adversarialOutsideChallenge(ctx: TemplateContext, isShip: boolean): string { - return `### ${outsideVoiceFor(ctx).label} adversarial challenge (runs whenever \`CODEX_MODE: ready\`) + return `### ${outsideVoiceFor(ctx).label} adversarial challenge (runs whenever \`CODEX_MODE\` is \`ready\` or \`unverified\`) If \`CODEX_MODE\` is \`ready\`: @@ -255,7 +255,7 @@ ${outsideVoicePreflight(ctx, { disabledBehavior: 'codex-only', nativeReview: tru \`ready\` runs them; every other mode skips them with the printed reason. The ${outsideVoiceFor(ctx).nativeLabel} adversarial subagent always runs. -**User override:** If the user explicitly requested "full review", "structured review", or "P1 gate", also run the ${outsideVoiceFor(ctx).label} structured review regardless of diff size (still requires \`CODEX_MODE: ready\`). +**User override:** If the user explicitly requested "full review", "structured review", or "P1 gate", also run the ${outsideVoiceFor(ctx).label} structured review regardless of diff size (still requires \`CODEX_MODE: ready\` (or \`unverified\`)). --- @@ -406,7 +406,7 @@ THE PLAN: } function codexPlanReviewRun(ctx: TemplateContext, ceo: boolean, needsApprovalReadiness: boolean): string { - return `**If \`CODEX_MODE: ready\` — run ${outsideVoiceFor(ctx).label}:** + return `**If \`CODEX_MODE: ready\` (or \`unverified\`) — run ${outsideVoiceFor(ctx).label}:** ${['plan-ceo-review', 'plan-eng-review'].includes(ctx.skillName) ? `Run this block only for \`ready\`, in the one foreground Bash call described below. Its opening harness guard rechecks the fresh shell: exit 78 uses the same Native @@ -717,7 +717,7 @@ numbers, and CHANGELOG entries that over- or under-sell what shipped. Be terse. THE DOCS AND DIFF: " -**If \`CODEX_MODE: ready\` — run ${outsideVoiceFor(ctx).label}:** +**If \`CODEX_MODE: ready\` (or \`unverified\`) — run ${outsideVoiceFor(ctx).label}:** ${outsideVoiceInvocation(ctx, { timeoutMs: 300000, diffCommand: 'DOC_DIFF_BASE=$(git merge-base origin/ HEAD 2>/dev/null || git merge-base HEAD) && git diff "$DOC_DIFF_BASE" HEAD' })} diff --git a/scripts/resolvers/outside-voice.ts b/scripts/resolvers/outside-voice.ts index 43a41dbea..751bcd5f9 100644 --- a/scripts/resolvers/outside-voice.ts +++ b/scripts/resolvers/outside-voice.ts @@ -128,13 +128,14 @@ export function outsideVoiceCommand(ctx: TemplateContext, opts: OutsideCommandOp const root = toShellPath(ctx.paths.skillRoot); const prompt = sh(opts.promptFile ?? ''); const codex = opts.structuredBase - ? `codex review --base ${sh(opts.structuredBase)} ${CODEX_REVIEW_MODEL_CONFIG_FLAG} -c 'model_reasoning_effort="${opts.reasoningEffort ?? 'high'}"' ${CODEX_WEB_SEARCH_FLAG} < /dev/null` - : `codex exec "$_OUTSIDE_PROMPT" -C "$_REPO_ROOT" -s read-only ${CODEX_MODEL_CONFIG_FLAG} -c 'model_reasoning_effort="${opts.reasoningEffort ?? 'high'}"' ${CODEX_WEB_SEARCH_FLAG} < /dev/null`; + ? `codex review --base ${sh(opts.structuredBase)} -c "sandbox_mode=\\"\${_GSTACK_CODEX_SANDBOX:?}\\"" ${CODEX_REVIEW_MODEL_CONFIG_FLAG} -c 'model_reasoning_effort="${opts.reasoningEffort ?? 'high'}"' ${CODEX_WEB_SEARCH_FLAG} < /dev/null` + : `codex exec "$_OUTSIDE_PROMPT" -C "$_REPO_ROOT" -s "\${_GSTACK_CODEX_SANDBOX:?}" ${CODEX_MODEL_CONFIG_FLAG} -c 'model_reasoning_effort="${opts.reasoningEffort ?? 'high'}"' ${CODEX_WEB_SEARCH_FLAG} --json -o "$_OUTSIDE_TMP/text" < /dev/null`; const invocation = v.id === 'codex' ? `source "${bin}/gstack-codex-probe" && _gstack_codex_select_model ${opts.structuredBase ? 'review' : 'exec'} || exit 1 +_gstack_codex_sandbox_preflight >/dev/null || exit 1 ${opts.structuredBase ? '' : '_OUTSIDE_PROMPT=$(cat "$_OUTSIDE_INPUT") || exit 1\n'}_OUTSIDE_EXIT=0 -_gstack_codex_timeout_wrapper ${Math.ceil(opts.timeoutMs / 1000)} ${codex} >"$_OUTSIDE_TMP/text" 2>"$_OUTSIDE_TMP/stderr" || _OUTSIDE_EXIT=$? -cat "$_OUTSIDE_TMP/text" || { [ "$_OUTSIDE_EXIT" -ne 0 ] || _OUTSIDE_EXIT=1; }` +_gstack_codex_timeout_wrapper ${Math.ceil(opts.timeoutMs / 1000)} ${codex} >"$_OUTSIDE_TMP/${opts.structuredBase ? 'text' : 'events'}" 2>"$_OUTSIDE_TMP/stderr" || _OUTSIDE_EXIT=$? +${opts.structuredBase ? 'cat "$_OUTSIDE_TMP/text"' : 'cat "$_OUTSIDE_TMP/text" 2>/dev/null || tail -n 20 "$_OUTSIDE_TMP/events"'}` : `_OUTSIDE_EXIT=0 "${bin}/gstack-claude-code" --cwd "$_REPO_ROOT" --access ${opts.access ?? 'none'} --timeout-ms ${opts.timeoutMs} <"$_OUTSIDE_INPUT" >"$_OUTSIDE_TMP/result.json" 2>"$_OUTSIDE_TMP/stderr" || _OUTSIDE_EXIT=$? # Preserve session/usage/modelUsage from this JSON; multiple models have no invented primary. @@ -159,7 +160,7 @@ ${v.id === 'codex' && ctx.skillName === 'autoplan' ? `if [ "$_OUTSIDE_EXIT" -eq fi` : ''} cat "$_OUTSIDE_TMP/stderr" >&2 || { [ "$_OUTSIDE_EXIT" -ne 0 ] || _OUTSIDE_EXIT=1; } _OUTSIDE_RC=0 -bun "${root}/lib/outside-review-result.ts" --label '${v.label} outside review' --exit "$_OUTSIDE_EXIT" --stderr "$_OUTSIDE_TMP/stderr" ${opts.gate ?? 'review'} "$_OUTSIDE_TMP/text" || _OUTSIDE_RC=$? +bun "${root}/lib/outside-review-result.ts" --label '${v.label} outside review' --exit "$_OUTSIDE_EXIT" --stderr "$_OUTSIDE_TMP/stderr" ${v.id === 'codex' && !opts.structuredBase ? '--events "$_OUTSIDE_TMP/events" ' : ''}${opts.gate ?? 'review'} "$_OUTSIDE_TMP/text" || _OUTSIDE_RC=$? ${v.id === 'claude-code' ? '[ "$_OUTSIDE_RC" -eq 1 ] || cat "$_OUTSIDE_TMP/text" || exit 1\n' : ''}case "$_OUTSIDE_RC" in 0|3) ;; 4) echo 'OUTSIDE_STATUS: unverified provider=${v.id} host=${ctx.host}'; exit 4 ;; @@ -241,7 +242,7 @@ export function outsideVoiceFailurePolicy(ctx: TemplateContext, opts: OutsideVoi export function outsideVoiceProvenance(ctx: TemplateContext, phase: string): string { const v = outsideVoiceFor(ctx); - return `Retain the historical review-log skill ID; add \`"host":"${ctx.host}","outside_provider":"${v.id}","outside_status":"completed|unavailable|disabled|skipped","phase":"${phase}"\`. Record differing attempt outcomes separately. \`source:"${v.id}"\` requires completed CLI output; native uses \`source:"in-host"\` (historical \`source:"claude"\`: native Claude). Availability/native fallback is not outside completion. Preserve all reported modelUsage; unknown model identity stays unknown.`; + return `Retain the historical review-log skill ID; add \`"host":"${ctx.host}","outside_provider":"${v.id}","outside_status":"completed|unavailable|disabled|skipped","phase":"${phase}"\`. Record differing attempt outcomes separately. \`source:"${v.id}"\` requires completed CLI output; native uses \`source:"in-host"\` (historical \`source:"claude"\`: native Claude). Availability/native fallback is not outside completion. Preserve all reported modelUsage; unknown model identity stays unknown.${v.id === 'codex' ? ' If `WARNING: GSTACK_CODEX_NO_SANDBOX=1` printed, add `"codex_sandbox":"danger-full-access"`.' : ''}`; } export function generateOutsideVoiceRouting(ctx: TemplateContext): string { diff --git a/test/autoplan-dual-voice-evidence.test.ts b/test/autoplan-dual-voice-evidence.test.ts index 0b6695a0e..ed22b72e6 100644 --- a/test/autoplan-dual-voice-evidence.test.ts +++ b/test/autoplan-dual-voice-evidence.test.ts @@ -29,17 +29,20 @@ function fixture(plan?:string){ return {dir,method,snapshot,file,events,options,read:(rows=events)=>autoplanDualVoiceEvidence(rows,options)}; } test('actual6bd source Read, unused probe branch and0Hspec Agent earn zero phase voice credit',()=>{ - const f=fixture(),actual=f.read(clone(captured.events)); + const f=fixture(); + // Judge the capture against the probe block it executed (B1 later added the sandbox preflight). + f.options.commands={...f.options.commands,probe:captured.sourceBoundB176.commandContract.probe}; + const actual=f.read(clone(captured.events)); expect(actual).toMatchObject({claudeVoiceFired:false,codexVoiceFired:false,codexUnavailable:false,reviewDispatched:false,probeMode:'ready'}); }); test('actual snapshot producer and complete parent request/ACK pairs establish both voices',()=>{ const f=fixture();expect(f.read()).toMatchObject({claudeVoiceFired:true,codexVoiceFired:true,codexUnavailable:false,reviewDispatched:true,nativeToolUseId:'native',outsideToolUseId:'outside'}); }); -test.each(['not_installed','not_authed','broken_install','model_unusable'])('actual final probe result supports unavailable fallback: %s',mode=>{ +test.each(['not_installed','not_authed','broken_install','sandbox_unavailable','model_unusable'])('actual final probe result supports unavailable fallback: %s',mode=>{ const f=fixture();f.events.splice(4);f.events[1]=ack('probe','CODEX_MODE: '+mode); expect(f.read()).toMatchObject({claudeVoiceFired:true,codexVoiceFired:false,codexUnavailable:true}); }); -test.each(['ready','disabled','under_codex','unknown'])('probe outcome supplies no unavailable credit: %s',mode=>{ +test.each(['ready','unverified','disabled','under_codex','unknown'])('probe outcome supplies no unavailable credit: %s',mode=>{ const f=fixture();f.events.splice(4);f.events[1]=ack('probe','CODEX_MODE: '+mode);expect(f.read().codexUnavailable).toBe(false); }); test.each(['missing','error','foreign','child','unowned','method','phase','math','no-launch','wrong-input','mutable'])('native dispatch rejects %s evidence',kind=>{ @@ -261,7 +264,7 @@ test.each(['assignment-only','set-config','other-config','foreign-reader','or-co if(kind==='before-cd')input.command=guards+'\ncd '+f.dir+'\n'+f.options.commands.outside.replace("''","'"+f.file+"'"); if(kind==='changed-harness')input.command=input.command.replace('exit 78','exit 0'); if(kind==='changed-timeout')input.command=input.command.replace('_gstack_codex_timeout_wrapper 540','_gstack_codex_timeout_wrapper 1'); - if(kind==='changed-sandbox')input.command=input.command.replace('-s read-only','-s danger-full-access'); + if(kind==='changed-sandbox')input.command=input.command.replace('-s "${_GSTACK_CODEX_SANDBOX:?}"','-s danger-full-access'); if(kind==='changed-prompt')input.command=input.command.replace('codex exec "$_OUTSIDE_PROMPT"','codex exec "Different plan"'); if(kind==='skipped-validator')input.command=input.command.replace(/^bun .*outside-review-result.*\n/m,''); if(kind==='suffix')input.command+='\ntrue'; diff --git a/test/codex-hardening.test.ts b/test/codex-hardening.test.ts index ff01ee005..f850df162 100644 --- a/test/codex-hardening.test.ts +++ b/test/codex-hardening.test.ts @@ -626,13 +626,16 @@ describe('codex skeleton+sections union: review sandbox + fail-closed gate + tim for (const relPath of ['codex tmpl union', 'codex rendered union'] as const) { const read = () => readCodexUnion(relPath === 'codex tmpl union' ? 'tmpl' : 'rendered'); - test(`${relPath}: (a) every scoped codex review invocation pins sandbox_mode="read-only"`, () => { + test(`${relPath}: (a) every scoped codex review invocation pins sandbox_mode from the selected sandbox`, () => { const invocations = read() .split('\n') .filter((l) => /_gstack_codex_timeout_wrapper\s+\d+\s+codex\s+review\b/.test(l)); expect(invocations.length).toBeGreaterThanOrEqual(1); for (const line of invocations) { - expect(line).toContain('sandbox_mode="read-only"'); + // _gstack_codex_select_model sets read-only (full access only for + // GSTACK_CODEX_NO_SANDBOX=1, test/codex-model-probe.test.ts); :? stops + // an unselected command instead of inheriting config.toml's default. + expect(line).toContain('sandbox_mode=\\"${_GSTACK_CODEX_SANDBOX:?}\\"'); // `codex review` has no -s/--sandbox flag (verified 0.147.0) — the // config override is the only lever. `-s read-only` here would fail // at argv parsing, which check (b) would then read as a gate FAIL. diff --git a/test/codex-invocation-sites.test.ts b/test/codex-invocation-sites.test.ts new file mode 100644 index 000000000..edcd1d067 --- /dev/null +++ b/test/codex-invocation-sites.test.ts @@ -0,0 +1,90 @@ +/** + * B1 static coverage: every rendered fence that runs `codex exec` or + * `codex review` as a command routes the result through the shared validator + * (lib/outside-review-result.ts) later in the same fence, and takes its + * sandbox from `_GSTACK_CODEX_SANDBOX` (read-only unless GSTACK_CODEX_NO_SANDBOX=1). + * Prose mentions of the commands are ignored: only command positions count. + */ +import { afterAll, describe, expect, test } from 'bun:test'; +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { spawnSync } from 'node:child_process'; + +const ROOT = path.resolve(import.meta.dir, '..'); +const TMP = fs.mkdtempSync(path.join(os.tmpdir(), 'codex-sites-')); +afterAll(() => fs.rmSync(TMP, { recursive: true, force: true })); + +/** `codex exec|review` at the start of a shell command, optionally behind the timeout wrapper. */ +const COMMAND = /(?:^|[;&|]\s*|\$\(\s*)(?:_gstack_codex_timeout_wrapper\s+\d+\s+)?codex\s+(?:exec|review)\b/; +const VALIDATOR = /outside-review-result\.ts\b/; + +interface Site { file: string; line: string; fence: string; after: string } + +function codexSites(markdown: string, file: string): Site[] { + const sites: Site[] = []; + for (const match of markdown.matchAll(/```bash\n([\s\S]*?)```/g)) { + const fence = match[1]!; + const lines = fence.split('\n'); + lines.forEach((line, i) => { + const code = line.trim(); + if (code.startsWith('#') || !COMMAND.test(code)) return; + sites.push({ file, line: code, fence, after: lines.slice(i + 1).join('\n') }); + }); + } + return sites; +} + +function render(host: string): string[] { + const out = path.join(TMP, host); + const r = spawnSync('bun', ['run', 'scripts/gen-skill-docs.ts', '--host', host, '--out-dir', out], { cwd: ROOT, encoding: 'utf8', timeout: 120000 }); + if (r.status !== 0) throw new Error(`render ${host} failed: ${r.stderr}`); + const files: string[] = []; + const walk = (dir: string) => { + for (const entry of fs.readdirSync(dir, { withFileTypes: true })) { + const p = path.join(dir, entry.name); + if (entry.isDirectory()) walk(p); + else if (entry.name.endsWith('.md')) files.push(p); + } + }; + walk(out); + return files; +} + +describe('every rendered Codex invocation is classified by the shared validator', () => { + const rendered = Object.fromEntries(['claude', 'codex'].map(host => [host, render(host).flatMap(f => codexSites(fs.readFileSync(f, 'utf8'), path.relative(TMP, f)))])); + + test('the Claude render has the expected invocation sites', () => { + const files = new Set(rendered.claude!.map(s => s.file.replace(/^claude\//, ''))); + for (const expected of ['codex/sections/review-mode.md', 'codex/sections/challenge-mode.md', 'codex/sections/consult-mode.md', + 'review/sections/adversarial.md', 'ship/sections/adversarial.md']) expect(files.has(expected)).toBe(true); + }); + + for (const host of ['claude', 'codex']) { + test(`${host}: each codex exec/review command is followed by the validator in its fence`, () => { + const missing = rendered[host]!.filter(site => !VALIDATOR.test(site.after)).map(s => `${s.file}: ${s.line}`); + expect(missing).toEqual([]); + }); + + test(`${host}: each codex exec/review command takes its sandbox from _GSTACK_CODEX_SANDBOX`, () => { + const literal = rendered[host]!.filter(site => !site.line.includes('${_GSTACK_CODEX_SANDBOX:?}')).map(s => `${s.file}: ${s.line}`); + expect(literal).toEqual([]); + }); + } + + test('negative controls: prose is ignored, an unvalidated or literal-sandbox command is flagged', () => { + const doc = [ + 'Run `codex review` to review.', + '```bash', + '# codex exec in a comment', + 'echo "use codex exec later"', + '_gstack_codex_timeout_wrapper 30 codex exec "x" -s read-only', + '```', + ].join('\n'); + const sites = codexSites(doc, 'planted.md'); + expect(sites.map(s => s.line)).toEqual(['_gstack_codex_timeout_wrapper 30 codex exec "x" -s read-only']); + expect(VALIDATOR.test(sites[0]!.after)).toBe(false); + expect(sites[0]!.line.includes('${_GSTACK_CODEX_SANDBOX:?}')).toBe(false); + expect(codexSites('```bash\nX=$(codex exec "y")\n```', 'p.md')).toHaveLength(1); + }); +}); diff --git a/test/codex-model-probe.test.ts b/test/codex-model-probe.test.ts index b51dd012c..1f7d3621b 100644 --- a/test/codex-model-probe.test.ts +++ b/test/codex-model-probe.test.ts @@ -28,11 +28,22 @@ import { spawnSync } from 'child_process'; import * as fs from 'fs'; import * as os from 'os'; import * as path from 'path'; +import { codexPreflight } from '../scripts/resolvers/constants'; const ROOT = path.resolve(import.meta.dir, '..'); const PROBE = path.join(ROOT, 'bin', 'gstack-codex-probe'); +const SANDBOX_FIXTURES = path.join(ROOT, 'test', 'fixtures', 'codex-sandbox'); const STUB = `#!/usr/bin/env bash +if [ "$1" = sandbox ]; then + echo "$*" >> "$STUB_LOG.sandbox" + case "\${STUB_SANDBOX:-ok}" in + ok) exit 0 ;; + userns) cat "$SANDBOX_FIXTURES/sandbox-userns-denied.stderr" >&2; exit 1 ;; + missing) cat "$SANDBOX_FIXTURES/sandbox-bwrap-missing.stderr" >&2; exit 101 ;; + unknown) echo "error: unrecognized subcommand 'sandbox'" >&2; exit 2 ;; + esac +fi echo "invoked" >> "$STUB_LOG" printf '%s\\n' "$*" >> "$STUB_ARGS_LOG" case "\${STUB_MODE:-ok}" in @@ -83,6 +94,7 @@ function runProbe(f: Fixture, stubMode: string, extraEnv: Record STUB_MODE: stubMode, STUB_LOG: f.stubLog, STUB_ARGS_LOG: f.stubArgsLog, + SANDBOX_FIXTURES, _TEL: 'off', ...extraEnv, }, @@ -339,3 +351,98 @@ describe('codex model probe (#2477)', () => { } }); }); + +describe('B1: codex sandbox preflight and unverified readiness', () => { + const unavailable = (detail: string) => + `Codex outside review unavailable: Codex's sandbox could not start here (${detail}). No review ran; this is missing coverage, not a pass. Fix: enable unprivileged user namespaces for this container, or set GSTACK_CODEX_NO_SANDBOX=1.`; + + for (const [mode, detail] of [ + ['userns', 'bwrap: No permissions to create new namespace, likely because the kernel does not allow non-privileged user namespaces. See or .'.slice(0, 240)], + ['missing', 'bubblewrap is unavailable: no system bwrap was found on PATH and no bundled codex-resources/bwrap binary was found next to the Codex executable'], + ] as const) { + test(`captured ${mode} failure -> sandbox unavailable (exit 3), named like the gate outcome`, () => { + const f = makeFixture(); + try { + const r = runProbe(f, 'ok', { STUB_SANDBOX: mode }, '_gstack_codex_sandbox_preflight; echo "rc=$?"'); + expect(r.stdout).toBe('CODEX_SANDBOX: unavailable\nrc=3\n'); + expect(r.stderr.trim()).toBe(unavailable(detail)); + expect(fs.readFileSync(`${f.stubLog}.sandbox`, 'utf-8')).toBe(`sandbox -c sandbox_mode="read-only" true\n`); + expect(invocations(f)).toBe(0); + } finally { fs.rmSync(f.home, { recursive: true, force: true }); } + }); + } + + test('a healthy sandbox, an older CLI without the subcommand, or a timeout defers to the post-run check', () => { + const f = makeFixture(); + try { + for (const mode of ['ok', 'unknown']) { + const r = runProbe(f, 'ok', { STUB_SANDBOX: mode }, '_gstack_codex_sandbox_preflight; echo "rc=$?"'); + expect(r.stdout).toBe('rc=0\n'); + expect(r.stderr).toBe(''); + } + const timedOut = runProbe(f, 'ok', { STUB_SANDBOX: 'userns' }, + '_gstack_codex_timeout_wrapper() { return 124; }; _gstack_codex_sandbox_preflight; echo "rc=$?"'); + expect(timedOut.stdout).toBe('rc=0\n'); + } finally { fs.rmSync(f.home, { recursive: true, force: true }); } + }); + + test('GSTACK_CODEX_NO_SANDBOX: exactly 1 selects full access with a warning each use; other values keep read-only', () => { + const f = makeFixture(); + try { + const warning = 'WARNING: GSTACK_CODEX_NO_SANDBOX=1: Codex runs this review without a sandbox and can read and write anything your user can. Use it only inside a container you trust.'; + const on = runProbe(f, 'ok', { GSTACK_CODEX_NO_SANDBOX: '1', STUB_SANDBOX: 'userns' }, + '_gstack_codex_select_model exec; echo "sandbox=$_GSTACK_CODEX_SANDBOX"; _gstack_codex_select_model exec; _gstack_codex_sandbox_preflight; echo "rc=$?"'); + expect(on.stdout).toBe('sandbox=danger-full-access\nrc=0\n'); + expect(on.stderr.split(warning).length - 1).toBe(2); + expect(fs.existsSync(`${f.stubLog}.sandbox`)).toBe(false); + for (const value of ['', 'true', 'yes', '0', '1 ']) { + const off = runProbe(f, 'ok', { GSTACK_CODEX_NO_SANDBOX: value }, '_gstack_codex_select_model exec; echo "sandbox=$_GSTACK_CODEX_SANDBOX"'); + expect(off.stdout).toBe('sandbox=read-only\n'); + expect(off.stderr).not.toContain('WARNING'); + } + } finally { fs.rmSync(f.home, { recursive: true, force: true }); } + }); + + test('the preflight runs only on Linux', () => { + const f = makeFixture(); + try { + fs.writeFileSync(path.join(f.stubDir, 'uname'), '#!/usr/bin/env bash\necho Darwin\n', { mode: 0o755 }); + const r = runProbe(f, 'ok', { STUB_SANDBOX: 'userns' }, '_gstack_codex_sandbox_preflight; echo "rc=$?"'); + expect(r.stdout).toBe('rc=0\n'); + expect(fs.existsSync(`${f.stubLog}.sandbox`)).toBe(false); + } finally { fs.rmSync(f.home, { recursive: true, force: true }); } + }); + + test('an inconclusive model probe marks readiness unverified without blocking stale callers', () => { + const f = makeFixture(); + try { + const r = runProbe(f, 'transient', {}, '_gstack_codex_model_probe; echo "rc=$? state=${_GSTACK_CODEX_PROBE_STATE:-}"'); + expect(r.stdout).toContain('CODEX_MODE: unverified'); + expect(r.stdout).toContain('rc=0 state=inconclusive'); + const ok = runProbe(f, 'ok', {}, '_gstack_codex_model_probe; echo "state=${_GSTACK_CODEX_PROBE_STATE:-none}"'); + expect(ok.stdout).toContain('state=none'); + } finally { fs.rmSync(f.home, { recursive: true, force: true }); } + }); + + test('the rendered preflight reports sandbox_unavailable before any paid probe, and unverified on an inconclusive probe', () => { + const block = codexPreflight({ disabledBehavior: 'codex-only' }).match(/```bash\n([\s\S]*?)\n```/)![1]; + for (const [sandbox, model, mode, probed] of [ + ['userns', 'ok', 'sandbox_unavailable', 0], + ['ok', 'transient', 'unverified', 1], + ['ok', 'ok', 'ready', 1], + ['ok', 'model400', 'model_unusable', 1], + ] as const) { + const f = makeFixture(); + try { + fs.mkdirSync(path.join(f.home, '.claude', 'skills'), { recursive: true }); + fs.symlinkSync(ROOT, path.join(f.home, '.claude', 'skills', 'gstack')); + const r = spawnSync('bash', ['-c', block], { encoding: 'utf8', timeout: 20000, env: { + PATH: `${f.stubDir}:${process.env.PATH ?? ''}`, HOME: f.home, CODEX_HOME: f.codexHome, GSTACK_HOME: f.gstackHome, + STUB_MODE: model, STUB_SANDBOX: sandbox, STUB_LOG: f.stubLog, STUB_ARGS_LOG: f.stubArgsLog, SANDBOX_FIXTURES } }); + expect(r.stdout).toContain(`CODEX_MODE: ${mode}`); + const paid = fs.existsSync(f.stubArgsLog) ? fs.readFileSync(f.stubArgsLog, 'utf-8').split('\n').filter(l => l.includes('reply OK')).length : 0; + expect(paid).toBe(probed); + } finally { fs.rmSync(f.home, { recursive: true, force: true }); } + } + }); +}); diff --git a/test/fixtures/codex-sandbox/README.md b/test/fixtures/codex-sandbox/README.md new file mode 100644 index 000000000..440ea4454 --- /dev/null +++ b/test/fixtures/codex-sandbox/README.md @@ -0,0 +1,29 @@ +# Codex sandbox fixtures (B1) + +Captured from codex-cli 0.160.0 (musl x86_64 vendor binary). They pin what +`lib/outside-review-result.ts` and `_gstack_codex_sandbox_preflight` classify. + +| File | Capture | Exit | +|---|---|---| +| `sandbox-userns-denied.stderr` | `codex sandbox -c 'sandbox_mode="read-only"' true` in Docker `ubuntu:24.04`, default seccomp profile, apt `bubblewrap` installed | 1 | +| `sandbox-bwrap-missing.stderr` | same command without `bubblewrap` installed (Rust panic) | 101 | +| `exec-json-userns-denied.jsonl` | `echo "Run cat a.txt and tell me its content in one word. If the command fails, say exactly what failed." \| codex exec - -s read-only -c 'model="gpt-5.4-mini"' -c skills.include_instructions=false --json` in the same container (stderr empty) | 0 | +| `review-userns-denied.stdout` / `.stderr` | `codex review --base master -c 'model="gpt-5.4-mini"' -c 'review_model="gpt-5.4-mini"' -c 'sandbox_mode="read-only"' :/cx:ro -v "$CODEX_HOME":/codexhome -e CODEX_HOME=/codexhome ubuntu:24.04 sh -c \ + 'apt-get update -qq && apt-get install -y -qq bubblewrap git; PATH=/cx:$PATH codex sandbox -c "sandbox_mode=\"read-only\"" true; echo $?' +``` + +Omit `bubblewrap` from the install for the exit-101 capture. Host paths in the +healthy captures are normalized to `/repo`, and lines written by the capture +machine's git wrapper are removed. diff --git a/test/fixtures/codex-sandbox/exec-json-healthy.jsonl b/test/fixtures/codex-sandbox/exec-json-healthy.jsonl new file mode 100644 index 000000000..45c58dc3f --- /dev/null +++ b/test/fixtures/codex-sandbox/exec-json-healthy.jsonl @@ -0,0 +1,8 @@ +{"type":"thread.started","thread_id":"01a10376-d6d1-7b60-8c14-6decd95f8437"} +{"type":"item.completed","item":{"id":"item_0","type":"error","message":"Model metadata for `gpt-5.4-mini` not found. Defaulting to fallback metadata; this can degrade performance and cause issues."}} +{"type":"turn.started"} +{"type":"item.completed","item":{"id":"item_1","type":"agent_message","text":"I’ll read `a.txt` and report it in one word."}} +{"type":"item.started","item":{"id":"item_2","type":"command_execution","command":"/bin/bash -c 'cat a.txt'","aggregated_output":"","exit_code":null,"status":"in_progress"}} +{"type":"item.completed","item":{"id":"item_2","type":"command_execution","command":"/bin/bash -c 'cat a.txt'","aggregated_output":"hello\n","exit_code":0,"status":"completed"}} +{"type":"item.completed","item":{"id":"item_3","type":"agent_message","text":"hello"}} +{"type":"turn.completed","usage":{"input_tokens":22391,"cached_input_tokens":11136,"cache_write_input_tokens":0,"output_tokens":85,"reasoning_output_tokens":0}} diff --git a/test/fixtures/codex-sandbox/exec-json-userns-denied.jsonl b/test/fixtures/codex-sandbox/exec-json-userns-denied.jsonl new file mode 100644 index 000000000..8f104b48c --- /dev/null +++ b/test/fixtures/codex-sandbox/exec-json-userns-denied.jsonl @@ -0,0 +1,8 @@ +{"type":"thread.started","thread_id":"01a10377-a0ec-7b01-81f6-384bd91a865b"} +{"type":"item.completed","item":{"id":"item_0","type":"error","message":"Model metadata for `gpt-5.4-mini` not found. Defaulting to fallback metadata; this can degrade performance and cause issues."}} +{"type":"turn.started"} +{"type":"item.completed","item":{"id":"item_1","type":"agent_message","text":"I’ll check `a.txt` and report the result exactly."}} +{"type":"item.started","item":{"id":"item_2","type":"command_execution","command":"/bin/bash -lc 'cat a.txt'","aggregated_output":"","exit_code":null,"status":"in_progress"}} +{"type":"item.completed","item":{"id":"item_2","type":"command_execution","command":"/bin/bash -lc 'cat a.txt'","aggregated_output":"bwrap: No permissions to create new namespace, likely because the kernel does not allow non-privileged user namespaces. See or .\n","exit_code":1,"status":"failed"}} +{"type":"item.completed","item":{"id":"item_3","type":"agent_message","text":"bwrap: No permissions to create new namespace, likely because the kernel does not allow non-privileged user namespaces."}} +{"type":"turn.completed","usage":{"input_tokens":22392,"cached_input_tokens":11136,"cache_write_input_tokens":0,"output_tokens":89,"reasoning_output_tokens":0}} diff --git a/test/fixtures/codex-sandbox/review-healthy.stderr b/test/fixtures/codex-sandbox/review-healthy.stderr new file mode 100644 index 000000000..06ae20ac7 --- /dev/null +++ b/test/fixtures/codex-sandbox/review-healthy.stderr @@ -0,0 +1,31 @@ +OpenAI Codex v0.160.0 +-------- +workdir: /repo +model: gpt-5.4-mini +provider: openai +approval: never +sandbox: read-only +reasoning effort: none +reasoning summaries: none +session id: 01a1037c-220d-7e32-816f-3dfb0b645a38 +-------- +user +changes against 'master' +warning: Codex could not find bubblewrap on PATH. Install bubblewrap with your OS package manager. See the sandbox prerequisites: https://developers.openai.com/codex/concepts/sandboxing#prerequisites. Codex will use the bundled bubblewrap in the meantime. +warning: Model metadata for `gpt-5.4-mini` not found. Defaulting to fallback metadata; this can degrade performance and cause issues. +warning: Model metadata for `gpt-5.4-mini` not found. Defaulting to fallback metadata; this can degrade performance and cause issues. +exec +/bin/bash -lc 'git diff e66c79820df46b1a805e79ffe69e1c0ea74e018c --stat && git diff e66c79820df46b1a805e79ffe69e1c0ea74e018c --' in /repo + succeeded in 0ms: + a.txt | 1 + + 1 file changed, 1 insertion(+) +diff --git a/a.txt b/a.txt +index ce01362..94954ab 100644 +--- a/a.txt ++++ b/a.txt +@@ -1 +1,2 @@ + hello ++world + +codex +The diff only adds a new line to a text file and does not introduce any code-path changes or functional regressions. No actionable defects are present in the patch. diff --git a/test/fixtures/codex-sandbox/review-healthy.stdout b/test/fixtures/codex-sandbox/review-healthy.stdout new file mode 100644 index 000000000..6d667e332 --- /dev/null +++ b/test/fixtures/codex-sandbox/review-healthy.stdout @@ -0,0 +1 @@ +The diff only adds a new line to a text file and does not introduce any code-path changes or functional regressions. No actionable defects are present in the patch. diff --git a/test/fixtures/codex-sandbox/review-mentions-sandbox.txt b/test/fixtures/codex-sandbox/review-mentions-sandbox.txt new file mode 100644 index 000000000..742267fa8 --- /dev/null +++ b/test/fixtures/codex-sandbox/review-mentions-sandbox.txt @@ -0,0 +1,2 @@ +[P2] lib/sandbox.ts:41 — the bwrap fallback swallows "bwrap: No permissions to create new namespace" when user namespaces are disabled, and the landlock error path is untested. +Recommendation: surface the sandbox error because a silent seccomp or landlock failure hides missing coverage. diff --git a/test/fixtures/codex-sandbox/review-userns-denied.stderr b/test/fixtures/codex-sandbox/review-userns-denied.stderr new file mode 100644 index 000000000..b83728f33 --- /dev/null +++ b/test/fixtures/codex-sandbox/review-userns-denied.stderr @@ -0,0 +1,37 @@ +OpenAI Codex v0.160.0 +-------- +workdir: /r +model: gpt-5.4-mini +provider: openai +approval: never +sandbox: read-only +reasoning effort: none +reasoning summaries: none +session id: 01a10377-a9a4-7802-9b30-b415edbc0bfb +-------- +user +changes against 'master' +warning: Model metadata for `gpt-5.4-mini` not found. Defaulting to fallback metadata; this can degrade performance and cause issues. +warning: Model metadata for `gpt-5.4-mini` not found. Defaulting to fallback metadata; this can degrade performance and cause issues. +exec +/bin/bash -lc 'git diff 98eb804c33b7c646f099ba6a3ba89ca40a3e28b7 --stat && git diff 98eb804c33b7c646f099ba6a3ba89ca40a3e28b7 --' in /r + exited 1 in 0ms: +bwrap: No permissions to create new namespace, likely because the kernel does not allow non-privileged user namespaces. See or . + +exec +/bin/bash -c "/bin/bash -lc 'cd /r && git diff 98eb804c33b7c646f099ba6a3ba89ca40a3e28b7 --stat && git diff 98eb804c33b7c646f099ba6a3ba89ca40a3e28b7 --'" in /r + exited 1 in 0ms: +bwrap: No permissions to create new namespace, likely because the kernel does not allow non-privileged user namespaces. See or . + +exec +/bin/bash -c 'pwd && ls -la && git status --short && git rev-parse --show-toplevel' in /r + exited 1 in 0ms: +bwrap: No permissions to create new namespace, likely because the kernel does not allow non-privileged user namespaces. See or . + +exec +/bin/bash -c 'ls /r' in / + exited 1 in 0ms: +bwrap: No permissions to create new namespace, likely because the kernel does not allow non-privileged user namespaces. See or . + +codex +I was unable to inspect the diff because every shell invocation failed immediately with the environment's bubblewrap/user-namespace restriction, so no concrete regressions could be identified from the provided patch. With no verifiable issues in the changed code, there are no findings to report. diff --git a/test/fixtures/codex-sandbox/review-userns-denied.stdout b/test/fixtures/codex-sandbox/review-userns-denied.stdout new file mode 100644 index 000000000..e4ac903b2 --- /dev/null +++ b/test/fixtures/codex-sandbox/review-userns-denied.stdout @@ -0,0 +1 @@ +I was unable to inspect the diff because every shell invocation failed immediately with the environment's bubblewrap/user-namespace restriction, so no concrete regressions could be identified from the provided patch. With no verifiable issues in the changed code, there are no findings to report. diff --git a/test/fixtures/codex-sandbox/sandbox-bwrap-missing.stderr b/test/fixtures/codex-sandbox/sandbox-bwrap-missing.stderr new file mode 100644 index 000000000..cddc7c25d --- /dev/null +++ b/test/fixtures/codex-sandbox/sandbox-bwrap-missing.stderr @@ -0,0 +1,5 @@ +WARNING: proceeding, even though we could not create PATH aliases: Refusing to create helper binaries under temporary dir "/tmp" (codex_home: AbsolutePathBuf("/tmp/.codex")) + +thread 'main' (15) panicked at linux-sandbox/src/launcher.rs:57:13: +bubblewrap is unavailable: no system bwrap was found on PATH and no bundled codex-resources/bwrap binary was found next to the Codex executable +note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace diff --git a/test/fixtures/codex-sandbox/sandbox-userns-denied.stderr b/test/fixtures/codex-sandbox/sandbox-userns-denied.stderr new file mode 100644 index 000000000..80e5f0140 --- /dev/null +++ b/test/fixtures/codex-sandbox/sandbox-userns-denied.stderr @@ -0,0 +1 @@ +bwrap: No permissions to create new namespace, likely because the kernel does not allow non-privileged user namespaces. See or . diff --git a/test/gen-skill-docs.test.ts b/test/gen-skill-docs.test.ts index 8a626db9e..59d9ef195 100644 --- a/test/gen-skill-docs.test.ts +++ b/test/gen-skill-docs.test.ts @@ -2193,7 +2193,8 @@ describe('DESIGN_OUTSIDE_VOICES resolver', () => { const marker = path.join(dir, 'must-not-execute'); const product = `A product with $(touch ${marker}), \`touch ${marker}\`, and "quotes".\nUsers: builders.\n`; fs.writeFileSync(brief, product); - fs.writeFileSync(path.join(dir, 'codex'), `#!${process.execPath}\nrequire('fs').writeFileSync(process.env.CAPTURE, JSON.stringify(process.argv.slice(2)));\nconsole.log('Recommendation: choose a clear hierarchy because builders need to find their work.');\n`, { mode: 0o700 }); + // Fake codex: the free sandbox preflight succeeds; exec writes its final message to -o. + fs.writeFileSync(path.join(dir, 'codex'), `#!${process.execPath}\nconst fs = require('fs'); const a = process.argv.slice(2); if (a[0] === 'sandbox') process.exit(0);\nfs.writeFileSync(process.env.CAPTURE, JSON.stringify(a));\nconst msg = 'Recommendation: choose a clear hierarchy because builders need to find their work.';\nif (a.includes('-o')) fs.writeFileSync(a[a.indexOf('-o') + 1], msg);\nconsole.log(msg);\n`, { mode: 0o700 }); const prepare = (file: string) => command.replace("''", quote(file)) .replaceAll('$HOME/.claude/skills/gstack', ROOT); const env = { ...process.env, PATH: dir + path.delimiter + process.env.PATH, CAPTURE: capture, diff --git a/test/helpers/autoplan-dual-voice-evidence.ts b/test/helpers/autoplan-dual-voice-evidence.ts index 63c8c73b0..3a8dd5363 100644 --- a/test/helpers/autoplan-dual-voice-evidence.ts +++ b/test/helpers/autoplan-dual-voice-evidence.ts @@ -259,7 +259,7 @@ export function autoplanDualVoiceEvidence(transcript: unknown[], options: Autopl } } } - result.codexUnavailable ||= result.claudeVoiceFired && ['not_installed', 'not_authed', 'broken_install', 'model_unusable'].includes(result.probeMode ?? ''); + result.codexUnavailable ||= result.claudeVoiceFired && ['not_installed', 'not_authed', 'broken_install', 'sandbox_unavailable', 'model_unusable'].includes(result.probeMode ?? ''); if (!result.claudeVoiceFired) result.reasons.push('No acknowledged current CEO phase dispatch'); if (!result.codexVoiceFired && !result.codexUnavailable) { result.reasons.push('No acknowledged outside execution or actual unavailable probe result'); diff --git a/test/outside-review-result.test.ts b/test/outside-review-result.test.ts index 4dea9c97a..cd27e487d 100644 --- a/test/outside-review-result.test.ts +++ b/test/outside-review-result.test.ts @@ -133,3 +133,66 @@ describe('outside-review-result CLI', () => { expect(r.stdout).toBe('OUTSIDE_STATUS: completed provider=codex host=claude\n'); }); }); + +describe('B1: a review whose sandbox could not start is unavailable, not clean', () => { + const FIX = path.join(ROOT, 'test', 'fixtures', 'codex-sandbox'); + const read = (name: string) => fs.readFileSync(path.join(FIX, name), 'utf8'); + const BWRAP = 'bwrap: No permissions to create new namespace, likely because the kernel does not allow non-privileged user namespaces.'; + + test('captured codex review in a container: exit 0, "no findings" text, bwrap failures on stderr', () => { + const result = classifyOutsideReview({ text: read('review-userns-denied.stdout'), stderr: read('review-userns-denied.stderr'), exit: 0, gate: 'structured' }); + expect(result.verdict).toBe('unavailable'); + expect(result.reason).toBe('sandbox_unavailable'); + expect(result.detail).toStartWith(BWRAP); + // The text-only wrapper (old importers) also stops passing it: the review says it could not read the diff. + expect(validateOutsideReview(read('review-userns-denied.stdout'), 'structured').completed).toBe(false); + }); + + test('captured codex exec --json in a container: every command failed with a sandbox error', () => { + const result = classifyOutsideReview({ text: 'hello', events: read('exec-json-userns-denied.jsonl'), gate: 'execution' }); + expect([result.verdict, result.reason]).toEqual(['unavailable', 'sandbox_unavailable']); + expect(result.detail).toStartWith(BWRAP); + }); + + test('captured healthy runs count as executed even though Codex warns about bubblewrap', () => { + const review = classifyOutsideReview({ text: read('review-healthy.stdout'), stderr: read('review-healthy.stderr'), gate: 'structured' }); + expect(review.execution.state).toBe('ran'); + expect(read('review-healthy.stderr')).toContain('could not find bubblewrap on PATH'); + expect(classifyOutsideReview({ text: 'hello', events: read('exec-json-healthy.jsonl'), gate: 'execution' }).verdict).toBe('clean'); + }); + + test('a real review that discusses bwrap, namespaces, landlock and seccomp still passes', () => { + const result = classifyOutsideReview({ text: read('review-mentions-sandbox.txt'), stderr: '', exit: 0, gate: 'review' }); + expect([result.verdict, result.findings.highest, result.execution.state]).toEqual(['clean', 'P2', 'ran']); + }); + + test('sandbox stderr with a non-zero exit (preflight fixtures) names the sandbox line', () => { + const denied = classifyOutsideReview({ text: '', stderr: read('sandbox-userns-denied.stderr'), exit: 1, gate: 'review' }); + expect([denied.reason, denied.detail?.slice(0, BWRAP.length)]).toEqual(['sandbox_unavailable', BWRAP]); + const missing = classifyOutsideReview({ text: '', stderr: read('sandbox-bwrap-missing.stderr'), exit: 101, gate: 'review' }); + expect(missing.reason).toBe('sandbox_unavailable'); + expect(missing.detail).toStartWith('bubblewrap is unavailable'); + }); + + test('exact execution-failure phrases are the last fallback; positive evidence wins', () => { + const probe = 'I could not run commands in this sandbox, so I reviewed nothing. No issues found.'; + expect(classifyOutsideReview({ text: probe, stderr: '', gate: 'structured' }).reason).toBe('commands_failed'); + expect(classifyOutsideReview({ text: `The diff could not be read.\n${RECOMMEND}`, gate: 'review' }).reason).toBe('commands_failed'); + const executed = '{"type":"item.completed","item":{"type":"command_execution","exit_code":0,"status":"completed","aggregated_output":"ok"}}'; + expect(classifyOutsideReview({ text: `If commands could not run, the helper retries.\n${RECOMMEND}`, events: executed, gate: 'review' }).verdict).toBe('clean'); + const failed = '{"type":"item.completed","item":{"type":"command_execution","exit_code":2,"status":"failed","aggregated_output":"fatal: not a git repository"}}'; + expect(classifyOutsideReview({ text: RECOMMEND, events: `${failed}\n${failed}`, gate: 'review' }).detail).toBe('all 2 commands failed'); + }); + + test('a clean "I did not find any issues" with empty stderr still passes', () => { + expect(classifyOutsideReview({ text: 'I did not find any issues.', stderr: '', exit: 0, gate: 'structured' }).verdict).toBe('clean'); + }); + + test('the CLI reads --events', () => { + const events = path.join(FIX, 'exec-json-userns-denied.jsonl'); + const r = cli(['--label', 'Codex outside review', '--events', events, 'execution', file('hello')]); + expect(r.status).toBe(1); + expect(r.stdout).toContain('REASON: sandbox_unavailable'); + expect(r.stderr).toStartWith(`Codex outside review unavailable: Codex's sandbox could not start here (${BWRAP}`); + }); +}); diff --git a/test/outside-voice-invocation.test.ts b/test/outside-voice-invocation.test.ts index adbce5dcb..fcac18851 100644 --- a/test/outside-voice-invocation.test.ts +++ b/test/outside-voice-invocation.test.ts @@ -24,6 +24,8 @@ fs.writeFileSync(PROMPT, PROMPT_TEXT); const fakeSource = ` import {writeFileSync} from 'node:fs'; const args = process.argv.slice(2); +// The free sandbox preflight (\`codex sandbox ... true\`) succeeds unless a test plants its failure. +if (args[0] === 'sandbox') { if (process.env.FAKE_SANDBOX_STDERR) { console.error(process.env.FAKE_SANDBOX_STDERR); process.exit(1); } process.exit(0); } const claude = process.env.FAKE_PROVIDER === 'claude-code'; const prompt = claude ? await Bun.stdin.text() : args[0] === 'exec' ? args[1] : ''; writeFileSync(process.env.CAPTURE!, JSON.stringify({args,prompt,cwd:process.cwd()})); @@ -36,7 +38,13 @@ const response = process.env.FAKE_RESPONSE || 'Recommendation: fix the seeded de if (claude) { if (process.env.FAKE_MODE === 'malformed') {console.log('{broken');process.exit(0);} console.log(JSON.stringify({result:response,session_id:'outside-session',modelUsage:{'model-a':{inputTokens:4},'model-b':{inputTokens:8}}})); +} else if (args.includes('-o')) { + // codex exec --json -o : events on stdout, the final message in . + writeFileSync(args[args.indexOf('-o') + 1], response); + if (process.env.FAKE_EVENTS_FILE) process.stdout.write(await Bun.file(process.env.FAKE_EVENTS_FILE).text()); + else console.log(JSON.stringify({type:'item.completed',item:{type:'agent_message',text:response}})); } else console.log(response); +if (process.env.FAKE_STDERR_FILE) process.stderr.write(await Bun.file(process.env.FAKE_STDERR_FILE).text()); process.exit(process.env.FAKE_MODE === 'nonzero' ? 4 : 0); `; fs.writeFileSync(FAKE_CLAUDE, fakeSource); @@ -211,6 +219,55 @@ describe('generated outside-review dispatch', () => { } }); + describe('B1: Codex sandbox failures are missing coverage, never a pass', () => { + const FIX = path.join(ROOT, 'test', 'fixtures', 'codex-sandbox'); + const SANDBOX_LINE = "Codex outside review unavailable: Codex's sandbox could not start here (bwrap: No permissions to create new namespace"; + + test('the free preflight stops before any paid call', () => { + const result = invoke('claude', {}, { FAKE_SANDBOX_STDERR: fs.readFileSync(path.join(FIX, 'sandbox-userns-denied.stderr'), 'utf8') }); + expect(result.status).toBe(1); + expect(fs.existsSync(CAPTURE)).toBe(false); + expect(result.stderr).toContain(SANDBOX_LINE); + expect(result.stdout).not.toContain('OUTSIDE_STATUS: completed'); + }); + + test('exec: captured events where every command hit the sandbox are unavailable despite exit 0', () => { + const result = invoke('claude', {}, { FAKE_EVENTS_FILE: path.join(FIX, 'exec-json-userns-denied.jsonl'), + FAKE_RESPONSE: 'I could not run commands here. No issues found.\nRecommendation: ship because no issues were found.' }); + expect(result.status).toBe(1); + expect(capture().args).toContain('--json'); + expect(result.stdout).toContain('REASON: sandbox_unavailable'); + expect(result.stderr).toContain(SANDBOX_LINE); + expect(result.stdout).not.toContain('OUTSIDE_STATUS: completed'); + }); + + test('structured review: captured container transcript on stderr is unavailable, healthy transcript completes', () => { + const failed = invoke('claude', { structuredBase: 'main', gate: 'structured' }, { + FAKE_RESPONSE: fs.readFileSync(path.join(FIX, 'review-userns-denied.stdout'), 'utf8'), + FAKE_STDERR_FILE: path.join(FIX, 'review-userns-denied.stderr') }); + expect(failed.status).toBe(1); + expect(failed.stderr).toContain(SANDBOX_LINE); + expect(failed.stdout).not.toContain('OUTSIDE_STATUS: completed'); + expect(capture().args).toContain('sandbox_mode="read-only"'); + const healthy = invoke('claude', { structuredBase: 'main', gate: 'structured' }, { + FAKE_RESPONSE: '[P2] naming nit', FAKE_STDERR_FILE: path.join(FIX, 'review-healthy.stderr') }); + expect(healthy.status).toBe(0); + expect(healthy.stdout).toContain('OUTSIDE_STATUS: completed'); + }); + + test('GSTACK_CODEX_NO_SANDBOX=1 switches every site to full access, warns, and skips the preflight', () => { + const env = { GSTACK_CODEX_NO_SANDBOX: '1', FAKE_SANDBOX_STDERR: 'bwrap: No permissions to create new namespace' }; + const exec = invoke('claude', {}, env); + expect(exec.status).toBe(0); + expect(capture().args.slice(capture().args.indexOf('-s'), capture().args.indexOf('-s') + 2)).toEqual(['-s', 'danger-full-access']); + expect(exec.stderr).toContain('WARNING: GSTACK_CODEX_NO_SANDBOX=1: Codex runs this review without a sandbox'); + expect(invoke('claude', { structuredBase: 'main', gate: 'structured' }, { ...env, FAKE_RESPONSE: '[P2] nit' }).status).toBe(0); + expect(capture().args).toContain('sandbox_mode="danger-full-access"'); + expect(invoke('claude', {}, { GSTACK_CODEX_NO_SANDBOX: 'true' }).status).toBe(0); + expect(capture().args.slice(capture().args.indexOf('-s'), capture().args.indexOf('-s') + 2)).toEqual(['-s', 'read-only']); + }); + }); + test('malformed Claude JSON cannot reach completion evaluation', () => { const result = invoke('codex',{}, {FAKE_MODE:'malformed'}); expect(result.status).toBe(1); @@ -251,7 +308,8 @@ describe('generated outside-review dispatch', () => { test('autoplan retains its Codex timeout event and hang record', () => { const events = path.join(TMP, 'autoplan-events'); const probe = path.join(BIN, 'gstack-codex-probe'); - fs.writeFileSync(probe, `_gstack_codex_select_model() { _GSTACK_CODEX_SEL=gpt-6-astra; } + fs.writeFileSync(probe, `_gstack_codex_select_model() { _GSTACK_CODEX_SEL=gpt-6-astra; _GSTACK_CODEX_SANDBOX=read-only; } +_gstack_codex_sandbox_preflight() { return 0; } _gstack_codex_timeout_wrapper() { echo 'Partial finding'; return 124; } _gstack_codex_log_event() { printf '%s %s\\n' "$1" "$2" >> "$FAKE_EVENTS"; } _gstack_codex_log_hang() { printf '%s %s\\n' "$1" "$2" >> "$FAKE_EVENTS"; } @@ -283,6 +341,7 @@ _gstack_codex_log_hang() { printf '%s %s\\n' "$1" "$2" >> "$FAKE_EVENTS"; } const pidFile = path.join(TMP, 'stubborn.pid'); fs.mkdirSync(stubborn, { recursive: true }); fs.writeFileSync(path.join(stubborn, 'codex'), `#!/bin/bash +[ "$1" = sandbox ] && exit 0 trap '' TERM echo $$ > "$STUBBORN_PID" echo 'Partial finding before the deadline' diff --git a/test/skill-validation.test.ts b/test/skill-validation.test.ts index e881e715f..393c5a90b 100644 --- a/test/skill-validation.test.ts +++ b/test/skill-validation.test.ts @@ -1517,8 +1517,9 @@ describe('Codex skill', () => { expect(match).not.toBeNull(); const resumeCommand = match![0]; expect(resumeCommand).not.toContain(' -C '); - expect(resumeCommand).not.toContain(' -s read-only'); - expect(resumeCommand).toContain("-c 'sandbox_mode=\"read-only\"'"); + expect(resumeCommand).not.toMatch(/ -s /); + // resume takes the sandbox only as config; _gstack_codex_select_model sets it (read-only by default). + expect(resumeCommand).toContain('-c "sandbox_mode=\\"${_GSTACK_CODEX_SANDBOX:?}\\""'); }); test('codex union contains cost tracking', () => {