Skip to content

[FIX][UI]: Include public MCP objects in team-scoped server associations#3514

Merged
crivetimihai merged 2 commits intomainfrom
3446-public-tools-on-team-server
Mar 12, 2026
Merged

[FIX][UI]: Include public MCP objects in team-scoped server associations#3514
crivetimihai merged 2 commits intomainfrom
3446-public-tools-on-team-server

Conversation

@gcgoncalves
Copy link
Copy Markdown
Collaborator

@gcgoncalves gcgoncalves commented Mar 6, 2026

🐛 Bug-fix PR

📌 Summary

When editing a team server, include selected objects from public MCP servers on saving.

Screen.Recording.2026-03-06.at.09.29.18.mov

Closes #3446

🔁 Reproduction Steps

Link the issue and minimal steps to reproduce the bug.

🐞 Root Cause

There was a filter for team items on the saving and fetching logic.

💡 Fix Description

Enabled both the team and public ones on the saving and fetching conditions.

🧪 Verification

Check Command Status
Lint suite make lint
Unit tests make test
Coverage ≥ 80 % make coverage
Manual regression no longer fails steps / screenshots

✅ Checklist

  • Code formatted (make black isort pre-commit)
  • No secrets/credentials committed

@gcgoncalves gcgoncalves added the release-fix Critical bugfix required for the release label Mar 6, 2026
@gcgoncalves gcgoncalves force-pushed the 3446-public-tools-on-team-server branch 4 times, most recently from 548a02b to 4b147e1 Compare March 6, 2026 15:19
madhav165

This comment was marked as off-topic.

@crivetimihai crivetimihai changed the title 3446 - Enable public MCP objects on team Servers [FIX][UI]: Include public MCP objects in team-scoped server associations Mar 7, 2026
@crivetimihai crivetimihai added bug Something isn't working SHOULD P2: Important but not vital; high-value items that are not crucial for the immediate release ui User Interface labels Mar 7, 2026
@crivetimihai crivetimihai added this to the Release 1.0.0 milestone Mar 7, 2026
@crivetimihai
Copy link
Copy Markdown
Member

Thanks @gcgoncalves. This is a solid fix for #3446 — the _merge_select_all_ids() extraction is a nice improvement over the duplicated inline logic, and the standalone visibility == 'public' condition correctly ensures platform-public items aren't silently dropped from team-scoped queries. Two items: (1) the CI checks are failing — please investigate and fix, (2) the title should follow the naming convention (e.g. [FIX][UI]: ...). The test coverage looks good.

@crivetimihai crivetimihai self-assigned this Mar 7, 2026
@gcgoncalves gcgoncalves requested a review from madhav165 March 9, 2026 10:57
@gandhipratik203 gandhipratik203 self-requested a review March 12, 2026 10:46
@gcgoncalves gcgoncalves force-pushed the 3446-public-tools-on-team-server branch 2 times, most recently from 62126ae to 8917a5e Compare March 12, 2026 13:11
gcgoncalves and others added 2 commits March 12, 2026 15:05
Signed-off-by: Gabriel Costa <gabrielcg@proton.me>
…/ids endpoints

Add direct unit tests for _merge_select_all_ids helper covering all
branches (flag not set, valid merge, int normalization, JSON decode
error, missing key, empty checked list). Add regression tests for
prompts and resources /ids endpoints to match the existing tool test,
ensuring standalone visibility='public' condition is present in
team-scoped queries. Closes #3446.

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
@crivetimihai crivetimihai force-pushed the 3446-public-tools-on-team-server branch from b4826f3 to 3b3e699 Compare March 12, 2026 15:39
@crivetimihai
Copy link
Copy Markdown
Member

Review & Changes

Rebased onto main (clean, no conflicts) and performed a thorough code review + manual Playwright testing.

Code Review

