Skip to content

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

@tgrunnagle

Description

@tgrunnagle

Description

Add tests that pin Tool.BackendID, Resource.BackendID, and Prompt.BackendID as
non-empty on every advertised capability, across all conflict-resolution
strategies
and advertising-filter modes. RFC THV-0076 commits to keeping
BackendID populated through the advertising filter as a first-class contract — the
new VMCP interface exposes BackendID to decorators and relies on it for routing
renamed/prefixed names via LookupTool/CallTool. This task pins that contract with
tests; production code change is small or none (add population only if a gap is found).

Context

Tool/Resource/Prompt.BackendID already exist as fields and are populated during
conflict resolution and merge across the resolvers, then carried onto the advertised
list. There is currently no test asserting BackendID != "" on every advertised
capability
— so a future regression in any resolver or in the advertising filter
could silently ship an empty BackendID, which would break logical-backend routing
and decorator visibility (BackendID is the logical, safe-to-expose identifier per
security.md).

This is a Phase 1 task that is fully independent of the interface/core work and is
intentionally a safe first landing for the epic: it locks down the population contract
before New(cfg) -> VMCP and the admission seam start relying on it. See
architecture.md ("PR-Sized Decomposition Guidance → Phase 1 → P1.2") and research.md
("Tool/Resource/Prompt.BackendID + advertising filter").

Parent Story: #5430
Dependencies: None (independent — safe first landing)
Blocks: None

Acceptance Criteria

  • Table-driven tests assert BackendID != "" on every advertised Tool,
    Resource, and Prompt returned by the aggregator.
  • Coverage spans all conflict-resolution strategies — ConflictStrategyPrefix,
    ConflictStrategyPriority, ConflictStrategyManual — and the advertising-filter
    modes that gate the advertised set: global ExcludeAllTools, per-workload
    ExcludeAll, and per-workload Filter (positive allow-list).
  • Tests cover renamed/prefixed names resolving back to the backend's original name
    via BackendTarget.GetBackendCapabilityName (the path LookupTool/CallTool use).
  • Tests use stdlib testing.T + testify (require/assert), table/subtest-driven,
    matching the existing pkg/vmcp/aggregator test style. No Ginkgo (R8). Regenerate
    mocks via task gen only if a new mock is required.
  • If a gap is found, the production fix is minimal and confined to the population
    site (resolver or merge step), not a redesign.
  • PR is ≤ 400 LOC and ≤ 10 files changed (excluding tests/docs/generated)
  • server.New signature and observable behavior unchanged
  • All tests pass (task test); lint clean (task lint-fix)
  • Code reviewed and approved

Technical Approach

Recommended Implementation

Add a focused, table-driven test (alongside the existing
pkg/vmcp/aggregator/default_aggregator_test.go) that drives
AggregateCapabilities / ProcessPreQueriedCapabilities with multi-backend fixtures
that force name conflicts, then asserts BackendID != "" on every entry of the
advertised tools/resources/prompts. Parameterize the table over the three conflict
strategies and over the three advertising-filter modes (ExcludeAllTools,
per-workload ExcludeAll, per-workload Filter), since the advertised set is the
subset produced by shouldAdvertiseTool.

The advertising filter affects advertising only, not routing — so the assertion
must run over the advertised list (the subset shown to MCP clients), which is the set
the VMCP interface will expose. For renamed/prefixed cases, additionally assert that
the routing target resolves the advertised name back to the backend's original name via
GetBackendCapabilityName, confirming the BackendID on the advertised capability and
the routing entry agree.

Treat this as test-only first: run the new tests; only if a strategy/filter
combination yields an empty BackendID do you add the minimal population fix at the
offending resolver/merge site.

Patterns & Frameworks

  • stdlib testing.T + github.com/stretchr/testify (require/assert), table-driven
    with t.Run subtests — mirrors the existing aggregator tests (R8; architecture.md
    Core Principle 7). No Ginkgo in pkg/vmcp.
  • gomock for any backend registry/client mocks, generated via task gen (only if a new
    mock is needed; reuse existing pkg/vmcp/aggregator/mocks where possible).
  • Conventions: .claude/rules/go-style.md (SPDX header on any new .go file),
    .claude/rules/testing.md, .claude/rules/vmcp-anti-patterns.md.
  • Behavior-preserving: this task must not change MCP behavior observed by clients; it
    pins an existing contract.

