Repository navigation
Retry failed turns automatically on recoverable errors - #45
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 87b13e456d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| map[string]any{"type": "input_text", "text": retryMessage(attempt+1, limit, decision)}, | ||
| }}}, | ||
| }) | ||
| if _, err := host.SubmitInputs(submit, tenant, turn.SessionID, "auto-retry:"+turn.ID, []sessions.Input{{Kind: "message", Payload: payload}}); err != nil { |
There was a problem hiding this comment.
When OAC_TURN_AUTO_RETRIES is enabled and an eligible failure occurs, this persists Core's control prompt through the normal input path as a role: user message. It therefore creates a public user Item and root Turn, changes Session history and status despite no application request, and provides no provenance by which clients can distinguish the synthetic input. The Sessions contract defines these messages as client-admitted input, so the retry needs a typed representation below the public input boundary rather than inventing new public semantics.
AGENTS.md reference: AGENTS.md:L49-L50
Useful? React with 👍 / 👎.
| map[string]any{"type": "input_text", "text": retryMessage(attempt+1, limit, decision)}, | ||
| }}}, | ||
| }) | ||
| if _, err := host.SubmitInputs(submit, tenant, turn.SessionID, "auto-retry:"+turn.ID, []sessions.Input{{Kind: "message", Payload: payload}}); err != nil { |
There was a problem hiding this comment.
If an application submits a follow-up after ListTurns returns but before this call acquires the Session lock, the latest-Turn check passes even though the application has now created a new queued or running Turn. SubmitInputs then treats the stale retry message as steering for that unrelated Turn—or starts another Turn if the follow-up already completed—so a user request receives an internal instruction meant for the preceding failure. The failed-Turn/latest-state precondition must be checked in the same transaction that admits the retry.
Useful? React with 👍 / 👎.
1 participant
Summary
When a turn fails with a recoverable error (upstream 400 format problems, tool or parameter errors, transient 5xx/429/connection errors), Core now submits a follow-up message so the conversation continues instead of stopping. If the same failure repeats, the retry message adds progressively stronger guidance so the model can adjust its tool call or parameters.
This replaces the earlier daemon-side "rebuild the thread inside the same turn" approach (#42, now draft). It stays inside
services/core: no daemon, protocol,delivery.go,finishRunor state machine changes.Behaviour
maybeAutoRetryinspects a failed turn.error_code=engine_failedandengine_error_codeis one of: empty,invalid_request,server_error,server_overloaded,rate_limit_exceeded,request_timeout,connection_failed. HTTP 401/402/403/404 onconnection_failedis not retried. Auth, billing, quota, model-not-found, context-too-long, policy, cancel, disconnect andinput_*failures are never retried.messages, request ending with a model turn, tool argument errors (these add a parameter hint). Encrypted reasoning, image-unsupported andtool_resultimage failures are not retried because the bad content already lives in the daemon's native history and Core cannot edit it.auto-retry:<failed turn id>.[auto-retry N/M], so no new storage. Escalation: N=1 "please continue" (plus a parameter hint for tool-argument errors), N=2 "try a different approach", N=3 "stop calling the failing tool".Configuration
OAC_TURN_AUTO_RETRIES: integer 0..5, default0(off). Invalid values fail startup. Registered as settingcore.turn_auto_retriesand wired through compose, the Kubernetes README,deploy.ymland both configuration docs. Roll back by setting it to0and restarting Core; there is no migration.Testing
Passing:
make build-core,go vet(execution, processconfig), processconfig tests, 9 new retry tests,python3 deploy/compose/test_compose.py.Not verified:
OAC_TEST_DATABASE_URLlocally).golangci-lintwas not run.TestSchedulingObservationBoundedConcurrentAndOwnerLocalflakes on plainorigin/betatoo (1 of 3 runs). The other failing packages (cmd/sandbox-node,internal/sandbox/docker,internal/sandbox/microsandbox) fail identically onorigin/beta.Rollout
beta, deploy with the switch off.OAC_TURN_AUTO_RETRIES=3on beta, trigger a real failure, confirm[auto-retry 1/3]appears and the conversation continues.decisionandfingerprintlog fields and token usage before deciding on wider rollout.Known limits and follow-ups