The fix is well-designed with two complementary changes:

  1. _merge_select_all_ids helper — refactors duplicated Select All parsing from admin_add_server/admin_edit_server into a reusable function. Key behavior change: takes the union of server-fetched IDs and UI-checked IDs (instead of replacing), so public items are never silently dropped on save.

  2. /ids endpoints include public items in team-scoped queries — adds standalone visibility='public' OR condition to admin_get_all_tool_ids, admin_get_all_prompt_ids, and admin_get_all_resource_ids when team_id is set, so "Select All" count matches what's visible in the UI.

No security or performance concerns — public-visibility items are platform-wide by design, and downstream persistence code enforces final access control.

Manual Testing (Playwright)

Test Result
Create server in team-alpha with "View Public" + Select All tools PASS — 6 tools (2 team + 4 public) associated
Edit server, save without changes PASS — all 6 tools preserved after save
API verification of tool associations after edit PASS — all 6 tool IDs confirmed
/tools/ids?team_id=... returns public items PASS — 6 IDs returned

My Changes (second commit)

Added 9 tests for 100% differential coverage:

  • TestMergeSelectAllIds (7 tests): Direct unit tests for all branches of _merge_select_all_ids — flag not set, valid merge, int normalization, JSON decode error, missing key, empty checked list
  • test_admin_get_all_prompt_ids_team_scoped_includes_public: Regression test for prompts /ids endpoint
  • test_admin_get_all_resource_ids_team_scoped_includes_public: Regression test for resources /ids endpoint

All 1044 tests pass.

Note (non-blocking)

The /ids endpoints for tools/resources/prompts now unconditionally include public items when team_id is set, while the listing/search endpoints and the gateways /ids endpoint use an explicit include_public parameter. This minor inconsistency is acceptable for a bug fix — the unconditional approach is the safest default to prevent data loss. Could be harmonized in a future follow-up if desired.

Copy link
Copy Markdown
Member

@crivetimihai crivetimihai left a comment

Choose a reason for hiding this comment

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

Approved. Code is correct, well-tested, and manually verified with Playwright. Added differential test coverage.

@crivetimihai crivetimihai merged commit c8b9491 into main Mar 12, 2026
38 checks passed
@crivetimihai crivetimihai deleted the 3446-public-tools-on-team-server branch March 12, 2026 15:42
Yosiefeyob pushed a commit that referenced this pull request Mar 13, 2026
…ons (#3514)

* 3446 - Enable public MCP objects on team Servers

Signed-off-by: Gabriel Costa <gabrielcg@proton.me>

* test: add differential coverage for public visibility in team-scoped /ids endpoints

Add direct unit tests for _merge_select_all_ids helper covering all
branches (flag not set, valid merge, int normalization, JSON decode
error, missing key, empty checked list). Add regression tests for
prompts and resources /ids endpoints to match the existing tool test,
ensuring standalone visibility='public' condition is present in
team-scoped queries. Closes #3446.

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

---------

Signed-off-by: Gabriel Costa <gabrielcg@proton.me>
Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
Co-authored-by: Mihai Criveti <crivetimihai@gmail.com>
Signed-off-by: Yosief Eyob <yosiefogbazion@gmail.com>
calculus-ask pushed a commit to calculus-ask/mcp-context-forge that referenced this pull request Mar 18, 2026
…ons (IBM#3514)

* 3446 - Enable public MCP objects on team Servers

Signed-off-by: Gabriel Costa <gabrielcg@proton.me>

* test: add differential coverage for public visibility in team-scoped /ids endpoints

Add direct unit tests for _merge_select_all_ids helper covering all
branches (flag not set, valid merge, int normalization, JSON decode
error, missing key, empty checked list). Add regression tests for
prompts and resources /ids endpoints to match the existing tool test,
ensuring standalone visibility='public' condition is present in
team-scoped queries. Closes IBM#3446.

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

---------

Signed-off-by: Gabriel Costa <gabrielcg@proton.me>
Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
Co-authored-by: Mihai Criveti <crivetimihai@gmail.com>
Signed-off-by: KRISHNAN, SANTHANA <sk8069@exo.att.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working release-fix Critical bugfix required for the release SHOULD P2: Important but not vital; high-value items that are not crucial for the immediate release ui User Interface

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG][API]: Public MCP server tools not included when added to team-owned virtual server

3 participants