Code Pointers

  • pkg/vmcp/aggregator/default_aggregator.goshouldAdvertiseTool (~598-630):
    the advertising filter (excludeAllTools, per-workload ExcludeAll, per-workload
    Filter). Advertised vs all-resolved sets built at ~521-558 (advertisedTools is
    post-filter; allResolvedTools is the full set). BackendID population sites:
    109 (query), 249 (merge), 331/510/550 (resolved/advertised tool construction).
  • pkg/vmcp/aggregator/prefix_resolver.go:69ResolvedTool.BackendID = backendID
    under ConflictStrategyPrefix (e.g. fetchfetch_fetch).
  • pkg/vmcp/aggregator/priority_resolver.go:79,107,121BackendID carried onto the
    winner/prefixed losers under ConflictStrategyPriority.
  • pkg/vmcp/aggregator/manual_resolver.go:150BackendID = backendID under
    ConflictStrategyManual (e.g. fetchcustom_name).
  • pkg/vmcp/aggregator/conflict_resolver.go:68BackendID set on the conflict-set
    entries feeding the strategy resolvers.
  • pkg/vmcp/aggregator/tool_adapter.go:118BackendID: backendID // source of truth
    on the adapted vmcp.Tool.
  • pkg/vmcp/types.go:130BackendTarget.GetBackendCapabilityName(resolvedName):
    returns OriginalCapabilityName if set, else the resolved name — the renamed/prefixed
    resolution used by LookupTool/CallTool. Conflict-strategy enum at types.go:609-621
    (ConflictStrategyPrefix/Priority/Manual); Tool.BackendID (386),
    Resource.BackendID (420), Prompt.BackendID (435).
  • pkg/vmcp/aggregator/default_aggregator_test.go — existing testify, table/subtest
    style to mirror (see TestDefaultAggregator_AggregateCapabilities,
    ..._ExcludeAllTools, ..._FilterPreservesRoutingTableForCompositeTools,
    ..._ProcessPreQueriedCapabilities).

Component Interfaces

No new interface is introduced. The relevant existing contracts the tests pin:

// pkg/vmcp/types.go — fields already present; tests assert they are non-empty
// on every ADVERTISED capability across all strategies/filters.
type Tool     struct { /* ... */ BackendID string }
type Resource struct { /* ... */ BackendID string }
type Prompt   struct { /* ... */ BackendID string }

// pkg/vmcp/types.go:130 — renamed/prefixed name → backend's original name.
// Returns OriginalCapabilityName if set, else resolvedName.
func (t *BackendTarget) GetBackendCapabilityName(resolvedName string) string

Testing Strategy

Unit Tests (table-driven, stdlib testing.T + testify; no Ginkgo)

  • For each ConflictStrategy in {ConflictStrategyPrefix,
    ConflictStrategyPriority, ConflictStrategyManual}: aggregate a multi-backend
    fixture with at least one name conflict and assert BackendID != "" on every
    advertised Tool.
  • Assert the same BackendID != "" invariant on every advertised Resource and
    every advertised Prompt.
  • Advertising-filter coverage: global ExcludeAllTools (advertised set empty —
    vacuously holds, but routing/all-resolved entries still carry BackendID),
    per-workload ExcludeAll, and per-workload Filter (only matching tools advertised);
    for the non-empty advertised subsets, assert BackendID != "" on each entry.
  • Renamed/prefixed resolution: for prefix and manual strategies, assert the
    advertised (renamed) name resolves back via
    target.GetBackendCapabilityName(advertisedName) to the expected original backend
    capability name, and the advertised capability's BackendID matches the routing
    target's backend.

Integration / Behavioral Parity Tests

  • No server.New-level change is expected; the existing aggregator-level tests are
    the gate. If a production population fix is needed, confirm the existing aggregator
    suite (default_aggregator_test.go, tool_adapter_test.go,
    conflict_resolver_test.go) still passes unchanged.

Edge Cases

  • Conflict where two backends expose the same tool name (drives prefix/priority/
    manual rename — the renamed entry must still carry a non-empty BackendID).
  • No-config backend (advertise-all default path) and nil aggregationConfig
    advertised tools still carry BackendID.
  • Filter with partial matches — only matching tools advertised, each with a
    non-empty BackendID.

Out of Scope

  • Adding or changing any BackendID field or domain type (fields already exist;
    no type/wire/storage change).
  • The VMCP interface, New, Serve, the admission seam, or the elicitation rewrite
    (other Phase 1 tasks).
  • Any change to advertising-filter semantics (ExcludeAll/Filter affect advertising
    only, not routing) or to conflict-resolution behavior.
  • Changing MCP behavior observed by clients.

References

Metadata

Metadata

Assignees

No one assigned

    Labels

    needs-triageIssue needs initial triage by a maintainerrefactorvmcpVirtual MCP Server related issues

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions