Conversation
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Claude <noreply@anthropic.com>
Remove an unreachable legacy-session cleanup clause detected by the future-version warnings-as-errors lane. Co-Authored-By: Claude <noreply@anthropic.com>
Address PR review findings across legacy HTTP sessions, client negotiation, subscriptions, capability enforcement, close/error handling, origin validation, conformance CI, and documentation evidence.\n\nCo-Authored-By: Claude <noreply@anthropic.com>
feat: complete MCP 2.0 dual-protocol SDK
feat: add SEP-2640 Skills Over MCP extension
…p-apps # Conflicts: # README.md
feat: add stable MCP Apps support
feat: harden SDK boundaries and add browser interoperability evidence
* fix: close exhaustive review findings * fix: address PR review feedback * test: cover abnormal post task exits * fix: preserve flat app metadata compatibility * fix: validate compatibility app resource URIs * fix: preserve URI field presence during validation * chore: add reproducible development bootstrap * fix: close final PR review findings
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: 2c5a410377
ℹ️ 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".
| def close(client) do | ||
| GenServer.call(client, :close) | ||
| catch | ||
| :exit, _ -> :ok | ||
| :exit, {:noproc, _call} -> :ok | ||
| :exit, reason -> {:error, {:close_failed, reason}} |
There was a problem hiding this comment.
When a legacy HTTP session DELETE or stdio process-tree cleanup takes more than GenServer's default 5-second call timeout, close/1 returns {:error, {:close_failed, ...}} even though the client is still executing its bounded shutdown; the HTTP transport explicitly permits up to its 60-second request budget and stdio may use shutdown_timeout + 4_000. Use a call timeout that covers the configured cleanup budget (or :infinity, since the inner operations are bounded), as the transport-level close functions already do.
Useful? React with 👍 / 👎.
| state, | ||
| from, | ||
| Methods.tools_call(), | ||
| name_args(name, arguments), |
There was a problem hiding this comment.
When a tool call carrying meta: receives a recognized custom-header mismatch, the first request includes that metadata, but the transparent retry is rebuilt solely with name_args/2. The retried operation therefore silently drops caller-supplied _meta values, which can change tracing, extension, or request-specific server behavior precisely on the recovery path; retain the original metadata through the refresh state and apply it here.
Useful? React with 👍 / 👎.
|
|
||
| case result do | ||
| :ok -> | ||
| handle = SubscriptionHandle.new(operation.id, operation.worker) |
There was a problem hiding this comment.
For handles returned by listen_subscriptions/3, this constructor runs inside the client GenServer, so the monitor's :DOWN message is delivered to the client rather than to the caller that receives the handle. Consequently SubscriptionHandle.take_down/1 can never observe that monitor, and if the worker exits abnormally between calls the consumer reports generic :closed (and close/2 may claim success) instead of preserving the exit reason promised by the handle API; establish the monitor in the consuming process or route the terminal reason through the worker/client.
Useful? React with 👍 / 👎.
1 participant
Summary\n- harden callback, subscription, and legacy session lifecycle ownership\n- add bounded protocol payload, admission, and cleanup safeguards\n- align conformance evidence and Req compatibility CI documentation\n\n## Verification\n- mix format --check-formatted\n- mix compile --warnings-as-errors\n- mix credo --strict\n- mix test --max-cases 1\n- elixir scripts/validate_conformance_ledgers.exs\n\n## Review\n- elixir-phoenix phx-review: pass after remediation\n- vibin review-pr: all actionable findings addressed and rechecked