fix: record MCP health on the transport every zip actually ships (#115) - #116
Merged
Conversation
…hips record_health() had one call site, inside register_adapter_server(). No zip carries the vendored library, so unless a site separately installs the official MCP Adapter plugin it runs the JSON-RPC transport, nothing ever wrote the record, and the Client traffic card opened with "No app has connected yet, so there's nothing to report" no matter what had happened. That is the first line of the one screen a support conversation points a customer at, and it reads as "this card is broken" — so the Record button underneath it, which does work and does answer the question, never gets pressed. Recorded from list_tools() rather than at route registration: abilities register lazily and rest_api_init fires on requests that have nothing to do with MCP, so this is the first moment the number is real. Counted before the tier filter, because the record answers "did the tools load?" — a property of the site — while "how many is this credential offered?" is per-request and already in the trace row. The copy was wrong in the same way on its other branch: it read the construction count as an availability count. It now says which it is and points at the per-request number for the other. Closes #115
This was referenced Aug 17, 2026
ifahimreza
added a commit
that referenced
this pull request
Aug 17, 2026
Carries the two fixes that rc5 missed. rc5 was built at 14:14 UTC on 2026-08-17 and #112 merged at 18:30 the same day, so the shim that exists specifically so ChatGPT can discover tools is on main but has never been in a zip — verified absent in dist/saddle-1.0.0-rc5.zip. Also fixes the version task itself. It wrote four of the five places the version lives and skipped package-lock.json entirely, so the lockfile sat at the previous version after every bump. Handled structurally rather than by text substitution: every dependency in that file carries a "version" field too, and a global regex for one would rewrite them all. Changelog entries for #112 and #116 written against 1.0.0 — Stable tag does not move for a release candidate.
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
1 participant
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.
Closes #115
What
Saddle_MCP_Diagnostics::record_health()now runs on the built-in JSON-RPC transport, not only on the adapter path. Both branches of the Client traffic card's headline are reworded to say what the number actually is.Why
The record had exactly one call site:
Saddle_MCP::record_server_health(), reached only fromregister_adapter_server(). Sincebff1a99the vendored library is in no zip, so unless a site separately installs the official MCP Adapter plugin it runsSaddle_MCP's own transport — where nothing wrote it.McpDiagnostics.jsxthen took itshealth.registered === undefinedbranch and printed "No app has connected yet, so there's nothing to report." on every build we ship, whatever had actually happened.That sentence is the first line of the one screen support points a customer at when their app connects and sees no tools. It reads as "this card is broken", so the Record the next hour button underneath — which works, and which is the thing that answers the question — never gets pressed. This is live right now in a customer conversation.
The other branch was wrong the same way:
%1$d of %2$d tools are available to connected appsreported a construction count (tools built vs. names handed over) as an availability count. Availability is tier-filtered per credential at dispatch and is already reported accurately, per request, as "N tools sent" in the trace.How
list_tools(), not at route registration: abilities register lazily andrest_api_initfires on requests that have nothing to do with MCP, so this is the first moment the number is real. Cost is one non-autoloadedget_optionon an actualtools/list, andrecord_health()already no-ops on an unchanged value.expectedandregisteredare the same list andassess()flags the case that does happen in the field: an empty one.Testing
composer test— 596 tests (was 594), 1993 assertions, 1 pre-existing skip, greenstashthe PHP change and they fail on "A tools/list must leave a health record on the built-in transport."composer lint— 0 errors (3 pre-existing warnings, none in the changed lines)npm run lint:js— cleannpm run build— bundle rebuilt and committedlanguages/saddle.potregenerated — diff is exactly the two changed strings plus the version stamp