Skip to content

fix(resources): skip post-fetch resolution for direct_proxy reads - #6503

Open
Ewertonslv wants to merge 1 commit into
IBM:mainfrom
Ewertonslv:fix/5451-direct-proxy-resource-read
Open

fix(resources): skip post-fetch resolution for direct_proxy reads#6503
Ewertonslv wants to merge 1 commit into
IBM:mainfrom
Ewertonslv:fix/5451-direct-proxy-resource-read

Conversation

@Ewertonslv

Copy link
Copy Markdown

🔗 Related Issue

Closes #5451

Fresh PR replacing #5463. That PR was closed on 2026-08-31 during the housekeeping pass on long-open contributions. GitHub refused to re-open it after the rebase (a force-push makes a closed PR non-reopenable), so this is the "or open a fresh one" path @jonpspri suggested. The change itself is unchanged — only rebased.

📝 Summary

In direct_proxy gateway mode, ResourceService.read_resource fetches the
resource live from the upstream and builds the final TextResourceContents /
BlobResourceContents. Execution then falls through into the shared post-fetch
RESOLVE CONTENT block, whose first branch runs getattr(content, "id")
unconditionally. Those MCP-compliant models (mcpgateway/common/models.py) have
no id field, so every direct_proxy text or blob read raises
AttributeError: 'TextResourceContents' object has no attribute 'id'. The
transport layer catches the exception and returns empty content, so the client
silently receives text="" instead of the fetched payload.

Root cause

The direct_proxy branch never marked its content as already-resolved — the
# Skip the rest of the DB lookup logic comment does not actually skip anything.

Fix

Track whether content came from the direct_proxy branch (direct_proxy_read) and
skip the metadata→content resolution for it. The gateway access check, the
resource access check, and the post-fetch plugin hooks all still run — only the
id-based invoke_resource re-resolution is bypassed, since the payload is final.

Tests (before → after)

The existing happy-path direct_proxy tests masked the bug by monkey-patching
id-bearing subclasses over the real models, so production classes never took the
failing path. This PR adds two regression tests exercising the real
TextResourceContents / BlobResourceContents:

  • Without the fix: both raise AttributeError at the getattr(content, "id") call.
  • With the fix: text read returns the live text, image/png blob read returns its
    blob, and invoke_resource is asserted not re-awaited.

🔄 Rebase notes (2026-08-31)

Rebased onto current main (ffcc0822). One conflict, resolved by keeping both
sides:
main grew the _set_gateway_content helper immediately above the
if isinstance(content, ...) branch. The helper is untouched — the
direct_proxy_read guard simply sits in front of it as
if direct_proxy_read: ... elif isinstance(...).

Re-verified against today's main, since a lot landed in the six weeks the PR sat:

  • The bug is still present. The RESOLVE CONTENT block still calls
    getattr(content, "id") unconditionally, and the MCP-compliant models still have
    no id.
  • The regression proof still holds. With the production hunk reverted to main,
    both new tests fail with
    AttributeError: 'TextResourceContents' object has no attribute 'id'; with the
    hunk applied, both pass.
  • pytest tests/unit/mcpgateway/services/test_resource_service.py — 309 passed.
  • ruff check, ruff format --check and black --check clean on both touched files.

Also: #5451 is no longer labeled triage (it is bug / COULD now), which was the
concern I flagged in my comment on the original PR.

📏 Reviewability

  • This PR has one clear purpose
  • The linked issue is not labeled triage
  • Unrelated bugs or improvements are tracked in separate issues/PRs
  • Tests are included with the code they validate
  • If AI-assisted, I understand and can explain the generated changes

🏷️ Type of Change

  • Bug fix

In direct_proxy gateway mode, read_resource builds the final
TextResourceContents/BlobResourceContents from the live upstream fetch,
but execution then fell into the post-fetch "RESOLVE CONTENT" block,
which calls getattr(content, "id") unconditionally. Those MCP-compliant
models have no `id` field, so every direct_proxy text or blob read
raised AttributeError, was swallowed at the transport layer, and
returned empty content to the client.

Track whether the content was produced by the direct_proxy branch and
skip the metadata->content resolution for it, keeping the gateway
access and resource access checks and the post-fetch hooks intact.

Adds regression tests that exercise the real (id-less) production models
instead of the id-bearing test subclasses that previously masked the bug.

Closes IBM#5451

Signed-off-by: Ewerton Silva <ewertoncom297@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG]: direct_proxy resource read raises AttributeError ('…ResourceContents' has no attribute 'id') and returns empty content

1 participant