Skip to content

Allow R session tools to be optional #68

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 12 commits into from
Sep 24, 2025
Merged

Allow R session tools to be optional #68

merged 12 commits into from
Sep 24, 2025

Conversation

@t-kalinowski
Copy link
Member

No description provided.

@t-kalinowski t-kalinowski changed the title make R session tools optional. Allow R session tools to be optional. Sep 11, 2025
t-kalinowski added a commit to tidyverse/ragnar that referenced this pull request Sep 11, 2025
@t-kalinowski
opt-out of r session tools
6672cb4 
Companion to posit-dev/mcptools#68
simonpcouch
simonpcouch reviewed Sep 11, 2025
View reviewed changes
Copy link
Collaborator

@simonpcouch simonpcouch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems reasonable to me! In addition to the dots addition, could you add a NEWS entry?

t-kalinowski and others added 3 commits September 11, 2025 09:11
@t-kalinowski @simonpcouch
Update R/server.R
825aba9 
Co-authored-by: Simon P. Couch <[email protected]>
@t-kalinowski @simonpcouch
Update R/tools.R
80c6b3a 
Co-authored-by: Simon P. Couch <[email protected]>
@t-kalinowski
add NEWS
f43dc89
@simonpcouch
Copy link
Collaborator

Now that I'm more caffeinated, one more bit that this brings up for me is that removing access to those tools doesn't necessarily mean that the model won't run code in a session connected with mcp_session(). If include_session_tools, we should make sure that the server doesn't bother trying to discover listening sessions (and thus the tool is invoked via handle_request() rather than forward_request()).

t-kalinowski added a commit to tidyverse/ragnar that referenced this pull request Sep 13, 2025
@t-kalinowski
MCP ragnar server (#123)
cfd6647 
* add `mcp_serve_store`

* opt-out of r session tools

Companion to posit-dev/mcptools#68

* update docs + small tweaks

* Change the default tool name prefix to `search_store_`

* add NEWS

* add `mcptools` to Suggests

* add `mcp_serve_store` to pkgdown
simonpcouch
simonpcouch reviewed Sep 15, 2025
View reviewed changes
@simonpcouch
Copy link
Collaborator

@shikokuchuo Once checks are passing, this PR seems reasonable to me. Could you give this a look and make sure the session comms seem reasonable? If so, you're welcome to merge—I will be out for 3 weeks post-conf, so do go ahead and merge when you're happy with how this looks!

No rush on getting to this, from my perspective. :)

shikokuchuo
shikokuchuo approved these changes Sep 23, 2025
View reviewed changes
Copy link
Member

@shikokuchuo shikokuchuo left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@t-kalinowski I've opened a PR that targets this one (enterprise SSO woes prevent me from pushing the changes directly) - very minor. Otherwise good to go!

@shikokuchuo shikokuchuo changed the title Allow R session tools to be optional. Allow R session tools to be optional Sep 24, 2025
@shikokuchuo shikokuchuo merged commit f700559 into posit-dev:main Sep 24, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@simonpcouch simonpcouch simonpcouch left review comments

@shikokuchuo shikokuchuo shikokuchuo approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@t-kalinowski @simonpcouch @shikokuchuo