Skip to content

Use conformant Accept in dual-era e2e - #6123

Merged
ChrisJBurns merged 4 commits into
stacklok:mainfrom
kocaemre:test/conformant-dual-era-accept
Aug 26, 2026
Merged

ChrisJBurns merged 4 commits into
stacklok:mainfrom
kocaemre:test/conformant-dual-era-accept

Conversation

@kocaemre

Copy link
Copy Markdown
Contributor

Summary

  • The dual-era e2e specs were still proving vMCP bridge behavior without the streamable-HTTP Accept: application/json, text/event-stream header that both MCP revisions require.
  • Add WithStreamableAccept() to the vMCP dual-era bridge and concurrent-mixing specs so those proxy paths now run under conformant request headers.
  • Add a vMCP bridge assertion that Legacy and Modern response content is identical with and without the SSE-capable Accept header, and update RawMCPClient comments/tests now that SSE response parsing is available.

Fixes #6104

Type of change

  • Bug fix
  • New feature
  • Refactoring (no behavior change)
  • Dependency update
  • Documentation
  • Other (describe): E2E coverage hardening

Test plan

  • Unit tests (task test)
  • E2E tests (task test-e2e)
  • Linting (task lint-fix)
  • Manual testing (describe below)

Manual testing:

  • PATH=/usr/local/go/bin:/root/go/bin:$PATH go test ./test/e2e -run 'TestRawClientBuilders/no_Accept|TestRawClientSSEResponse'
  • PATH=/usr/local/go/bin:/root/go/bin:$PATH go test -race ./test/e2e -run 'TestRawClientBuilders/no_Accept|TestRawClientSSEResponse'
  • PATH=/usr/local/go/bin:/root/go/bin:$PATH task build
  • PATH=/usr/local/go/bin:/root/go/bin:$PATH THV_BINARY=$PWD/bin/thv TOOLHIVE_DEV=true TEST_TIMEOUT=15m LABEL_FILTER='vmcp && dual-era' ./test/e2e/run_tests.sh — 7 passed, 0 failed, 500 skipped
  • PATH=/usr/local/go/bin:/root/go/bin:$PATH task lint
  • git diff --check

Note: I also tried go test ./test/e2e before exporting THV_BINARY; it failed because the full e2e suite could not find thv on $PATH. The labeled e2e run above used the freshly built bin/thv and passed.

API Compatibility

  • This PR does not break the v1beta1 API, OR the api-break-allowed label is applied and the migration guidance is described above.

Changes

File Change
test/e2e/vmcp_dual_era_test.go Run the vMCP dual-era bridge specs with conformant Accept and add plain-vs-SSE content parity coverage.
test/e2e/dual_era_mixing_test.go Run concurrent mixed-era proxy traffic with conformant Accept.
test/e2e/mcp_raw_client.go Update RawMCPClient comments for proxy-bound conformant Accept usage.
test/e2e/mcp_raw_client_test.go Update the no-default-Accept assertion message.

Does this introduce a user-facing change?

No.

Special notes for reviewers

The production behavior is unchanged. This only flips test traffic to the spec-required Accept header now that RawMCPClient can parse SSE-framed POST responses.

Signed-off-by: Emre K <110906681+kocaemre@users.noreply.github.com>
Comment thread test/e2e/vmcp_dual_era_test.go Outdated
@kocaemre
kocaemre force-pushed the test/conformant-dual-era-accept branch from 30d7207 to 7246fe5 Compare August 18, 2026 20:50
Signed-off-by: Emre K <110906681+kocaemre@users.noreply.github.com>
@kocaemre
kocaemre force-pushed the test/conformant-dual-era-accept branch from 7246fe5 to ac56ca5 Compare August 18, 2026 20:50
@kocaemre

Copy link
Copy Markdown
Contributor Author

Addressed the review nit in ac56ca53 by updating the suite-level comment to call out the intentional plain-Accept parity-check exception.

Verification:

  • go test ./test/e2e -run 'TestRawClientBuilders/no_Accept|TestRawClientSSEResponse' → passed\n- git diff --check origin/main..HEAD → passed\n- task lint → passed (golangci-lint + go vet, 0 issues)

@kocaemre

Copy link
Copy Markdown
Contributor Author

CI follow-up: the one red job is Tests / Test Go Code (ubuntu-8cores-32gb), failing in pkg/auth/discovery:

  • TestTryWellKnownDiscovery/endpoint-specific_well-known_URI_found
  • assertion: Expected AuthInfo but got nil at pkg/auth/discovery/discovery_test.go:1016

That package/test is outside this PR's touched e2e raw-client/dual-era Accept path. I reran the failing test and package locally against the PR branch:

  • go test ./pkg/auth/discovery -run 'TestTryWellKnownDiscovery/endpoint-specific_well-known_URI_found' -count=1 -v → passed
  • go test ./pkg/auth/discovery -count=1 → passed

So this looks like an unrelated CI flake/regression in auth discovery rather than a failure introduced by this PR.

@codecov

codecov Bot commented Aug 25, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 77.82%. Comparing base (7c9c55e) to head (3416b07).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6123      +/-   ##
==========================================
+ Coverage   77.81%   77.82%   +0.01%     
==========================================
  Files         760      761       +1     
  Lines       73133    73182      +49     
==========================================
+ Hits        56908    56956      +48     
- Misses      16220    16221       +1     
  Partials        5        5              

☔ View full report in Codecov by Harness.
📢 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.

@ChrisJBurns
ChrisJBurns merged commit 811b75b into stacklok:main Aug 26, 2026
41 checks passed
@github-actions github-actions Bot mentioned this pull request Aug 26, 2026
2 tasks
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.

Run the dual-era e2e specs under the conformant Accept header

2 participants