fix(046): persist LauncherWaitTimeout to BBolt + regenerate OAS#454
Merged
Conversation
- Add LauncherWaitTimeout to UpstreamRecord so launcher servers added via the REST upstream_servers API retain the timeout across restarts. Wired through SaveUpstreamServer / GetUpstreamServer / ListUpstreamServers, matching the ReconnectOnUse precedent from spec 354. - Whitelist LauncherWaitTimeout in TestSaveServerSyncFieldCoverage so the ServerConfig field-coverage guard accepts the new field. - Regenerate oas/swagger.yaml + oas/docs.go so the Verify OpenAPI Artifacts CI step stops complaining about drift. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Deploying mcpproxy-docs with
|
| Latest commit: |
569e8cd
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://3411524e.mcpproxy-docs.pages.dev |
| Branch Preview URL: | https://fix-spec-046-launcher-timeou.mcpproxy-docs.pages.dev |
reap() called cmd.Wait() before pumpWG.Wait(). Per exec.Cmd docs, Wait closes the parent-side stdout/stderr pipes as part of its cleanup — so if Wait ran before both pump goroutines reached EOF, the pump scanners could race against that close and observe EOF with buffered child output still in the kernel pipe, silently dropping it. CI repro: TestSpawn_LogSinkCaptured on Linux runners (Unit Tests amd64 and Build Binaries linux-arm64) saw the sink contain only the startup banner, no [launcher stdout]/[launcher stderr] lines. Locally the test passed because timing happened to favor pumps. 20x -race iterations green after flipping the order. Pumps naturally hit EOF when the child exits and the kernel propagates the close — calling Wait() is not what unblocks them. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
7 tasks
Windows GitHub runners have something bound on 127.0.0.1:80 (IIS/WinNAT default), so the test's "should be unreachable" assertion failed: the dial succeeded and WaitForURL returned nil where the test expected a "not reachable" error. Switched the host to 192.0.2.1 (TEST-NET-1, RFC 5737) — reserved for documentation and guaranteed non-routable, so the dial fails consistently on every OS while still exercising default-port inference from the URL scheme. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
📦 Build ArtifactsWorkflow Run: View Run Available Artifacts
How to DownloadOption 1: GitHub Web UI (easiest)
Option 2: GitHub CLI gh run download 25756022180 --repo smart-mcp-proxy/mcpproxy-go
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to #452 (spec 046 local launcher for HTTP/SSE upstreams). #452 was merged with two CI failures because the org-owned fork blocked maintainer pushes; this PR makes main green again.
TestSaveServerSyncFieldCoverage(internal/storage): the reflection guard verifies everyServerConfigfield is either persisted toUpstreamRecordor explicitly whitelisted. The newLauncherWaitTimeoutfield needed handling. Following theReconnectOnUseprecedent from spec 354, persisted it to BBolt rather than marking it config-file-only — this way launcher servers added via the RESTupstream_serversAPI retain the timeout across restarts instead of silently dropping it.Verify OpenAPI Artifacts: regeneratedoas/swagger.yaml+oas/docs.goviamake swaggerso the drift check passes.Changes
internal/storage/models.go— addLauncherWaitTimeout config.DurationtoUpstreamRecord(JSON taglauncher_wait_timeout,omitempty).internal/storage/manager.go— wire it throughSaveUpstreamServer/GetUpstreamServer/ListUpstreamServers.internal/storage/async_ops_test.go— whitelistLauncherWaitTimeoutinexpectedFieldswith a Spec 046 comment.oas/{docs.go,swagger.yaml}— regenerated.Test plan
go test ./internal/storage/ -count=1— green locallygo build ./...— green locallyTestSaveServerSyncFieldCoveragenow passes (reproduced the original failure first, then fixed)Credit for spec 046 belongs to @electrolobzik — this PR only patches the persistence/OAS gap.