diff --git a/ods/Makefile b/ods/Makefile index 3e32c98db..1a6bcef07 100644 --- a/ods/Makefile +++ b/ods/Makefile @@ -88,6 +88,7 @@ test: ## Run unit and contract tests @echo "" @echo "=== ods-cli pipefail tolerance ===" @bash tests/test-ods-cli-pipefail-tolerance.sh + @bash tests/test-chat-error-exit-parity.sh @echo "" @echo "=== offline model validation ===" @python3 tests/test-offline-model-validation.py diff --git a/ods/installers/macos/ods-macos.sh b/ods/installers/macos/ods-macos.sh index a650a348b..a06a49ae5 100755 --- a/ods/installers/macos/ods-macos.sh +++ b/ods/installers/macos/ods-macos.sh @@ -983,8 +983,16 @@ cmd_chat() { return 1 } + local content + if ! content=$(printf '%s' "$response" | jq -er '.choices[0].message.content'); then + local api_error + api_error=$(printf '%s' "$response" | jq -r '.error.message // "unparseable response"') + ai_err "LLM error: ${api_error}" + return 1 + fi + echo "" - echo "$response" | jq -r '.choices[0].message.content // .error.message // "Error: no response"' + printf '%s\n' "$content" echo "" } diff --git a/ods/installers/windows/ods.ps1 b/ods/installers/windows/ods.ps1 index 9b23411f4..7b1066c9a 100644 --- a/ods/installers/windows/ods.ps1 +++ b/ods/installers/windows/ods.ps1 @@ -2411,13 +2411,20 @@ function Invoke-Chat { -Method POST -Body $body -ContentType "application/json" -TimeoutSec 120 if ($resp.choices -and $resp.choices[0].message) { + $content = [string]$resp.choices[0].message.content + if ([string]::IsNullOrWhiteSpace($content)) { + throw "Chat response did not contain assistant content" + } Write-Host "" - Write-Host $resp.choices[0].message.content + Write-Host $content Write-Host "" + } else { + throw "Chat response did not contain assistant content" } } catch { Write-AIError "Chat request failed: $_" Write-AI "Is llama-server running? Try: .\ods.ps1 status" + exit 1 } } diff --git a/ods/tests/test-chat-error-exit-parity.sh b/ods/tests/test-chat-error-exit-parity.sh new file mode 100644 index 000000000..9c6390fbc --- /dev/null +++ b/ods/tests/test-chat-error-exit-parity.sh @@ -0,0 +1,39 @@ +#!/usr/bin/env bash +# Contract: chat commands must return nonzero for transport and response errors. + +set -euo pipefail + +root_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +linux_cli="$root_dir/ods-cli" +macos_cli="$root_dir/installers/macos/ods-macos.sh" +windows_cli="$root_dir/installers/windows/ods.ps1" + +linux_chat="$(awk '/^cmd_chat\(\)/,/^}/' "$linux_cli")" +macos_chat="$(awk '/^cmd_chat\(\)/,/^}/' "$macos_cli")" +windows_chat="$(awk '/^function Invoke-Chat/,/^}/' "$windows_cli")" + +grep -q "jq -er '.choices\[0\].message.content'" <<< "$linux_chat" || { + printf '[FAIL] Linux chat no longer rejects error-only response payloads\n' >&2 + exit 1 +} +grep -q "jq -er '.choices\[0\].message.content'" <<< "$macos_chat" || { + printf '[FAIL] macOS chat does not reject error-only response payloads\n' >&2 + exit 1 +} +grep -q 'Chat response did not contain assistant content' <<< "$windows_chat" || { + printf '[FAIL] Windows chat accepts a response without assistant content\n' >&2 + exit 1 +} + +windows_catch="$(awk ' + /^function Invoke-Chat/ { in_chat=1 } + in_chat && /} catch {/ { in_catch=1 } + in_catch { print } + in_catch && /^ }$/ { exit } +' "$windows_cli")" +grep -q 'exit 1' <<< "$windows_catch" || { + printf '[FAIL] Windows chat swallows transport/response errors with exit 0\n' >&2 + exit 1 +} + +printf '[PASS] chat errors return nonzero on every platform\n' diff --git a/ods/tests/test-macos-cli-mode-routing.sh b/ods/tests/test-macos-cli-mode-routing.sh index c1612c067..93a5c1701 100755 --- a/ods/tests/test-macos-cli-mode-routing.sh +++ b/ods/tests/test-macos-cli-mode-routing.sh @@ -43,6 +43,11 @@ cat > "$MOCK_BIN/jq" <<'MOCK_JQ' #!/usr/bin/env bash if [[ "${1:-}" == "-n" ]]; then printf '%s\n' '{"model":"default","messages":[{"role":"user","content":"test"}]}' +elif [[ "${TEST_CHAT_API_ERROR:-0}" == "1" ]]; then + if [[ " $* " == *" -e"* || " $* " == *" -er "* ]]; then + exit 1 + fi + printf '%s\n' 'model unavailable' else printf '%s\n' 'mock answer' fi @@ -57,6 +62,10 @@ for arg in "$@"; do exit 0 fi done +if [[ "${TEST_CHAT_API_ERROR:-0}" == "1" ]]; then + printf '%s\n' '{"error":{"message":"model unavailable"}}' + exit 0 +fi printf '%s\n' '{"choices":[{"message":{"content":"mock answer"}}]}' MOCK_CURL @@ -126,6 +135,15 @@ fi [[ ! -s "$CURL_LOG" ]] || fail "cloud chat contacted LiteLLM without a key" pass "cloud chat fails before transport when authentication is unavailable" +write_local_env +: > "$CURL_LOG" +if TEST_CHAT_API_ERROR=1 run_cli chat "API error" > "$TMP_DIR/chat-error.out" 2>&1; then + fail "macOS chat returned success for an API error payload" +fi +grep -Fq 'LLM error: model unavailable' "$TMP_DIR/chat-error.out" \ + || fail "macOS chat did not surface the API error message" +pass "macOS chat fails on an API error payload" + write_local_env : > "$CURL_LOG" run_cli status >/dev/null