Skip to content

Pin BackendID through the advertising filter (tests)#5452

Merged
tgrunnagle merged 1 commit into
mainfrom
vmcp-core-p1-2_issue_5435
Jun 3, 2026
Merged

Pin BackendID through the advertising filter (tests)#5452
tgrunnagle merged 1 commit into
mainfrom
vmcp-core-p1-2_issue_5435

Conversation

@tgrunnagle
Copy link
Copy Markdown
Contributor

Pin BackendID through the advertising filter (tests)

Closes #5435

Summary

RFC THV-0076 commits to keeping BackendID populated on every advertised capability as a first-class contract: it is the logical, safe-to-expose backend identifier that decorators see and that LookupTool/CallTool rely on to route renamed/prefixed names back to their backend. Until now there was no test asserting BackendID != "" on every advertised Tool, Resource, and Prompt, so a future regression in any resolver or in the advertising filter could silently ship an empty BackendID and break logical-backend routing and decorator visibility. This PR adds contract tests that pin that invariant across all conflict-resolution strategies and advertising-filter modes.

No production code was changed: BackendID is already populated at every resolver and merge site and by the backend client on resources and prompts. The investigation found no gap, so this is a test-only change that locks down the existing behavior before the VMCP core interface and admission seam start relying on it.

Changes Made

pkg/vmcp/aggregator (test-only)

  • Add advertised_backendid_test.go with three table-driven tests:
    • TestDefaultAggregator_AdvertisedCapabilitiesCarryBackendID — drives AggregateCapabilities with a multi-backend fixture that forces a fetch name conflict, then asserts BackendID != "" on every advertised tool, resource, and prompt across the prefix, priority, and manual conflict-resolution strategies. For renamed/prefixed entries it also asserts the advertised name resolves back to the backend's original capability name via BackendTarget.GetBackendCapabilityName, and that the advertised capability's BackendID agrees with the routing target's WorkloadID.
    • TestDefaultAggregator_AdvertisingFilterPreservesBackendID — exercises the advertising-filter modes that gate the advertised set: global ExcludeAllTools, per-workload ExcludeAll, and per-workload Filter (positive allow-list). Asserts BackendID != "" on every advertised entry, confirms the advertised tool set matches each filter mode, and verifies the routing table retains all tools with their backend identity intact (the filter affects advertising only, not routing).
    • TestDefaultAggregator_ProcessPreQueriedCapabilitiesCarryBackendID — covers the pre-queried path across the three strategies plus ExcludeAllTools and Filter, asserting the invariant on both the full resolved set and the advertised subset, and that each advertised tool's BackendID matches its routing target.

Implementation Details

  • Tests use stdlib testing.T plus testify (require/assert), table/subtest-driven, mirroring the existing pkg/vmcp/aggregator test style. No Ginkgo.
  • Existing aggregator mocks (pkg/vmcp/mocks) and test helpers are reused; no new mock was required.
  • The assertions run over the advertised list (the subset shown to MCP clients), since the advertising filter produces a subset via shouldAdvertiseTool. For the renamed/prefixed cases the routing target is checked alongside the advertised capability to confirm the two agree on backend identity.

Testing

  • New tests added as described above; all pass via task test.
  • Lint is clean for the new file. The only repo-wide lint finding is a pre-existing, unrelated gosec warning in cmd/thv/app/upgrade.go.
  • No production code changed, so observable MCP behavior and the aggregator's existing test suite are unaffected.

Additional Notes

Implements tests for issue #5435 (P1.2):
- Assert BackendID != "" on every advertised Tool, Resource, and Prompt
  across the prefix, priority, and manual conflict-resolution strategies
- Cover advertising-filter modes: global ExcludeAllTools, per-workload
  ExcludeAll, and per-workload Filter
- Verify renamed/prefixed names resolve back to the backend's original
  name via GetBackendCapabilityName (the LookupTool/CallTool path)

No production change needed: BackendID is already populated at every
resolver/merge site and by the backend client.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added the size/M Medium PR: 300-599 lines changed label Jun 3, 2026
@tgrunnagle tgrunnagle marked this pull request as ready for review June 3, 2026 22:08
@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 3, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 68.84%. Comparing base (255686a) to head (4a0e12c).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5452      +/-   ##
==========================================
- Coverage   68.85%   68.84%   -0.01%     
==========================================
  Files         634      634              
  Lines       64439    64439              
==========================================
- Hits        44370    44364       -6     
- Misses      16789    16794       +5     
- Partials     3280     3281       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@tgrunnagle tgrunnagle merged commit 05f11b5 into main Jun 3, 2026
75 of 76 checks passed
@tgrunnagle tgrunnagle deleted the vmcp-core-p1-2_issue_5435 branch June 3, 2026 23:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/M Medium PR: 300-599 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

P1.2 Pin BackendID through the advertising filter (tests)

2 participants