Skip to content

fix(mcpcompat/server): skip resources with unparseable URI in syncSessionResources - #388

Open
reyortiz3 wants to merge 2 commits into
mainfrom
worktree-follow-up-syncSessionResources
Open

reyortiz3 wants to merge 2 commits into
mainfrom
worktree-follow-up-syncSessionResources

Conversation

@reyortiz3

@reyortiz3 reyortiz3 commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • go-sdk's AddResource panics when the resource URI fails url.Parse. syncSessionResources can run on the live session goroutine, so a malformed URI in a per-session overlay could crash the session.
  • Run the same url.Parse check first and log/skip the offending resource, so no recover is needed.
  • Fix the sibling comments that wrongly said AddResource doesn't panic (the likely cause of this gap).

Follow-up requested in review of stacklok/toolhive-GHSA-r928-c58c-5ppq#1.

Test plan

  • New TestSyncSessionResources_UnparseableURIDoesNotPanic
  • go test -race ./mcpcompat/server/
  • task lint

Not covered: registerGlobalFeatures still calls srv.AddResource without a check.

🤖 Generated with Claude Code

reyortiz3 and others added 2 commits October 16, 2026 16:16
go-sdk's AddResource panics when the resource URI fails url.Parse. Since
syncSessionResources can run on the live session goroutine, a bad URI in a
per-session overlay would crash the session. Register each resource via
addSessionResource, which recovers, logs and skips the offending resource,
mirroring addSessionTool and addSessionResourceTemplate.

Also correct the sibling comments that claimed AddResource cannot panic.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…vering

go-sdk's AddResource only panics when url.Parse fails, so run the same check
first and skip the resource, avoiding the recover/defer helper.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@reyortiz3 reyortiz3 changed the title fix(mcpcompat/server): recover AddResource panic in syncSessionResources fix(mcpcompat/server): skip resources with unparseable URI in syncSessionResources Oct 16, 2026
@reyortiz3
reyortiz3 enabled auto-merge (squash) October 16, 2026 20:49
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.

1 participant