Repository navigation
test: fix three red main checks (OAuth replay fixtures, fetch --wait timing, quarantine-pilot model) - #1829
Merged
Conversation
…ir own TestRefreshReplay_HTTPTimeout failed on main (run 42203437083) with a token request carrying no refresh_token, while the test's own two requests both arrived and every assertion on the test goroutine passed. The code under test cannot send that request: the synthetic AS stays up for the 10s first-attempt timeout, and a request leaked by an earlier test reached its reused loopback port. An injected stray GET reproduces the CI output exactly. The three synthetic AS handlers now count and assert only on POST /token carrying rt-0; anything else is logged and answered 404. A replay that sent a different token still fails the test, because it gets the 404.
TestReviewFetchWaitBoundsWholeCommand/post failed in the shuffle job at 1.59s against a 1.45s bound. The stopwatch ran around runFetch, so it also timed the harness (two temp dirs, a config write, the test server) and left 450ms for all of it on top of the 1s deadline. The fixture now records whether a delayed request ran its full delay, and the test requires that the command returned before that. Checked both ways: with the POST taken off the shared deadline the post case fails, and with a 600ms stall added to the harness the test passes.
The scheduled exploration lane has failed since #1714: the model still approved every tool on a trusted server's first discovery, while the implementation now holds a tool that scans dangerous (and does not promote it on the next identical list). The deterministic lane had no scenario with a poisoned first-seen tool, so the PR checks stayed green. - model: a baseline pass approves a new tool, or promotes an unchanged pending one, only on a clean verdict - S30, S31: the mixed and the all-dangerous first discovery, on every PR - M20, M21: removing either baseline hold is detected by those scenarios - QP-08 and the pilot doc say what the baseline now means
Deploying mcpproxy-docs with
|
| Latest commit: |
e28cc02
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://58b2ceb8.mcpproxy-docs.pages.dev |
| Branch Preview URL: | https://claude-main-release-readines.mcpproxy-docs.pages.dev |
Contributor
📦 Build ArtifactsWorkflow Run: View Run Available Artifacts
How to DownloadOption 1: GitHub Web UI (easiest)
Option 2: GitHub CLI gh run download 42259565575 --repo smart-mcp-proxy/mcpproxy-go
|
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
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
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
Three checks are red on
mainat28247002e. All three are test-side problems; no production code changes here. One commit per check.TestRefreshReplay_HTTPTimeout:expected "rt-0", actual ""POST /tokencarryingrt-0; anything else is logged and answered 404.TestReviewFetchWaitBoundsWholeCommand/post:1.59s is not less than 1.45srunFetch, so it also timed the harness (temp dirs, config write, test server) and left 450ms for all of it on top of the 1s--wait.TestQAPilot_Explore: tool expectedapproved, actualpendingEvidence
OAuth fixture. In the failing run the test's own two token requests both arrived and every assertion on the test goroutine passed (one error in the output, test ended at 10.02s), so there was a third request, and
refreshTokenin mcp-go always sendsrefresh_token. Injecting a strayGETinto the unfixed test reproduces the CI output line for line; with the fix the same injected request is logged and the test passes. The sender of the CI request is not identified: the fixture did not log it. It does now.Fetch
--wait. Not reproduced locally (6 runs under 3×ncpu CPU load stayed at 1.03s). Readingreview_fetch.go: every stage runs under onecontext.WithTimeout(wait)andDoRawhas no retry, so the overrun is outside the deadline. The new assertion was checked both ways: with the discover POST taken off the shared context thepostcase fails; with a 600ms sleep added to the harness the test passes.Pilot model. Reproduced locally with the CI seed (
-rapid.seed=2531019367865565570). S30 and S31 fail against the old model and pass with the new one. The exploration lane passes with the CI seed and with a fresh seed, 200 checks each.go run ./cmd/qapilot mutate --only M20,M21: bothdetected.Verification
go test -race -count=1 ./internal/upstream/core/ ./internal/qapilot/... ./cmd/qapilot/...go test -race -count=1 -run 'TestReviewFetch|TestQAPilot' ./cmd/mcpproxy/ ./internal/runtime/golangci-lint run --config .github/.golangci.yml, bare and--build-tags server, on the touched packages: 0 issues./scripts/test-api-e2e.shnot run: no production code changedNot in this PR
TestEgress_RateLimitMovesNextCheckAndNothingIsRecorded(internal/security/scanner) failed once onmainon Nov 4 (run 41894266461):next_check_atstill showed the previous subtest's 24h cap instead of the 10-minute tick. Not investigated here.Review
opencode
github-copilot/gpt-6.1-sol, read-only, one round over the three commits against the production paths:VERDICT: SHIP, no findings.