fix(mcp): close the review findings on the mcp 2.x port

[mocha06]

Stacked on #547, targeting refactor/543-mcp-2x-port so the port and these fixes stay separately reviewable.

Motivation

#547 was reviewed across four independent lenses: behavior parity, the request-scoped plumbing rewrite, test-suite integrity, and runtime validation against production. The port itself held up well, and this branch does not relitigate it. What remained at f5b28ca were three defects the suite could not see and a set of smaller gaps the reviews left open.

The one that mattered most: a default mcp 2.x Client uses mode="auto", opens with the enveloped server/discover, and lands on protocol revision 2026-07-28. That revision has no server-to-client back channel, so create_card and fill_card_phase_fields returned a JSON-RPC protocol error instead of a tool result. #547's own falsification check drives a client offering protocolVersion in initialize, which is the handshake leg and does correctly negotiate 2025-11-25, so it could not see this. Nothing in the suite asserted a negotiated revision either, which is why the failure surfaced only as a cluster of confusing elicitation failures.

Outcome

Elicitation is gated on the back channel rather than the advertised capability. supports_elicitation now requires ServerSession.can_send_request, which is the SDK's own gate, and _elicit_field_details absorbs NoBackChannelError for the residual cases the guard cannot see. Both tools fall back to the caller's supplied fields, the same path they already took for a client advertising no elicitation. This also covers stateless HTTP, which has no back channel at any revision, so a version-string check would not have been enough.

A short-circuiting middleware no longer leaks a 2026-era field onto a legacy connection. The SDK shapes a handler's result per negotiated revision inside call_next, so a middleware returning without awaiting it was never shaped, and CallToolResult's resultType default reached the wire on connections that negotiated 2025-11-25. short_circuit_error now returns the wire dict shaped by the SDK's own serialize_server_result, so there is no hardcoded assumption about which keys belong at which revision.

Per-session credential binding discriminates per user. mcp 2.0 compares a (client_id, issuer, subject) triple to decide whether a request may use an existing session, and only client_id was supplied. For the hosted server that is the single public pipefy-mcp OAuth client every end user authorizes through, so the check could not tell two users apart and either could present the other's mcp-session-id. The JWT sub and iss are now carried on the fields the comparison reads.

Two previously unasserted facts are pinned: the negotiated protocol revision and the advertised serverInfo. Each expected literal is cross-checked against mcp_types.version, so a failure distinguishes "this server's negotiation changed" from "the SDK's revision set changed".

Smaller, all verified against the tree rather than taken from the reviews: starlette is now declared where four modules import it, instead of relying on it arriving transitively through the SDK. A tools/call that names no tool is logged under <unnamed> rather than sharing a blank dashboard bucket with real tool names, while ctx.tool_name stays the raw view of what the client sent. The last FastMCP references outside the changelog are gone, including a documented FASTMCP_LOG_LEVEL variable that does not exist. runtime.py no longer claims to own the transport allowlist it stopped holding. And the suite is back to a clean warning baseline behind one scoped ignore, matched on the message rather than the class so the SDK's other deprecation subjects still surface.

The changelog entry for the moved transport arguments is corrected: it listed port, which only ever reached the uvicorn bind, and omitted json_response, which genuinely moved. That omission matters for an embedder that calls streamable_http_app() directly and expects plain JSON rather than SSE framing.

Notes

Considered and rejected: pinning the server to 2025-11-25. It is the obvious fix for the negotiation finding and it is wrong. Measured on both transports: over streamable HTTP a mode="auto" client falls back cleanly, but on a duplex connection the connect fails outright with -32601, because send_discover stamps the modern envelope and the opening request permanently locks the connection's protocol era. This package ships stdio as the default for local installs, so pinning would have fixed hosted HTTP callers and broken every default local client. There is also no supported API for it; the only lever is the private _lowlevel_server, which the SDK marks with a TODO to make public.

