[SYMPP-3107] Retry read-only requests after backend recovery - #625
Conversation
Summary: - retry non-tool-call requests after successful backend recovery - cover Node and PowerShell backend-only read recovery in the launcher smoke Rationale: - read-only requests are safe to repeat even when a dead transport may have received them - ambiguous tool calls must remain indeterminate and must not replay Tests: - focused Node and Windows PowerShell backend-only recovery cases - focused ambiguous tool-call and 10-client owner-loss cases - make -C elixir all - full launcher suite attempted once; unchanged owner-loss case failed once before passing in isolation Co-authored-by: Codex <codex@openai.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
|
@codex review |
![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: 4be1534d51
ℹ️ 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".
Summary: - classify tool calls inside JSON-RPC batches in both bridge implementations - prove Node and PowerShell do not replay a batch containing a mutation Rationale: - a top-level method check treated every JSON-RPC batch as read-only - assigning the PowerShell payload preserves array enumeration on 5.1 Tests: - focused Node and Windows PowerShell ambiguous batch recovery cases - focused Node and Windows PowerShell backend-only read recovery cases - focused singleton ambiguous tool-call recovery case Co-authored-by: Codex <codex@openai.com>
Summary: - remove mixed-batch recovery handling from both bridge implementations - retain focused singleton read recovery and mutation ambiguity coverage Rationale: - the shipped HTTP /mcp endpoint rejects arrays before member execution - source-only direct STDIO batch support does not use the recovery bridges Tests: - three focused HTTP endpoint batch rejection tests - focused Node and Windows PowerShell backend-only read recovery cases - focused singleton ambiguous tool-call recovery case Co-authored-by: Codex <codex@openai.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
518edc8
into
integration/mcp-first-caller-lifecycle
1 participant
Context
Backend-only loss can leave a reused connection in an ambiguous delivery state. Read-only MCP requests must recover, while tool calls must remain indeterminate.
TL;DR
Retry read-only MCP requests after backend recovery without replaying ambiguous tool calls.
Summary
tools/callrequests after successful Node and PowerShell recovery.tools/callresults indeterminate withreplayed=false.Alternatives
/mcprejects arrays before member execution.Test Plan
make -C elixir allmake -C elixir ci-full(not required by this WorkPackage)tools/listrecovery with final-zero cleanuptools/call: one mutation andreplayed=falsetools/call, including lossy parse: one mutation andreplayed=false/mcpbatch-rejection testsowner_lossrecoveryowner_loss; its exact case passed in isolation