feat: add remote MCP transport reliability - #23
Conversation
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
![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: 04790e36ac
ℹ️ 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".
| const hasFailures = fingerprintedFindings.some( | ||
| (finding) => finding.severity === "fail" | ||
| ); | ||
| ) || (options.requireRemoteReliability === true && !remoteReliabilityGatePassed(runtimeResult?.scorecard)); |
There was a problem hiding this comment.
When reliability is warn or is unavailable, this expression makes validatePlugin fail without adding a failing finding. Every normal check path subsequently passes the result through applyDoctorConfig, which recomputes status solely from findings; with the default failOnWarnings: false, it downgrades the result to warn or pass and exits successfully. Consequently, check and the GitHub Action do not actually enforce --require-remote-reliability for the outcomes the flag is intended to block.
Useful? React with 👍 / 👎.
| if (resumed.sessionRestarted) { | ||
| scorecard.resumability = "skipped"; | ||
| } else { |
There was a problem hiding this comment.
If an initial event supplies an ID, the resume request returns 404, reinitialization succeeds, and the replacement-session GET then returns an invalid response such as HTTP 500, sessionRestarted causes that response to bypass all status, content-type, and framing classification. The scorecard records restart as passing and resumability as skipped, so the overall reliability result can pass despite the failed replacement GET; classify this response before deciding that only resumability should be skipped.
Useful? React with 👍 / 👎.
| } | ||
| lineStart = index + 1; | ||
|
|
||
| if (line.length === 0) { |
There was a problem hiding this comment.
Any empty line currently reports a complete event even when the preceding frame contains only a comment, id, retry, or no fields at all. Such frames do not dispatch an SSE event without data, so a common : keepalive\n\n stream is incorrectly marked as passing GET/disconnect reliability, and an empty resume response is marked as passing resumability. Track whether the frame contains event data and keep observing otherwise.
Useful? React with 👍 / 👎.
1 participant
Summary
Verification