Three corrections to the review that prompted this branch, recorded because they are wrong in a public comment on #547. The read-side back-channel concern is inert: Context.elicit passes related_request_id, and the SDK routes such messages to that request's own POST stream, so holding another session's id does not expose its elicitation. The real variant is write-side response injection, currently masked by the wrapper's json_response=True, which is itself one of the calls that breaks under 2.0. The review also named pipe_config and service_account as the eliciting tools; neither elicits, only create_card and fill_card_phase_fields do, and that misattribution came from #547's own _mcp_compat docstring, corrected here. Finally, the isError:true line in the review's wire capture is from a raised exception, not a Pipefy-style tool_error envelope, which is a successful call reporting a business error and carries isError:false.

Known gap, deliberately not fixed here. On a 2026-07-28 connection the SDK also stamps _meta.serverInfo inside _serialize, which a short-circuit skips exactly like resultType did. Unlike resultType it cannot be fixed from short_circuit_error, because the stamp needs a server-scoped value unreachable from a ToolCallContext. Not reachable today given what the deployment negotiates, but whoever adopts the modern revision should know a governance stop will silently omit it.

Out of scope, and still the prerequisite for any deployment. The hosted wrapper breaks at three call sites under 2.0, and the shortcut fix (deleting the two app.settings assignments and leaving streamable_http_app() argument-less) produces a pod that passes its readiness probe while rejecting every proxied MCP request with 421. Combined with the upstream session-leak issue, that state also strands memory per rejected request. That work belongs in the wrapper repo.

Gates on this branch: ruff check and format across all five src trees and the changed files, lint-imports 2 kept 0 broken, bump_version.py verify, uv sync --locked, uv build --all-packages --wheel, and pytest -m "not integration" at 4029 passed against a 4008 baseline.

[adriannoes]

Reviewed at ef82857b and re-verified with a second pass: fresh worktree at head, every finding re-checked adversarially, plus the CI gates this PR gets no Actions run for (base is not main/dev).

Verdict: merge with notes into refactor/543-mcp-2x-port, then #547 into dev. No blocker. One authorization residual worth an explicit decision before #548 is treated as done.

What passes

