Repository navigation
fix: stop blank exception messages from masking turn failures - #685
Merged
Merged
Conversation
An exception with an empty str() (e.g. bare RuntimeError, or a
PipelineError(str(exc)) wrapper around one) produced TurnOutcome.error ==
"". The runtime store's non-empty error invariant then raised
ValueError("error must be non-empty") during update_agent_run, masking
the real provider failure (observed live: codex OAuth refresh 403
surfaced only as the masking ValueError).
- adapter/types.py: add exception_error_message() falling back to the
exception type name (plus chained cause) when str(exc) is blank
- adapter/pipeline.py, runs/lifecycle.py, tools/subagent.py: use it at
every TurnOutcome.error / run-error construction site
- stores/runtime_store.py: _normalize_optional_error() maps blank error
strings to None at the update_agent_run write boundary (JSONL/SQLite/PG)
as defense-in-depth
- postmortem: add PM-0026 pattern
Provider-agnostic: any exception with empty str() hit this, not just codex.
Contributor
📝 WalkthroughWalkthroughThe change adds exception-message normalization, applies it to adapter and lifecycle error handling, normalizes blank errors before runtime-store persistence, adds regression tests, and documents the issue in postmortem PM-0026. ChangesException error normalization
Lifecycle and runtime-store persistence
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@coding-agent/src/coding_agent/adapter/types.py`:
- Around line 46-48: Update exception_error_message to track visited exception
identities while traversing explicit __cause__ links, stopping or falling back
to a non-recursive message when a cycle is detected, including self-referential
and two-exception cycles. Add regression tests covering both cycle shapes and
ensure normal chained-cause formatting remains unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f6d7e85f-cd3b-48d1-9e07-4ec3a4f2c8cb
📒 Files selected for processing (12)
coding-agent/postmortem/index.yamlcoding-agent/postmortem/patterns/PM-0026-never-persist-blank-exception-messages-as-run-errors.mdcoding-agent/src/coding_agent/adapter/pipeline.pycoding-agent/src/coding_agent/adapter/types.pycoding-agent/src/coding_agent/runs/lifecycle.pycoding-agent/src/coding_agent/stores/runtime_store.pycoding-agent/src/coding_agent/tools/subagent.pycoding-agent/tests/coding_agent/test_adapter_types.pycoding-agent/tests/coding_agent/test_jsonl_runtime_store.pycoding-agent/tests/coding_agent/test_pipeline_adapter.pycoding-agent/tests/coding_agent/test_run_lifecycle.pycoding-agent/tests/coding_agent/test_sqlite_runtime_store.py
delete-cloud
added a commit
that referenced
this pull request
Jul 31, 2026
…686) * docs(spec): webui codex oauth device-flow login design * docs(spec): multi-account codex oauth design * docs(spec): disconnect deletes local record only, no remote revoke * feat(webui): codex OAuth device flow + multi-account support - SettingsPanel: new CodexAccountsCard listing connected accounts (label/email/plan badge) with confirm+disconnect, add-account device flow (label input, verification link, click-to-copy user code, 3s polling until authorized/error/expired, retry), in-flight flow cancel, pending-flow recovery on reload, and a not-supported note when the server lacks the /oauth/* endpoints (404). - Header: append connected codex:<label> accounts to the provider dropdown (static list on fetch failure); codex providers get gpt-5.5/5.4/5.2 datalist presets instead of a live model fetch. - lib/api.ts: 6 oauth client methods with defensive payload parsing; lib/types.ts: CodexFlow/CodexFlowStart/OAuthAccount types. * feat(server): multi-account codex OAuth device flow for webui - oauth/store.py: default_auth_path() with CODING_AGENT_OAUTH_AUTH_PATH env override; OAuthStore() and oauth CLI resolve through it - server/oauth_flows.py: in-memory flow registry + background poll tasks (pending/authorized/error/expired/cancelled, 10min TTL); label from explicit request or id_token (email first, account-id short code, -2/-3 suffix on conflict); local exception_error_message helper (stands in for unmerged #685) - server/http_server.py: POST /oauth/codex/start, GET /oauth/codex/flows{,/{id}}, POST /oauth/codex/flows/{id}/cancel, GET /oauth/accounts, DELETE /oauth/accounts/{key} (local delete, no remote revoke); 400 at session create for unconnected codex:<label> - plugins/llm_provider.py: codex:<label> routing — StoreBackedCodex- TokenSource parameterized by provider key, refresh writes back same key - server/schemas.py: CreateSessionRequest.provider accepts ProviderName literals plus codex:<label> (^[a-z0-9][a-z0-9-]{0,30}$) - tests: tests/ui/test_oauth_flows.py (mocked CodexOAuthClient), plugin codex:* routing tests, store env-path tests - docs: web-client-api-contract.md gains the oauth endpoints section * fix(webui): stop polling lost codex oauth flows Review P2: the flow poll catch swallowed every error, so a flow the server forgot (restart cleared the in-memory registry, TTL prune) or a failing auth path was retried every 3s until page close. Now a 404 marks the flow expired with a server-lost note, and other errors give up after 5 consecutive failures. Expired rows render flow.error when present.
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.
Root cause
Observed live (o6n): a codex turn failed and the session showed
failure_details: "Agent turn failed: error must be non-empty", with pod logs showingValueError: error must be non-emptyfromruntime_store.py__post_init__duringRuntimeTurnFinalizer.complete→finish_run→update_agent_run. The real failure (codex OAuth refresh 403) was completely masked.Traced path:
PipelineError(str(exc), stage=...)(src/agentkit/runtime/pipeline.py:535). If the original exception'sstr()is empty, the wrapper'sstr()is empty too.PipelineAdapter.run_turnbuilt the outcome with bareerror=str(exc)(src/coding_agent/adapter/pipeline.py:236) — no fallback, soTurnOutcome.error == "". Same pattern existed inruns/lifecycle.py(handle_fatal/handle_generic) andtools/subagent.py.RuntimeTurnFinalizer.complete→finish_run→update_agent_run, where_require_non_empty("error", ...)raisedValueError("error must be non-empty"), which then became the user-visible failure — the original error was lost entirely.Provider-agnostic? Yes. Verified empirically that an
httpx.HTTPStatusError(the codex 403) survives the full provider → pipeline → adapter path with its message intact; the bug bites for any exception whosestr()is empty/whitespace (bareRuntimeError(), wrappers re-raised without a message, etc.), regardless of provider. The empty string the pod wrote must have come from such an intermediate exception; the code had no guard against it anywhere along the path.Fix
exception_error_message()insrc/coding_agent/adapter/types.py: returnsstr(exc)when non-blank, otherwise falls back to the exception type name, unwrapping a chained__cause__(PipelineError: RuntimeError). Used at everyTurnOutcome.error/ run-error construction site:adapter/pipeline.py,runs/lifecycle.py(handle_fatal,handle_generic, incl.last_failure_details),tools/subagent.py. No empty error can reach persistence again, and the user-visible error is always informative._normalize_optional_error()insrc/coding_agent/stores/runtime_store.py:update_agent_run(JSONL/SQLite/PG) normalizes blank error strings toNoneat the write boundary, so a blank error can never trip theAgentRunRecordnon-empty invariant on write or read-back.Tests
test_adapter_types.py: unit tests pinning the fallback (empty, whitespace, verbatim message, chained cause, httpx 403 message preserved).test_pipeline_adapter.py: provider stream raisingRuntimeError()(empty str) →outcome.error == "PipelineError: RuntimeError".test_run_lifecycle.py: error handler with empty-message exception records non-empty error; end-to-end repro — empty-message exception outcome → finalizer backed by realSQLiteRuntimeStore→ run finishesfailedwitherror == "RuntimeError", noValueError.test_sqlite_runtime_store.py/test_jsonl_runtime_store.py: blank error normalizes toNone, no raise.Verification
uv run pytest tests/coding_agent -q: 1620 passed, 20 xfaileduv run pytest tests/agents tests/coding_agent/subagents: passruff check/ruff format --checkon all changed files: cleanNot merged — review welcome.
Summary by CodeRabbit
Bug Fixes
Documentation