Fix JSON deserialization crashes in MCP list_properties/list_units/list_leases/list_transactions - #13
Open
markd70 wants to merge 1 commit into
Open
Conversation
…st_leases/list_transactions TenantCloud's internal API returns property_status as a numeric value in some tenant accounts, but TcProperty.Status was typed as a plain string without a converter, causing System.Text.Json to throw and the MCP list_properties tool (and anything that triggers EntityCache, i.e. list_leases and list_transactions) to fail with a generic 'An error occurred invoking ...' message. Similarly, TcUnit.Price is a non-nullable decimal, but some units legitimately have a null price (e.g. no price set yet), crashing list_units the same way. Fixes: - Added JsonFlexibleStringConverter: accepts string, number, or boolean JSON tokens and normalizes to a string, matching the codebase's existing pattern of tolerant converters (see JsonTcLeaseStatusConverter, JsonAutoLongConverter, etc.) - Applied it to TcProperty.Status - Added JsonNullableDecimalConverter: accepts null, number, or numeric string and returns decimal? (or null) - Changed TcUnit.Price to decimal? and applied the new converter Verified against a live multi-property TenantCloud account (14 properties, units, leases, and transactions) after rebuilding tc-mcp self-contained win-x64 — all four previously-crashing tools now return correct data. Closes yllibed#12
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.
Problem
Fixes #12. Against a live multi-property tenant account,
list_properties,list_units,list_leases, andlist_transactionsall returned a genericAn error occurred invoking '...'.with no diagnostic detail.Root cause was two separate JSON shape mismatches:
property_statusonTcPropertycomes back as a number on this tenant, butStatusis a plainstring?with no converter ->System.Text.Json.JsonExceptionon any call that loads properties. SinceEntityCache(used to resolve unit/property names for leases and transactions) also loads all properties, this cascades and breakslist_leasesandlist_transactionstoo -- even though those tools have nothing to do with property status directly.priceonTcUnitis sometimesnull, butPriceis a non-nullabledecimal-> same style of crash onlist_units.Both failures were swallowed into the same generic MCP error string, so the underlying cause wasn't visible without attaching a debugger / running the server manually with debug logging.
Fix
JsonFlexibleStringConverter: accepts JSON string, number, or boolean tokens and normalizes tostring?, following the same pattern as the existing tolerant converters in this file (JsonTcLeaseStatusConverter,JsonAutoLongConverter, etc.). Applied toTcProperty.Status.JsonNullableDecimalConverter: accepts JSON null, number, or numeric string and returnsdecimal?. ChangedTcUnit.Pricetodecimal?and applied it.Testing
Rebuilt
tc-mcpself-contained win-x64 (dotnet publish -c Release -r win-x64 --self-contained true -p:PublishSingleFile=true) and ran it directly over stdio JSON-RPC against a live TenantCloud account with 14 properties across several buildings/units. Before the fix,list_properties,list_units,list_leases, andlist_transactionsall failed. After the fix, all four return correct data (verified property names/addresses, unit prices including nulls, lease status + tenant/unit name enrichment, and transaction records with status/category filters).get_user_infoandlist_contactswere unaffected before and after, as noted in #12.