Gate	Result
ruff check packages/mcp	pass
pytest packages/mcp/tests -m "not integration"	1776 passed, 5 skipped, 4 deselected
Full repo suite (what CI's test step runs)	4029 passed, 5 skipped, 34 deselected, zero warning records
scripts/bump_version.py verify	pass
ruff check + ruff format --check, full tree	pass, 211 files formatted
Banned upward imports (sdk, auth, infra)	pass
lint-imports in packages/mcp	68 files, 216 dependencies, 2 contracts kept, 0 broken
uv build --all-packages --wheel	5 wheels
Clean-venv wheel install, deps resolved from PyPI	installs and both entry points run

The clean-venv install is the one gate that resolves past uv.lock. It pulls uvicorn 0.52.0 (lock: 0.34.0), httpx2 2.9.1 (lock: 2.6.0) and typing_extensions 4.16.0, and the server works on all of them.

I also ran the live smoke that the earlier pass had to skip, launching the server from the worktree at head so provenance is not in question. The initialize handshake negotiates 2025-11-25 with serverInfo {name: pipefy, version: 0.4.0-beta.1} and tools/list returns 187 tools; --toolsets power returns 9; get_tool_categories returns isError=false. Same result from the wheel install. So the protocol and serverInfo pins hold live, not only in tests.

The one thing I would decide before closing #548

_to_access_token maps subject from sub only when it is a str, so sub absent, sub: null, and any non-string sub all become subject=None, while sub: "" and whitespace-only are kept verbatim. Under the hosted single public client (azp=pipefy-mcp) plus one issuer, two bearers that share a shape produce an identical (client_id, iss, subject) triple, and the SDK then serves one user's request on the other's session. Reproduced at head against the real StreamableHTTP manager and the real verifier: ping and tools/list both return 200 with real results on the other principal's session. The sharpest case is two distinct numeric subs (101 vs 102): the IdP does distinguish them and the mapping discards the distinction. Mixed shapes do not collapse (sub absent vs sub: "" correctly gets a 404), and distinct string subs are correctly isolated.

Two things to note in the PR's favour, which is why this is a note and not a change request. First, the base mapping put the claim on PipefyAccessToken.sub and never set the SDK's subject or claims, so principal_components compared (client_id, None, None) and every hosted user of pipefy-mcp shared sessions, distinct well-formed subs included (reproduced against the base mapping). This PR strictly narrows the hole. Second, the token is signature-protected, so nobody forges a sub-less bearer; it takes an IdP that mints one, for both parties, plus knowledge of a uuid4().hex session id.

What makes it still worth a decision: JwtValidator requires only exp, verify_audience defaults to False for the same-audience interim and typ is never checked, so any realm-signed RS256 JWT with iss and exp reaches the mapping. Keycloak lightweight access tokens and the 25+ basic-scope removal are documented ways to get a sub-less access token out of the default issuer, so "the IdP always sends sub" is a configuration fact rather than an invariant. And the same collapsed triple is the SDK's default requestState principal binding (authenticated_principal, installed unconditionally via RequestStateSecurity.ephemeral()), so cross-user replay of sealed elicitation state rides on the same seam, and this PR touches the elicitation paths.

One caveat on the obvious fix: rejecting a missing sub outright would contradict the azp or client_id or sub precedence you documented right above, whose point is to accept a bearer whose only identity claim is client_id. So my suggestion is to split it:

• Reject empty and whitespace-only sub in the mapping. Never a valid RFC 9068 subject, so this is safe on its own.
• For absent or non-string sub, either fail closed for ownership (derive a per-credential discriminator such as jti or a hash of the bearer for subject, so a second bearer of the same client gets a 404 instead of session reuse; note subject is also read by request_log_middleware, so keep it recognizable in logs), or write the waiver down with a test that pins today's behavior for two azp-only bearers and two sub: "" bearers.

Either way, the comment that ends "the comparison then falls back to the remaining components, as the SDK documents" reads as cover for a fail-open. The SDK's fallback is safe when the remaining components identify one principal; six lines above, the same comment explains why they do not here.

Changelog: the polymorphic bullet contradicts itself

CHANGELOG.md:19 still says short_circuit_error returns a CallToolResult model. Line 20, packages/mcp/AGENTS.md:388-389, and the signature (-> dict[str, Any]) all say it returns the wire dict shaped by serialize_server_result. Both bullets are new in this PR, so this is internal rather than leftover 1.x prose. result_is_error's own docstring already has the wording to lift.

Same edit is needed at CHANGELOG.md:14, which still ends "a middleware returns a CallToolResult rather than a types.ServerResult". The seam is typed Awaitable[HandlerResult] now, and a short-circuiting middleware returns a dict. Line 14 is the worse of the two to leave wrong, since it is the bullet enumerating the migration steps a middleware author reads first, and while you are there, that same "two shape changes for a middleware author" list omits the new required ToolCallContext.protocol_version. Since the changelog is the lockstep channel for embedders, these two lines matter more than the lockstep itself.

Embedder lockstep, for the record

Any outside-repo extra_tool_middlewares calling short_circuit_error("...") 1.x-style has to pass ctx. In-repo callers are all updated. Worth noting only because the break, while loud (TypeError: missing 1 required positional argument), fires lazily: it surfaces the first time the short-circuit branch actually executes, so an embedder without deny-path coverage ships it undetected. result_is_error handles both result shapes for a wrapper that inspects the flag.

Small stuff

• The back-channel gate covers one more case than the docs say. supports_elicitation's docstring (mcp_capabilities.py:20-22) and CHANGELOG.md:50 both name stateless HTTP as the non-revision case, but the SDK stamps can_send_request = not is_json_response_enabled (streamable_http.py:239), so a stateful deployment serving json_response=True also loses elicitation, at 2025-11-25 included. Measured through wire_hosted_observability over the real ASGI stack: json_response=False gives True at 2025-11-25, json_response=True gives False. No impact on the served path, since the default is False and run_server never passes it. Worth a clause anyway, because json_response is precisely the flag CHANGELOG.md:16 tells an embedder to pass to streamable_http_app().
• test_protocol_handshake.py:56 carries runtime.transport_security = None in its copied mocked_runtime fixture, the same dead line 658997b removed from test_server.py in this PR for setting an attribute McpRuntime no longer has. The fixture is a MagicMock, so nothing fails and nothing catches it.
• core/runtime.py:88-90, in the for_profile docstring this PR rewrote: "selects the per-profile identity from it (and, for remote, the inbound-auth pair derived from that same resource)" reads as if the identity comes from the parsed resource on both profiles. _local_identity(settings) does not take it, and on the local profile the parsed resource is discarded rather than stored.
• uvicorn is imported and used on the --transport http path (server.py:191) but not declared. It is guaranteed only transitively by mcp, whose requirement carries a sys_platform != 'emscripten' marker. Your own starlette comment in this PR writes the policy that covers it. Nothing breaks today (verified against 0.52.0), so this is consistency. anyio and typing_extensions are the same class, weaker because their transitive guarantees are unconditional.
• CHANGELOG.md:26 and pyproject.toml:69 say 68 ctx.debug/ctx.info calls across 11 tool modules producing ~997 warnings. At head it is 64 calls across 10 modules and 1010 warning records. The 68/11 figure was already wrong at the base (63/10 there). Dropping the exact numbers is probably more durable than chasing them.
• CHANGELOG.md:15 hardcodes serverInfo.version as 0.4.0-beta.1. Correct for this head and for dev, but main already shipped beta.2, and test_protocol_handshake.py asserts against pipefy_mcp.__version__ dynamically, so nothing catches the literal after the next back-merge.
• On revision 2026-07-28, a short-circuited result omits the _meta.serverInfo stamp that ServerRunner._serialize adds on the call_next path, so a modern client sees the stamp on real tool results and not on middleware denials. ToolCallContext has no route to server_info_stamp, so I would just say so in the short_circuit_error docstring, which already explains the shaping obligation, rather than restating the server info.
• create_card and fill_card_phase_fields differ on the no-back-channel path in a way worth one line each in their docstrings: the promise "an interactive form is presented" is false whenever the session cannot carry server-to-client requests, which the client cannot observe. With fields omitted, fill_card_phase_fields returns {"success": true, "message": "No fields to update."} while required phase fields go uncollected, and create_card sends {} on to the API. The gate-False path logs nothing, while the NoBackChannelError absorb at least emits ctx.debug. No test covers elicitation-unavailable plus required-fields-present plus fields-omitted; the four tests in TestElicitationWithoutABackChannel all supply fields.

Cleared

The new tests hold up under mutation. Replacing _mcp_compat's mode="legacy" pin makes exactly seven elicitation tests in test_pipe_tools.py fail, matching the docstring's claim, and makes the handshake test fail with '2026-07-28' == '2025-11-25', so the protocol pin is genuinely can-fail. test_session_ownership.py drives the real StreamableHTTPSessionManager rather than a stub and asserts the same-user 200 control alongside the hijack 404. The filterwarnings entry is load-bearing: test_pipe_tools.py alone emits 198 warnings with it off and 0 with it on.

Security sweep over the diff found nothing: no bearer, claim value, or tool argument value reaches any log (verified with an in-process ASGI harness capturing the structured logger and the root logger at DEBUG), transport_security's diff is docstring-only with enforcement live, session ids stay uuid4().hex with unknown client-supplied ids answered 404, and dropping email/name/groups from the claim copy narrows what request.scope["user"] exposes. Every other Unreleased claim checks out against the code, including the four-modules-import-starlette rationale, the whole lockfile transition list, and the "port is unaffected" note. The starlette constraint is compatible with the SDK's own floor, and the filterwarnings entry is message-scoped: a different MCPDeprecationWarning subject still raises under simplefilter("error").

[mocha06]

Thanks, this was a useful review, and the two changelog contradictions were both mine. Pushed as 9c8bce4. Every gate re-run on the integrated tree: ruff check and format across all five src trees, lint-imports 2 kept 0 broken, bump_version.py verify, uv sync --locked, uv build --all-packages --wheel 5 wheels, and pytest -m "not integration" at 4049 passed, 5 skipped, 34 deselected, zero warning records.

The sub residual: partly fixed, partly waived, and two of the premises do not hold

Reproduced all eleven shapes against the real manager and verifier before touching anything. Your finding is real. Three corrections to how it is framed, all measured:

verify_audience is true in both deployments, not left at the default. PIPEFY_JWT_VERIFY_AUDIENCE: "true" with PIPEFY_JWT_AUDIENCE pinned to the resource URL (pipefy-mcp-server k8s/production/production-oci01.yaml:44-54, staging :45-64). A bearer also needs aud=https://mcp.pipefy.com/mcp, which only the hand-made audience mapper emits, so "any realm-signed RS256 JWT with iss and exp reaches the mapping" is not the deployed posture.

Neither Keycloak mechanism you cite exists on our issuer. Prod and staging run 21.1.2 (keycloak/Dockerfile.production:1; prod tag v0.3.10, staging v0.3.32). The basic client scope arrived in 24 and lightweight access tokens in 25. The realm export has no basic entry and lightweight.access.token has zero grep hits across the workspace. 21.1.2 sets a string-UUID sub on every access token, client-credentials included, and no mapper in the export touches sub.

So this is not reachable in production. It is a real fail-open in a general-purpose JWT adapter, reachable by a self-hosted PIPEFY_JWT_ISSUER_URL pointed at another IdP. Worth fixing on those terms, not an incident.

But your "configuration fact, not invariant" instinct is right, for a better reason than the one given. The production realm config is in no repository at all: keycloakConfigCli.enabled: false in both env values files, no Keycloak terraform, and the only realm JSON is a local-dev export last touched 2024-09-09 that does not contain pipefy-mcp or pipefy-cli. The sub guarantee rests on the version pin plus Keycloak behavior, not on a versioned realm definition. That is the part worth carrying into #548.

One correction to the reproduction. "Mixed shapes do not collapse" holds only for the pair you tested. 101 vs absent does collapse, as do bool and list. The real equivalence class is {absent, null, every non-string} -> None, a single principal, so the numeric case is one member of a wider class. Separately, sub is the only fail-open shape hole: a non-string azp/client_id already fails closed, because pydantic rejects it with a ValidationError that verify_token catches into a 401.

And the blast radius is different. Tool calls run under the caller's own bearer, read per message via RequestScopedIdentity.resolve, so a collapsed triple does not let a hijacker act as the victim against Pipefy. What it gives is control of the victim's protocol session and back channel, including the elicitation prompts our tools send. That is sharper than the requestState angle, which is currently unexercised since none of our tools emit input_required.

Applied

• Non-string sub is coerced with str rather than dropped, so an IdP emitting a numeric sub keeps distinguishing its users. This matches how the SDK's own principal_components coerces a non-string iss.
• Empty and whitespace-only sub is rejected (401), per your item 1. Worth flagging for anyone implementing this: it has to be a rejection, not a map to None. None is the no-subject class, so folding a blank sub into it would make two blank-sub bearers share a context where at head they do not, which is a regression. Easy to write as or None by accident.
• Non-blank values carried verbatim, never stripped, so " u1" cannot fuse with "u1".
• The misleading comment is gone. You were right that "falls back to the remaining components, as the SDK documents" read as cover for a fail-open six lines after explaining why they do not identify one principal here.

Not applied: the jti or bearer-hash fallback

Took your option 2 instead, with the waiver written down and pinned against the real session manager. Four reasons:

1. It is not a subject. request_log_middleware.py:81 logs AccessToken.subject as sub. A per-credential id there destroys per-user log grouping and adds unbounded cardinality. You flagged the recognizable-in-logs constraint yourself; this is the same problem one step further.
2. It breaks liveness. jti is per-token, so a sub-less caller's own session would 404 after every refresh, for exactly the azp-only integration class the azp or client_id or sub precedence exists to serve.
3. It buys nothing semantically. An azp-only bearer names no end user, it is the client, so two of them are the same principal. Collapsing them is correct, and it is the degradation the SDK documents for an unsupplied component.
4. jti is not repo-verifiable. Nothing suppresses it and nothing reads it, but its presence on pipefy-mcp tokens is console-only config. Building an ownership component on that is worse than a documented waiver.

Also unchanged: the iss guard, since PyJWT already compares iss for equality against the configured issuer so a non-string one cannot arrive (the docstring now explains why the two guards differ), and the identity precedence chain, which is already fail-closed.

On #548: the residual is now decided rather than open, with the waiver and its reachability evidence in the module docstring and pinned by test. Whether that closes it is your call, but nothing about it is undecided from our side.

Both changelog contradictions fixed, and they were self-inflicted

CHANGELOG.md:19 said short_circuit_error returns a CallToolResult model against a -> dict[str, Any] signature. Reworded from result_is_error's docstring as you suggested. CHANGELOG.md:14 was the worse one and is fixed too: read the real types first (HandlerResult = BaseModel | dict[str, Any] | None at mcp/server/context.py:132, and the seam is Awaitable[HandlerResult]), so "two shape changes" is now three, with the required ToolCallContext.protocol_version you spotted missing.

For the record, these came from integrating two independent branches: one changed short_circuit_error's return type while the other wrote the polymorphism bullet, and I merged both without reconciling them. Exactly the failure mode a changelog-as-lockstep-channel is supposed to catch.

Small stuff

json_response as a third no-back-channel case: applied, and it is broader than a doc clause. Confirmed can_send_request = not is_json_response_enabled and measured through wire_hosted_observability over the real ASGI stack at both 2025-06-18 and 2025-11-25. So there are three cases, and the docstring now names json_response explicitly, as does docs/mcp/tools/pipes-and-cards.md, which listed only agents, CLIs, and SDK consumers as the elicitation-unavailable clients and now names the hosted server too.

One thing worth knowing that changes the conclusion, though. The wrapper pins mcp==1.28.1 today, and 1.28.1 has no can_send_request concept at all: in its json_response branch a server-initiated request is logged and discarded. So a hosted elicitation would have hung awaiting a reply that never came. Hosted elicitation has been non-functional the whole time, silently, and 2.0 makes the same condition fail fast and take the documented fallback. That argues for carrying json_response=True across rather than against it. Filed on the wrapper repo with the port work.

create_card and fill_card_phase_fields: docstrings fixed, missing test added, one return shape declined. Both behaviors reproduced verbatim: create_card sends create_card('9982', {}), and fill_card_phase_fields returns {"success": true, "message": "No fields to update."} with update_card never called. Both docstrings now state when no form is possible and what happens instead, the gate-False path logs symmetrically with the absorb path, and the missing test exists for both tools (elicitation unavailable, required fields present, fields omitted), plus two tests pinning the json_response fact over the real ASGI stack.

The message is split: a phase with no editable fields keeps "No fields to update.", while a phase that has them now reports that nothing was updated and how to supply values. Payload keys and success are unchanged, deliberately. Returning tool_error would diverge from pipefy card fill, which returns the identical payload for the same input (packages/cli/src/pipefy_cli/commands/card.py:409,420, 3 tests), contradict the asymmetry docs/parity.md documents, and turn a today-quiet no-op into an error for any caller whose fields are all dropped by the editable filter, which is the same branch. If success: false is wanted there it should be one change across both surfaces plus the parity doc, not an MCP-only flip. Happy to file that.

We also declined local pre-validation of required fields in create_card: the API owns that check and returns an error the tool surfaces, so a local guard duplicates it.

_meta.serverInfo: documented, not fixed, as you asked. Confirmed at 2026-07-28 that a real tool result carries _meta={'io.modelcontextprotocol/serverInfo': ...} and a short circuit carries no _meta, and that _stamp_server_info runs only on the call_next path. The short_circuit_error docstring now records it and why ToolCallContext cannot reach the stamp.

Counts dropped, and both halves were wrong. Real numbers at ef82857: 64 ctx.debug call sites across 10 modules (raw grep says 69/11, but 2 are docstring mentions and 3 are ctx_debug=ctx.debug method passes), and ctx.info does not exist anywhere in packages/mcp/src, zero call sites, so the ctx.debug/ctx.info phrasing was wrong on both sides. 1010 warning records across 66 unique locations, 100% the logging-capability message. Counts are gone from both the changelog and the pyproject.toml comment; the boundedness rationale stays.

CHANGELOG.md:15 now says pipefy_mcp.__version__, matching what test_protocol_handshake.py actually asserts.

core/runtime.py:88-90 fixed, and you slightly understated it. _local_identity(settings) takes no resource and local discards the parsed one, but _remote_identity does not derive its identity from the resource either: it returns a bare RequestScopedIdentity(). The resource is a hard requirement it fails fast on and the input to the inbound-auth pair. The docstring now says that.

test_protocol_handshake.py:56 dead line removed. inbound_auth = None on the line above still does real work, and all five handshake tests pass without it.

uvicorn declared (>=0.34.0,<1), floor at the locked version, cap matching the repo's only upper-bound idiom, with a docs/dependencies.md row and a changelog sentence beside starlette's. anyio and typing_extensions deliberately not declared, and the distinction is exactly the one you drew: mcp's requirements for both are unconditional, so no marker can drop them from a resolution, which is what made uvicorn different.

Lockfile, worth raising as its own problem. uv lock with a current uv rewrites all 984 package blocks and bumps revision = 1 to 2, adding upload-time everywhere. Two separate agents hit it on this branch and both reverted and made the minimal equivalent edit instead, verifying uv sync --locked passes. Whoever normally locks this repo uses an older uv, and CI's setup-uv@v7 pins no version, so this will keep biting. Probably worth pinning the uv version in CI.

Also fixed, from your earlier round

Extended CHANGELOG.md:13's supersession clause to name both entries the port invalidates. It named only the >=1.29.0,<2 floor bump, but the <2 cap entry in Fixed closes with "The <2 upper bound stays", which reads as current policy against a shipped ==2.0.* pin.

Not verified, and I would rather say so

Live end-to-end coverage of these fixes through a built installation is still missing. The negotiation asymmetry is confirmed on the installed binaries (legacy to 2025-11-25, auto and default to 2026-07-28), and the earlier live smoke against production org 300514213 covered read paths, --toolsets power, and execute_tool dispatching to a real read. But the elicitation fix itself cannot be proven without a write, because the tool must successfully fetch field definitions before it decides whether to elicit, and both eliciting tools write. That run is queued next with a create, read, update, delete round trip on a throwaway card.

Your clean-venv install gate is a good one and it caught something worth keeping: it is the only gate that resolves past uv.lock. Nothing in CI does that today, which is the same gap the ==2.0.* pin reasoning leans on.

[adriannoes]

Re-checked at 9c8bce4 after the follow-up commits that answered the earlier COMMENT review.

Verdict: approve for merge into refactor/543-mcp-2x-port.

What I verified on this tip:

• ruff check packages/mcp pass
• Targeted ownership / resource / capabilities / elicitation / middleware / handshake suite: 78 passed
• Full packages/mcp suite (not integration): 1796 passed
• F2 pins: blank/whitespace sub refused; numeric sub coerced and not collapsed; subjectless same-client waiver pinned; distinct string subjects still isolated
• Live smoke from a worktree at head: initialize negotiates 2025-11-25 with serverInfo {name: pipefy, version: 0.4.0-beta.1}; tools/list returns 187 tools
• Changelog middleware bullets now agree that short_circuit_error returns the wire dict; uvicorn is declared; json_response is named as a no-back-channel case

Accepted residual: the author's queued mutating elicitation E2E against a real org. Unit coverage and the live handshake are enough for this stack layer.

Stack reminder unchanged: land this onto the port branch, then #547 into dev, with the hosted wrapper lockstep still owned on #547.