fix(release): drain base-reds r7 of v3.8.51 — connection-test operator-disable race, two vitest teardown leaks - #15130
Merged
Conversation
…ot the pre-probe snapshot testSingleConnection() read the connection (cached) before the probe and decided whether to activate on that snapshot after the probe returned. The probe can take seconds, and POST /api/providers fires one in the background on create, so an operator disable landing mid-probe was undone: the stale snapshot still read 'never activated' and the passing test wrote isActive:true (and could overwrite providerSpecificData with the stale copy, dropping the operator-disable marker). This race made the two connection-test-respects-operator-disable tests flaky in CI (#14941). Re-read the row uncached right before the write and use it for the operator-disable check and the key-health recovery base. New test holds a probe in flight, disables the connection, and asserts it stays off.
…down
The vitest UI job passed every file but failed on two unhandled errors:
- translator-friendly-raw-json-panel: roots were never unmounted, so the
RawJsonPanel 600 ms auto-detect debounce stayed armed and fired after the
environment was torn down ('window is not defined'). Cleanup now unmounts
each root; a new test proves no auto-detect runs after cleanup.
- playground-studio: the next/dynamic mock started each tab import without
awaiting it; all three were still loading when the tests finished
(EnvironmentTeardownError on BuildTab's import graph). The imports are now
tracked and awaited in afterAll.
… size Tighten the fresh-row read and two nearby comments so the route stays under its check-file-size ceiling (1252 lines).
Jaani2634
pushed a commit
to Jaani2634/OmniRoute
that referenced
this pull request
Sep 29, 2026
… to the provider test route (diegosouzapw#15134) Release-captain base-red fix: classify diegosouzapw#15130's uncached connection re-read in the hard-lease inventory (class C state read). Inventory 3/3.
Merged
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.
Drains two reds from the release PR #11442 CI (run 36601097731, head fc8e91b).
1. Unit Tests (6/8):
connection-test-respects-operator-disable(2 tests)Cause — a real production race, not DNS.
testSingleConnection()reads the connection (cached) before the probe and decided whether to activate on that snapshot after the probe returned.POST /api/providersfires a backgroundtestSingleConnection()on create, so in the test (and in production) this interleaving happens:isActive:false, no marker ("never activated"), starts probing;operatorDisabledAt);isOperatorDisabled(staleSnapshot)is false → writesisActive:true.recoverKeyHealth()also rebuiltproviderSpecificDatafrom the stale copy, which could drop the marker.The
ENOTFOUND proxy.operator-disable.example.comlines are noise: the undici dispatcher falls back to the stubbedfetch. They only change the probe's timing. The test has been intermittent since it landed with #14941. It failed on ed664fc, 0816f62 and 6f665f3, and passed on 4f65ef3 and 4796624, so it is not a #15101/#15064/#15045 regression.Fix (
src/app/api/providers/[id]/test/route.ts): re-read the row uncached right before the write and use it for the operator-disable check (both the skipped and the valid paths) and as therecoverKeyHealthbase.Proof: new test
an operator disable during an in-flight probe is not undone when the probe passesholds the probe on a gate, disables the connection mid-probe, then releases it. It was red on the tip (1 !== 0) and is green with the fix. The file passed 4/4 three times, and all 19 neighbouring files that exercisetestSingleConnectionpassed (159/159).2. Vitest UI: 2 unhandled errors (all 431 files passed)
Unhandled Rejection: window is not defined(RawJsonPanel.tsx:85), fromtests/unit/translator-friendly-raw-json-panel.test.tsx. The suite never unmounted its React roots. Removing the container does not run effect cleanups, so the 600 ms auto-detect debounce stayed armed and fired after jsdom teardown. The component itself is correct because it clears the timer on unmount. Fix: amountRoot()helper unmounts every root on cleanup. The new testcleanup leaves no debounced auto-detect running after the testwas red before the harness fix (the detectfetchwas called 3 times after cleanup) and is green after.EnvironmentTeardownError: Cannot load partialWithoutDefaults.ts … after the environment was torn down, fromtests/unit/ui/playground-studio.test.tsx. Thenext/dynamicmock started each tab import without awaiting it. A probe showed all 3 imports still pending whenafterAllstarted ([false,false,false]in 3/3 runs). Fix: the imports are tracked and awaited inafterAll. Unhandled-error detection is unchanged.Both files passed 3/3 (22 tests).
Gates
typecheck:coreOK ·check:open-sse-typecheckOK (0) · eslint on the changed files OK ·check-file-sizeOK after commit (the route was tightened back under its 1252-line ceiling).Note:
tests/unit/monitoring-health-cached-credential.test.ts› "returns the stale cached summary immediately" failed once locally at 2262 ms against a 2000 ms budget. That is cold-import timing: it passed 3/3 with this change (1.1 to 1.7 s) and does not touch the connection-test route. It is not addressed here.