diff --git a/CHANGELOG.md b/CHANGELOG.md index 1b15b9f..06570cb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,39 @@ All notable changes to this project are documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/). +## [Unreleased] + +### Fixed +- **codex backend produced garbage instead of a prompt.** `codex exec` streams its + whole session log to stdout — version banner, workdir/model/session-id block, + `hook:` lines, MCP and skill-loading `ERROR` lines, the echoed prompt, and a + trailing token count — and `codex.sh` piped all of it through as the improved + prompt. It now takes the agent's final message from `--output-last-message`, + and only falls back to emitting the session log (on failure) so rate-limit + detection can still cascade. +- **codex backend consumed inherited stdin**, appending a duplicate `` + block to the prompt. It now runs with ` block to the prompt. +# Mitigation: take the agent's final message from `-o/--output-last-message`, +# close stdin, and run read-only so the generator cannot execute the request. +# On failure the session log still goes to stdout so the caller's rate-limit +# detection can sniff it. set -euo pipefail @@ -22,4 +35,42 @@ if [ -n "${PROMPT_IMPROVER_MODEL:-}" ]; then MODEL_ARGS=(-m "$PROMPT_IMPROVER_MODEL") fi -exec codex exec "${MODEL_ARGS[@]}" "$(cat "$PROMPT_FILE")" +MSG_FILE=$(mktemp -t pi-codex-msg.XXXXXX) +LOG_FILE=$(mktemp -t pi-codex-log.XXXXXX) +ERR_FILE=$(mktemp -t pi-codex-err.XXXXXX) +trap 'rm -f "$MSG_FILE" "$LOG_FILE" "$ERR_FILE"' EXIT + +set +e +codex exec \ + "${MODEL_ARGS[@]}" \ + --output-last-message "$MSG_FILE" \ + --sandbox read-only \ + --skip-git-repo-check \ + --color never \ + "$(cat "$PROMPT_FILE")" \ + >"$LOG_FILE" 2>"$ERR_FILE" &2 || true +fi + +if [ "$CODE" -eq 0 ] && [ -s "$MSG_FILE" ]; then + cat "$MSG_FILE" + exit 0 +fi + +# Failure (or empty final message): surface the session log so the caller can +# detect rate/usage limits and cascade, and keep the exit code intact. +sed 's/^/[codex log] /' "$LOG_FILE" >&2 || true +if [ -s "$LOG_FILE" ]; then + cat "$LOG_FILE" +fi + +if [ "$CODE" -eq 0 ]; then + echo "codex exec exited 0 but wrote no final message." >&2 + exit 1 +fi + +exit "$CODE" diff --git a/skills/prompt-improver/scripts/generate-prompt.sh b/skills/prompt-improver/scripts/generate-prompt.sh index 060fb66..6dcf19a 100755 --- a/skills/prompt-improver/scripts/generate-prompt.sh +++ b/skills/prompt-improver/scripts/generate-prompt.sh @@ -66,7 +66,7 @@ Options: Model resolution order: 1. --model / per-prompt model: token 2. PROMPT_IMPROVER_MODEL or settings.model - 3. settings.default_models[backend] (shipped: sonnet, grok-composer-2.5-fast, gemini-2.5-pro, gpt-5.5) + 3. settings.default_models[backend] (shipped: claude-opus-5, grok-4.5, gemini-2.5-pro, gpt-5.6-terra) 4. Backend CLI default (discouraged) HELP exit 0 @@ -182,7 +182,7 @@ fi # Priority: # 1) model: / settings.model → infer CLI from model family (cross-host OK) # 2) settings.backend when not auto -# 3) host CLI (Claude session → claude + sonnet, Grok → grok + composer, …) +# 3) host CLI (Claude session → claude + claude-opus-5, Grok → grok + grok-4.5, …) # 4) else headless blocked → host bounce # shellcheck disable=SC2207 PREFS=( $(parse_preferred_backends) ) @@ -448,6 +448,20 @@ if command -v jq >/dev/null 2>&1; then unset _unwrapped fi +# Drop CLI narration ahead of the XML (grok --output-format plain prefixes a +# line like "I'll read the full offloaded prompt ..." before ). +# Single awk pass with no early exit — an exiting reader would SIGPIPE the +# producer under `set -o pipefail`. Falls through untouched when no XML is +# present, so validation still reports the real body. +_stripped=$(printf '%s\n' "$GENERATED" | awk ' + started { print; next } + /^[[:space:]]*<[a-zA-Z]/ { started = 1; print } +') +if [ -n "${_stripped:-}" ]; then + GENERATED="$_stripped" +fi +unset _stripped + # --- Validate --- if [ "$SKIP_VALIDATE" = true ] || [ "$SKIP_VALIDATE" = "true" ]; then echo "$GENERATED" diff --git a/skills/prompt-improver/scripts/lib/settings.sh b/skills/prompt-improver/scripts/lib/settings.sh index 04a7cf5..91e7b90 100755 --- a/skills/prompt-improver/scripts/lib/settings.sh +++ b/skills/prompt-improver/scripts/lib/settings.sh @@ -103,10 +103,10 @@ _pi_matches_any_pattern() { } # Built-in fallbacks when jq/settings tables unavailable -_PI_BUILTIN_DEFAULT_MODELS_claude="sonnet" -_PI_BUILTIN_DEFAULT_MODELS_grok="grok-composer-2.5-fast" +_PI_BUILTIN_DEFAULT_MODELS_claude="claude-opus-5" +_PI_BUILTIN_DEFAULT_MODELS_grok="grok-4.5" _PI_BUILTIN_DEFAULT_MODELS_gemini="gemini-2.5-pro" -_PI_BUILTIN_DEFAULT_MODELS_codex="gpt-5.5" +_PI_BUILTIN_DEFAULT_MODELS_codex="gpt-5.6-terra" _builtin_normalize_model_id() { local raw="${1:-}" @@ -122,6 +122,7 @@ _builtin_normalize_model_id() { sonnet) echo "sonnet" ;; haiku-4.5|haiku4.5|claude-haiku-4-5|claude-haiku-4.5) echo "haiku" ;; haiku) echo "haiku" ;; + opus-5|opus5|claude-opus-5) echo "claude-opus-5" ;; opus-4.8|claude-opus-4-8) echo "claude-opus-4-8" ;; opus-4.6|claude-opus-4-6) echo "claude-opus-4-6" ;; opus) echo "opus" ;; @@ -133,7 +134,7 @@ _builtin_normalize_model_id() { gpt5|gpt-5) echo "gpt-5.5" ;; gpt-5.3-codex|gpt5.3-codex) echo "gpt-5.3-codex" ;; gpt-5.2-codex|gpt5.2-codex) echo "gpt-5.2-codex" ;; - codex|openai) echo "gpt-5.5" ;; + codex|openai) echo "gpt-5.6-terra" ;; o4-mini|o4mini) echo "o4-mini" ;; grok-4.5|grok4.5) echo "grok-4.5" ;; grok-4.3|grok4.3) echo "grok-4.3" ;; @@ -374,15 +375,18 @@ get_model_fallback_chain() { case "$low" in *mythos*|mythos) echo "claude-mythos-5 claude-mythos-preview claude-fable-5 fable opus sonnet" ;; *fable*|fable) echo "claude-fable-5 fable opus sonnet" ;; + claude-opus-5|opus-5|opus5) echo "claude-opus-5 opus sonnet" ;; *opus*|opus) echo "opus sonnet" ;; *sonnet*|sonnet) echo "$primary sonnet" ;; *haiku*|haiku) echo "$primary haiku sonnet" ;; *sol*|gpt-5.6) echo "gpt-5.6-sol gpt-5.6-terra gpt-5.6-luna gpt-5.5" ;; *terra*) echo "gpt-5.6-terra gpt-5.6-luna gpt-5.5" ;; *luna*) echo "gpt-5.6-luna gpt-5.5" ;; - gpt-5.5|gpt-5|codex|openai) echo "gpt-5.5" ;; - grok-4.5|grok-4*) echo "grok-4.5 grok-composer-2.5-fast grok-build" ;; - *composer*) echo "$primary grok-composer-2.5-fast" ;; + gpt-5.5|gpt-5) echo "gpt-5.5" ;; + codex|openai) echo "gpt-5.6-terra gpt-5.6-luna gpt-5.5" ;; + # grok models: `grok models` lists only grok-4.5 — composer/grok-build are retired + grok-4.5|grok-4*) echo "grok-4.5" ;; + *composer*) echo "$primary grok-4.5" ;; *gemini*pro*|gemini-2.5-pro) echo "gemini-2.5-pro gemini-2.5-flash" ;; *gemini*flash*|gemini-2.5-flash) echo "gemini-2.5-flash" ;; *) echo "$primary" ;; diff --git a/skills/prompt-improver/scripts/smoke-test.sh b/skills/prompt-improver/scripts/smoke-test.sh index e1c1b9c..a30245c 100755 --- a/skills/prompt-improver/scripts/smoke-test.sh +++ b/skills/prompt-improver/scripts/smoke-test.sh @@ -133,7 +133,7 @@ echo "[9] default generator models" # shellcheck disable=SC1091 source scripts/lib/settings.sh load_settings -for pair in "claude:sonnet" "grok:grok-composer-2.5-fast" "gemini:gemini-2.5-pro" "codex:gpt-5.5"; do +for pair in "claude:claude-opus-5" "grok:grok-4.5" "gemini:gemini-2.5-pro" "codex:gpt-5.6-terra"; do b="${pair%%:*}" expect="${pair#*:}" got=$(get_default_model_for_backend "$b") @@ -153,8 +153,9 @@ for pair in \ "mythos-5:claude-mythos-5:claude" \ "sonnet:sonnet:claude" \ "gpt-5.5:gpt-5.5:codex" \ - "codex:gpt-5.5:codex" \ - "openai:gpt-5.5:codex" \ + "codex:gpt-5.6-terra:codex" \ + "openai:gpt-5.6-terra:codex" \ + "opus-5:claude-opus-5:claude" \ "gpt-5.6-sol:gpt-5.6-sol:codex" \ "sol:gpt-5.6-sol:codex" \ "terra:gpt-5.6-terra:codex" \ @@ -192,11 +193,19 @@ if echo "$chain_sol" | grep -q 'terra' && echo "$chain_sol" | grep -q 'luna'; th else bad "sol cascade: $chain_sol" fi -if echo "$chain_g45" | grep -q 'composer'; then - ok "grok-4.5 cascade includes composer" +# `grok models` lists only grok-4.5 — composer/grok-build are retired, so the +# chain must not offer them; an explicit composer request still cascades to 4.5. +if [ "$(echo "$chain_g45" | tr -s ' ')" = "grok-4.5" ]; then + ok "grok-4.5 cascade is grok-4.5 only" else bad "grok-4.5 cascade: $chain_g45" fi +chain_comp=$(get_model_fallback_chain "grok-composer-2.5-fast") +if echo "$chain_comp" | grep -q 'grok-4.5'; then + ok "composer cascade falls back to grok-4.5" +else + bad "composer cascade: $chain_comp" +fi if is_model_retryable_failure 1 "Error: rate limit exceeded for model"; then ok "retryable rate-limit detection" else @@ -279,19 +288,19 @@ if [ "$_nh" -eq 3 ] && grep -q 'HOST_BOUNCE:NO_HEADLESS' /tmp/pi-nohost.out; the else bad "expected NO_HEADLESS exit 3, got $_nh: $(head -5 /tmp/pi-nohost.err)" fi -# Host claude with no model → sonnet (only check selection line if claude on PATH) +# Host claude with no model → claude-opus-5 (only check selection line if claude on PATH) if command -v claude >/dev/null 2>&1; then export PROMPT_IMPROVER_HOST=claude set +e bash scripts/generate-prompt.sh --mode plan --raw-input "x" --skip-validate >/tmp/pi-host.out 2>/tmp/pi-host.err _hc=$? set -e - if grep -q 'host CLI (claude)' /tmp/pi-host.err && grep -q 'model: sonnet' /tmp/pi-host.err; then - ok "claude host → sonnet default selection" + if grep -q 'host CLI (claude)' /tmp/pi-host.err && grep -q 'model: claude-opus-5' /tmp/pi-host.err; then + ok "claude host → claude-opus-5 default selection" else # may fail generation (rate limit) but selection reason should appear - if grep -qE 'host CLI \(claude\).*sonnet|model: sonnet' /tmp/pi-host.err; then - ok "claude host → sonnet default selection" + if grep -qE 'host CLI \(claude\).*claude-opus-5|model: claude-opus-5' /tmp/pi-host.err; then + ok "claude host → claude-opus-5 default selection" else bad "claude host selection: $(head -8 /tmp/pi-host.err)" fi