refactor(availability): one shared per-entry probe for server + doctor (#435, part 1) - #437
Merged
Merged
Conversation
#435, part 1) list_supported_languages and doctor previously hand-mirrored the same per-entry loop (disabled gate, installed gate, getFactory fail-open, metadata-over-entry attach preference) with only computeModeAvailability shared — so the "doctor can never disagree with the server" guarantee rested on a mirroring convention. probeLanguageEntry (language-availability.ts) is now that loop, consumed by both: the server injects its TTL validation cache via the runValidate wrapper; doctor injects its per-probe timeout and records the raw outcome through the same seam. A parity test runs both paths against one registry fixture covering every availability shape (valid, invalid, throwing validate, metadata-attach override, not-installed, disabled) and asserts identical modes — the fence that keeps them from drifting apart again. Behavioral deltas, both strict improvements the doctor path already had: the server's loop now fault-isolates a throwing factory.getMetadata (previously it would crash the whole list_supported_languages response) and no longer swallows getFactory load errors invisibly (recorded on the probe result; server behavior unchanged — it still fails open). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Review batch on PR #437 (13 of 15 findings addressed; the remaining two — routing checkLaunchToolchain through the shared probe, and a typed getFactory surface instead of the duck-typed cast — noted on #435): - Timing regression: hoisting the probe clock made a slow factory dynamic import eat the validate/extras budget, manufacturing a spurious probe.timedOut that escalated to the handler's forced exit on healthy languages. The budget clock now starts when validate runs; factory-import time is excluded (regression-tested with a 900ms import + 500ms extras under a 1s budget). - A wedged getFactory (hung dynamic import) previously hung doctor forever — the whole probe now runs under a 2x-timeout envelope and reports broken + probe.timedOut, wiring it into the force-exit containment. - probeLanguageEntry now carries the validate outcome itself (validation / validationError / probeable) instead of doctor smuggling it out via mutated closure variables; dead result fields (installed, attach) dropped. - Unknown attach strings from third-party factory metadata are normalized (metadata → entry → 'none') so computeModeAvailability's non-exhaustive switch can no longer yield modes.attach undefined and crash a consumer. - getMetadata throws and validate-less factories now leave a warn-level breadcrumb (main's server previously surfaced the former as a hard error; the graceful fallback kept, the silence not). - Doctor's broken diagnoses are now accurate per shape: load failure (with the underlying error when the registry propagates one — the concrete AdapterRegistry swallows loader errors today, noted on #435), registry-returned-nothing, and loaded-but-no-validate (version skew), each with honest durationMs instead of 0. - list_supported_languages probes all languages in parallel like the doctor path (cold-cache cost = max, not sum). - Parity test reuses the shared createMockEnvironment / createMockFileSystem helpers. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Aug 23, 2026
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.
Part 1 of #435 (from the #423 doctor review):
list_supported_languagesandmcp-debugger doctorhand-mirrored the same per-entry availability loop — disabled gate, installed gate,getFactory().catchfail-open, metadata-over-entry attach preference — with onlycomputeModeAvailabilityactually shared. The "doctor can never disagree with the server" guarantee rested on keeping two loops manually in sync.What changed
probeLanguageEntry(src/utils/language-availability.ts) is now that loop, and both consumers call it:runValidatewrapper seam — same caching behavior as before, one line where the loop used to be.validation/probeError/timedOut) via closure; the wrapper rethrows socomputeModeAvailabilityfails open exactly as it does for the server, and doctor keeps reporting the divergence honestly (probe.failed/probe.timedOut).Parity fence:
tests/core/unit/server/server-doctor-parity.test.tsruns the reallist_supported_languageshandler and the realdiagnose()against one registry fixture covering every availability shape (valid toolchain, invalid, throwing validate, metadata-attach overriding the registry entry, not-installed,DEBUG_MCP_DISABLE_LANGUAGES) and asserts the per-languagemodesare identical. If either path grows semantics the other doesn't, this test fails.Plus 10 unit tests for
probeLanguageEntryitself (disabled/not-installed gates skip factory load, fail-open on load error, metadata preference,runValidateseam, unprobeable factories).Behavioral deltas (server side, both strict improvements doctor already had)
factory.getMetadata()no longer crashes the wholelist_supported_languagesresponse — it falls back to the registry entry's attach declaration.getFactoryload errors are recorded on the probe result instead of being silently discarded (the server still fails open; doctor uses the recorded error for itsbrokenverdict).Part 2 of #435 (adapter-owned doctor presentation via typed
AdapterMetadatafields or an optional factory method) stays open — it's a wider@debugmcp/sharedinterface change across all nine adapter packages.Verification
list_supported_languagestests unchanged.doctor python mockon the rebuilt dist: identical output, exit 0.🤖 Generated with Claude Code