fix(chat): propagate API failures across CLIs - #3166
Open
tang-vu wants to merge 1 commit into
Open
Conversation
This was referenced Aug 25, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why this matters
The macOS chat command currently renders an API error message as ordinary output and exits zero when an OpenAI-compatible backend returns an error-only JSON payload with HTTP 200. The Windows command catches transport or JSON failures, prints an error, and also falls through with exit zero; it likewise accepts a 2xx response with no assistant content. Shell scripts and operators therefore cannot distinguish a generated answer from a failed request by exit status.
This aligns both CLIs with the existing Linux behavior. macOS requires assistant message content via
jq -e, surfaces the backend error message, and returns nonzero when content is absent. Windows validates non-empty assistant content, turns malformed success envelopes into the same failure path, and exits nonzero from its catch boundary.Behavioral invariant:
chatexits zero only after receiving non-empty assistant content; transport failures and API error or malformed payloads must be visible failures.Overlap check
Searched open and closed PRs for
chat exit code macOS Windows,ods chat error response, and the changed production functions inods/installers/macos/ods-macos.shandods/installers/windows/ods.ps1. No PR covers this behavior.Linux
ods-clialready usesjq -erand exits througherror()for missing assistant content. The new parity contract treats that implementation as the established boundary and leaves it unchanged.Regression test
tests/test-macos-cli-mode-routing.shnow drives the real macOS chat command with a successful HTTP response containing only the backend errormodel unavailable. Before the fix it printed that message and returned zero; the regression requires a nonzero exit and an actionableLLM error: model unavailablereceipt.tests/test-chat-error-exit-parity.shverifies all three platform implementations reject error-only or missing-content responses and that the Windows catch path exits nonzero. Existing local/cloud route and authentication assertions continue to run around the new negative case.Validation
bash tests/test-macos-cli-mode-routing.sh? passed all local, cloud, auth, routing, and API-error casesbash tests/test-chat-error-exit-parity.sh? passed Linux/macOS/Windows contractbash tests/test-ods-cli-pipefail-tolerance.sh? 16 passed, 0 failed, 0 skippedmake lint? passedinstallers/windows/ods.ps1? passedgit diff --check? passedThe macOS regression executes the real CLI with boundary shims. Windows validation is parser and static-contract based because no live Windows inference endpoint was used; it proves control-flow placement, not a real network request. Reverting restores the false-success exit codes and needs no state migration.
Batch compatibility
This PR was validated on synthetic integration head
9af795fc, which applies #3158 through #3167 in numeric order onupstream/main(6ff9b4fc). Combinedmake lint,make test,make smoke,make simulate, and all 418 BATS cases passed (one root-specific permission assertion skipped by design).Recommended merge order: #3158 ? #3159 ? #3160 ? #3161 ? #3162 ? #3163 ? #3164 ? #3165 ? #3166 ? #3167. The only manual reconciliation observed was the adjacent Makefile test insertion shared by #3164 and #3166; retain both
test-unix-restart-recreate-env.shandtest-chat-error-exit-parity.shlines. Production code merged automatically across the full batch.