Honor configured vMCP backend timeouts - #6411
Conversation
Apply operational timeout defaults and per-workload overrides to Modern and Legacy backend operations, including SSE operation contexts and session initialization. Defer the MCP POST write deadline during backend computation, then restore a bounded deadline when a non-streaming response begins. Signed-off-by: Jairus Christensen <christensenjairus@gmail.com>
|
Sorry, codex did more than I asked of it 😅. Sorry for requesting each of you directly. I'll humanize this. |
|
From the documentation & from what I can see poking around, it seems this was legitimately missing. The 30s I caught this as I was using a remote mcp (elastic) that takes roughly ~100s to start writing a response, and nothing in the current implementation can get vmcp to wait that long for the mcp to start writing before it closes the connection. The fix here works for me & I tried to keep it conservative. |
|
Thanks for tackling this, @christensenjairus — the write-up is thorough and the test coverage across the client, session, and middleware layers is genuinely nice to see, especially the paired timeout/success cases that guard against the old hardcoded 30s accidentally satisfying the new tests. The One correctness issue I'd like resolved before merging: Per-workload timeout can unintentionally shrink the backend-init allowance, in
Would you mind either having Everything else looks solid — happy to take another pass once this is addressed. |
Use the factory's widened initialization context when connecting persistent backends, while retaining the configured timeout for ordinary session operations. Cover the shorter request-timeout case with the real HTTP connector. Signed-off-by: Jairus Christensen <christensenjairus@gmail.com>
Signed-off-by: Jairus Christensen <christensenjairus@gmail.com>
|
@reyortiz3 Thanks for the review! I believe I've addressed your concerns. |
Summary
vMCP accepts the documented
operational.timeouts.defaultandperWorkloadsettings, but it did not use them for backend MCP calls. Backend clients kept their 30-second default. A separate 30-second serverWriteTimeoutcould also close a POST before a slow backend returned anything.This change resolves the configured timeout for each workload and passes it to both client paths, including initialization and SSE operations. It also pauses the server write deadline while the backend call runs, then starts a bounded deadline when a non-streaming response begins.
Fixes #6410
Type of change
Test plan
task test)task test-e2e)task lint-fix)Validation details:
pkg/vmcp/client,pkg/vmcp/session,pkg/vmcp/session/internal/backend,pkg/vmcp/cli,pkg/vmcp/server, andpkg/transport/middleware.task testran the full race-enabled package suite. The only failure waspkg/secrets/keyring/TestCompositeProvider_RealProviders; this host has no usable OS keyring provider.task test-e2eran until its 20-minute suite timeout. It passed 58 specs, including the vMCP CLI feature tests. Six Git skill tests failed because their local Git HTTP server returned 404, which the API reported as 502. The bulk-workload spec was running when the global timeout expired. The harness removed its test workloads afterward.task lint-fix,task lint,go vet ./...,task build, andgit diff --checkpassed.240sper-workload timeout.API compatibility
v1beta1API, OR theapi-break-allowedlabel is applied and the migration guidance is described above.The default behavior does not change when no operational timeout is configured. The variadic duration argument keeps
WriteTimeoutsource-compatible.Changes
pkg/vmcp/cli,pkg/vmcp/clientpkg/vmcp/sessionpkg/transport/middleware,pkg/vmcp/server*_test.goDoes this introduce a user-facing change?
Yes. vMCP now honors its documented backend timeout settings. Configurations without timeout settings keep the current defaults.
Special notes for reviewers
An SSE connection can stay open indefinitely, but each MCP operation on that connection uses the configured backend timeout. Session initialization keeps its existing allowance unless the workload timeout is longer.