mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-01 06:24:41 +01:00
B1: Codex reviews whose sandbox could not start are unavailable, not a pass
In containers without unprivileged user namespaces every Codex command fails, yet the review exits 0 and concludes 'no findings' (captured in test/fixtures/codex-sandbox/). The validator now takes execution evidence: 'codex exec --json' command events and the 'codex review' stderr transcript (a command that succeeded proves execution); sandbox failure lines on stderr or in failed command output return unavailable(sandbox_unavailable); exact 'could not run commands / read the diff' phrases are the last fallback. Review text that merely discusses bwrap or namespaces still passes. A free 'codex sandbox ... true' preflight (Linux, 10 s) runs before the paid model probe and every automatic outside review; CODEX_MODE gains sandbox_unavailable, and an inconclusive model probe reports unverified instead of ready (exit code kept for stale callers). Every Codex site (outside-voice resolver, /codex review, challenge, consult) takes its sandbox from _GSTACK_CODEX_SANDBOX and pipes its result through the validator; a static test parses rendered command positions for both. GSTACK_CODEX_NO_SANDBOX=1 (exactly 1, shell env only) switches to danger-full-access with a warning on every use. Sandbox pins in existing tests move from the literal read-only to the selected-sandbox variable; captured autoplan probes are judged against the probe block they executed.
This commit is contained in:
garrytan committed
1 parent
6f5eb600ca
commit
0642559f60
28 files changed
+639
-59
No files matched your search
@@ -110,7 +110,8 @@ On any error: continue — ${feature} is informational, not a gate.`;
|
||||
* 5. sets ONE canonical mode var and echoes `CODEX_MODE: <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=<supported-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=<supported-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.`;
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -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: <include current contents of each touched document, with its path, plus affected source context; the parent appends the release diff below>"
|
||||
|
||||
**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/<base> HEAD 2>/dev/null || git merge-base <base> HEAD) && git diff "$DOC_DIFF_BASE" HEAD' })}
|
||||
|
||||
|
||||
@@ -128,13 +128,14 @@ export function outsideVoiceCommand(ctx: TemplateContext, opts: OutsideCommandOp
|
||||
const root = toShellPath(ctx.paths.skillRoot);
|
||||
const prompt = sh(opts.promptFile ?? '<prepared-prompt-file>');
|
||||
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 {
|
||||
|
||||
Reference in new issue
Block a user